[MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu #6361

zhongnansu · 2024-04-07T06:52:44Z

Description

Read data_source.hideLocalCluster setting from opensearch_dashboards.yml, and always use this value to decide if to hide local cluster in DataSourceSelector and DataSourceMenu. This is expected behaviour because hide/show local cluster in data source visual components should be consistent across anywhere that consumes those components.
make hideLocalHost an optional property of DataSourceSelector and DataSourceMenu
Update related unit tests

Issues Resolved

#6336

Screenshot

iShot_2024-04-08_00.34.05.mp4

Testing the changes

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

BionIT

Added comments, in the recording, we can also show the local cluster should not be an option any more in the drop down when hidelocalcluster prop is set true

BionIT · 2024-04-07T23:20:29Z

...lugins/data_source_management/public/components/data_source_menu/create_data_source_menu.tsx

          </EuiHeaderLinks>
        </MountPointPortal>
      );
    }
-    return <DataSourceMenu {...props} />;
+    return <DataSourceMenu {...props} hideLocalCluster={hideLocalCluster} />;


Looks like we missed adding the uiSettings option here in 726fb0e, could you help to add uiSettings here, to test, please use the example plugin for top nav menu https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/examples/multiple_data_source_examples/public/components/data_source_via_top_nav_menu.tsx

Added. However when I tried to test it by using DataSourceSelectable for top nav menu, it doesn't show "default" label for any options, Here's the recording.

iShot_2024-04-08_00.11.24.mp4

I think the issue is with that in top_nav_menu.tsx, DataSourceMenu is not obtained by calling the dataSourceManagement.ui.getDataSourceMenu<DataSourceSelectableConfig>();, but through a direct import as below code suggests.

OpenSearch-Dashboards/src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx

Line 44 in a0eaf84

import { DataSourceMenu, DataSourceMenuProps } from '../../../data_source_management/public';

Therefore, even I added the uiSetting to createDataSourceMenu(), it makes no difference for top nav menu use case, because ui.getDataSourceMenu API is never called, and uiSetting is never being passed through.

To solve this, we may want to make change to retrieve DataSourceMenu through the ui.getDataSourceMenu in top_nav_menu.tsx. This also brings up another issue with dev_tool and tutorial plugin, should they also change the way how they consume data source selector, to have consistent behavior? @BionIT What do you think? I can file an issue if it makes sense

Right, yeah, could you help to cut an issue for that?

created issue #6368 and #6369

zhongnansu · 2024-04-08T07:36:48Z

Added comments, in the recording, we can also show the local cluster should not be an option any more in the drop down when hidelocalcluster prop is set true

sure, updated the recording

…ector and data source menu Signed-off-by: Zhongnan Su <szhongna@amazon.com>

codecov · 2024-04-08T07:55:55Z

Codecov Report

Attention: Patch coverage is 88.88889% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 54.53%. Comparing base (a0eaf84) to head (9e04be8).

❗ Current head 9e04be8 differs from pull request most recent head af54941. Consider uploading reports for the commit af54941 to get more accurate results

Files	Patch %	Lines
...rc/plugins/data_source_management/public/plugin.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #6361       +/-   ##
===========================================
- Coverage   67.56%   54.53%   -13.04%     
===========================================
  Files        3379     2271     -1108     
  Lines       65894    44499    -21395     
  Branches    10660     8276     -2384     
===========================================
- Hits        44522    24268    -20254     
+ Misses      18774    18469      -305     
+ Partials     2598     1762      -836

Flag	Coverage Δ
Linux_1	`?`
Linux_2	`?`
Linux_3	`45.01% <88.88%> (+<0.01%)`	⬆️
Linux_4	`34.98% <0.00%> (-0.01%)`	⬇️
Windows_1	`?`
Windows_2	`?`
Windows_3	`?`
Windows_4	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yujin-emma

LGTM

...ata_source_management/public/components/data_source_selector/create_data_source_selector.tsx

Signed-off-by: Zhongnan Su <szhongna@amazon.com>

…ector and data source menu (#6361) * [MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu Signed-off-by: Zhongnan Su <szhongna@amazon.com> * improve naming Signed-off-by: Zhongnan Su <szhongna@amazon.com> --------- Signed-off-by: Zhongnan Su <szhongna@amazon.com> (cherry picked from commit b4130aa) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

…ector and data source menu (#6361) (#6374) * [MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu Signed-off-by: Zhongnan Su <szhongna@amazon.com> * improve naming Signed-off-by: Zhongnan Su <szhongna@amazon.com> --------- Signed-off-by: Zhongnan Su <szhongna@amazon.com> (cherry picked from commit b4130aa) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

github-actions bot added the all-star-contributor label Apr 7, 2024

zhongnansu force-pushed the bug-6336 branch from 832cd53 to 519501d Compare April 7, 2024 07:05

BionIT reviewed Apr 7, 2024

View reviewed changes

[MD]Read hideLocalCluster setting from yml and set in data source sel…

9e04be8

…ector and data source menu Signed-off-by: Zhongnan Su <szhongna@amazon.com>

zhongnansu force-pushed the bug-6336 branch from 519501d to 9e04be8 Compare April 8, 2024 07:38

zhongnansu marked this pull request as ready for review April 8, 2024 07:40

zhongnansu requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl and xinruiba as code owners April 8, 2024 07:40

BionIT previously approved these changes Apr 8, 2024

View reviewed changes

yujin-emma approved these changes Apr 8, 2024

View reviewed changes

zhongnansu mentioned this pull request Apr 8, 2024

[BUG] data_source.hideLocalCluster setting and default datasource is not being honored when mounting data source menu via Top Nav Menu #6368

Closed

xinruiba reviewed Apr 8, 2024

View reviewed changes

...ata_source_management/public/components/data_source_selector/create_data_source_selector.tsx Outdated Show resolved Hide resolved

huyaboo mentioned this pull request Apr 8, 2024

[MDS] Update Data source selector behavior for defaultOption #6372

Merged

7 tasks

improve naming

af54941

Signed-off-by: Zhongnan Su <szhongna@amazon.com>

zhongnansu dismissed BionIT’s stale review via af54941 April 8, 2024 23:02

xinruiba approved these changes Apr 8, 2024

View reviewed changes

BionIT approved these changes Apr 8, 2024

View reviewed changes

BionIT merged commit b4130aa into opensearch-project:main Apr 8, 2024
66 checks passed

zhongnansu added the backport 2.x label Apr 8, 2024

opensearch-trigger-bot bot mentioned this pull request Apr 8, 2024

[Backport 2.x] [MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu #6374

Merged

zhongnansu added the v2.14.0 label Apr 9, 2024

zhongnansu mentioned this pull request Apr 16, 2024

[MDS] TSVB Support #6298

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu #6361

[MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu #6361

zhongnansu commented Apr 7, 2024 •

edited

Loading

BionIT left a comment

BionIT Apr 7, 2024

zhongnansu Apr 8, 2024 •

edited

Loading

BionIT Apr 8, 2024

zhongnansu Apr 8, 2024 •

edited

Loading

zhongnansu commented Apr 8, 2024

codecov bot commented Apr 8, 2024 •

edited

Loading

yujin-emma left a comment

[MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu #6361

[MD]Read hideLocalCluster setting from yml and set in data source selector and data source menu #6361

Conversation

zhongnansu commented Apr 7, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Check List

BionIT left a comment

Choose a reason for hiding this comment

BionIT Apr 7, 2024

Choose a reason for hiding this comment

zhongnansu Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

BionIT Apr 8, 2024

Choose a reason for hiding this comment

zhongnansu Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

zhongnansu commented Apr 8, 2024

codecov bot commented Apr 8, 2024 • edited Loading

Codecov Report

yujin-emma left a comment

Choose a reason for hiding this comment

zhongnansu commented Apr 7, 2024 •

edited

Loading

zhongnansu Apr 8, 2024 •

edited

Loading

zhongnansu Apr 8, 2024 •

edited

Loading

codecov bot commented Apr 8, 2024 •

edited

Loading