Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Enhancement] Deregister dashboards, applications, logs in MDS #2097

Merged
merged 5 commits into from
Aug 28, 2024
Merged

[Enhancement] Deregister dashboards, applications, logs in MDS #2097

merged 5 commits into from
Aug 28, 2024

Conversation

sumukhswamy
Copy link
Collaborator

Description

De-register plugins when in NEO

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@sumukhswamy
Deregister plugins in NEO
6a63164 
Signed-off-by: sumukhswamy <[email protected]>
@sumukhswamy sumukhswamy added backport 2.x enhancement New feature or request labels Aug 26, 2024
@sumukhswamy sumukhswamy changed the title [Enhanvement] Deregister plugins in NEO [Enhanvement] Deregister plugins when MDS is enabled Aug 26, 2024
@ps48 ps48 changed the title [Enhanvement] Deregister plugins when MDS is enabled [Enhanvement] Deregister dashboards, applications, logs in MDS Aug 26, 2024
@sumukhswamy
updated the comment
587bf53 
Signed-off-by: sumukhswamy <[email protected]>
@sumukhswamy sumukhswamy changed the title [Enhanvement] Deregister dashboards, applications, logs in MDS [Enhancement] Deregister dashboards, applications, logs in MDS Aug 26, 2024
ps48
ps48 reviewed Aug 26, 2024
View reviewed changes
public/plugin.tsx
Comment on lines +311 to +318
core.application.register({
id: observabilityApplicationsID,
title: observabilityApplicationsTitle,
category: OBSERVABILITY_APP_CATEGORIES.observability,
order: observabilityApplicationsPluginOrder,
mount: appMountWithStartPage('applications'),
});
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when we have enabled the new navigation and MDS together? Do have to modify plugin_nav file to not register applications there where MDS is enabled?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not break anything as the core navigation group just fails to find observabilityApplicationsID and doesn't register anything to the left side. It would be cleaner to handle this condition though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just fails to find observabilityApplicationsID

@TackAdam @sumukhswamy Is this leading to some error on browser or Dashboards app? If so then let's handle that in this PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no error seen

@TackAdam
Copy link
Collaborator

LGTM.
Checked functionality and seem correct.
For old navigation mds being enabled no longer registers Applications, Logs, or Dashboards.
Old Navigation MDS off:
OldNav_MDS_Off
Old Navigation MDS on: (application, logs, and dashboards present)
OldNav_MDS_On

For new navigation mds on now disables applications:
NewNav_MDS_On
And it remains when disabled:(mds off)
NewNac_MDS_Off

@sumukhswamy sumukhswamy merged commit 428cc98 into opensearch-project:main Aug 28, 2024
10 of 18 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 28, 2024
@github-actions
[Enhancement] Deregister dashboards, applications, logs in MDS (#2097)
178113b 
* Deregister plugins in NEO

Signed-off-by: sumukhswamy <[email protected]>

* updated the comment

Signed-off-by: sumukhswamy <[email protected]>

* Deregister plugins in NEO

Signed-off-by: sumukhswamy <[email protected]>

---------

Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: Sumukh Swamy <[email protected]>
(cherry picked from commit 428cc98)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 28, 2024
Merged
ps48 pushed a commit that referenced this pull request Aug 28, 2024
@opensearch-trigger-bot @github-actions
[Enhancement] Deregister dashboards, applications, logs in MDS (#2097) (
1430567 
#2104)

* Deregister plugins in NEO



* updated the comment



* Deregister plugins in NEO



---------



(cherry picked from commit 428cc98)

Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: Sumukh Swamy <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ps48 ps48 ps48 approved these changes

@TackAdam TackAdam TackAdam approved these changes

Assignees
No one assigned
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sumukhswamy @TackAdam @ps48