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.

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

[MDS] Add Vega support for importing saved objects #6123

Merged
merged 31 commits into from
Mar 19, 2024

Conversation

huyaboo
Copy link
Member

@huyaboo huyaboo commented Mar 12, 2024

Description

This PR adds (limited) support for importing Vega saved objects when data_source.enabled: true and when specifying a datasource. Support in this case means:

  • A user can import Vega Visualizations from non-MDS enabled clusters to MDS enabled clusters in create mode or in overwrite mode
  • A user can import Vega Visualizations from other MDS enabled clusters to MDS enabled clusters in create mode or in overwrite mode

Limitations:

  • Only one data_source_name can be changed; if users have multiple data_source_names specified in their Vega spec, the origin datasource name will be changed but the others will stay the same
  • Possibility of duplicate Data-source saved objects being created. The user may potentially have to handle that

Issues Resolved

Closes #5927

Screenshot

When updating the data_source_names, the datasource references also change. In this recording, Data Source A is updated to Data Source B

Screen.Recording.2024-03-15.at.7.48.07.PM.mov

Importing a similar visualization that references Data Source B and has a local query will update the spec to include Data Source A in the old local queries. Data Source A is also shown to be added as a reference

Screen.Recording.2024-03-15.at.8.31.20.PM.mov
Screen.Recording.2024-03-12.at.1.11.02.PM.mov

Testing the changes

The following scenarios were tested:

  • Import from non-MDS cluster -> non-MDS cluster (existing functionality should not be impacted)
  • Import from non-MDS cluster -> MDS cluster (new datasource name should be attached to each OpenSearch query)
  • Import from MDS cluster -> MDS cluster (all local queries will be replaced with the new data_source_name and the references will append this new datasource id)

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

Signed-off-by: Huy Nguyen <[email protected]>
Signed-off-by: Huy Nguyen <[email protected]>
Signed-off-by: Huy Nguyen <[email protected]>
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

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

Project coverage is 67.31%. Comparing base (4a77617) to head (18f412c).

Files Patch % Lines
...erver/saved_objects/import/create_saved_objects.ts 68.88% 10 Missing and 4 partials ⚠️
src/core/server/saved_objects/import/utils.ts 90.24% 2 Missing and 2 partials ⚠️
...d_objects/import/check_conflict_for_data_source.ts 80.00% 0 Missing and 2 partials ⚠️
src/plugins/vis_type_vega/server/utils.ts 95.23% 1 Missing and 1 partial ⚠️
...e_vega/server/vega_visualization_client_wrapper.ts 93.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6123      +/-   ##
==========================================
+ Coverage   67.25%   67.31%   +0.06%     
==========================================
  Files        3345     3348       +3     
  Lines       64835    64966     +131     
  Branches    10432    10465      +33     
==========================================
+ Hits        43602    43731     +129     
+ Misses      18690    18689       -1     
- Partials     2543     2546       +3     
Flag Coverage Δ
Linux_1 31.72% <44.04%> (+0.07%) ⬆️
Linux_2 55.55% <79.16%> (+0.10%) ⬆️
Linux_3 44.68% <6.25%> (-0.05%) ⬇️
Linux_4 35.02% <6.25%> (-0.05%) ⬇️
Windows_1 31.74% <44.04%> (+0.07%) ⬆️
Windows_2 55.52% <79.16%> (+0.10%) ⬆️
Windows_3 44.68% <6.25%> (-0.07%) ⬇️
Windows_4 35.02% <6.25%> (-0.05%) ⬇️

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.

@huyaboo huyaboo changed the title Import vega [MDS] Add Vega support for importing saved objects Mar 12, 2024
@huyaboo huyaboo marked this pull request as ready for review March 12, 2024 20:51
Signed-off-by: Huy Nguyen <[email protected]>
]),
})
);
});
Copy link
Member

Choose a reason for hiding this comment

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

minor: for better coverage, maybe we want to test that savedobjectclient.get have been called? And also test called with expected attributes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, added a test to ensure get() was called.

]),
})
);
});
Copy link
Member

Choose a reason for hiding this comment

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

same as above

Copy link
Member

Choose a reason for hiding this comment

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

also can we add test for duplicate and undefined data source title?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since this makes a get by id, I don't think we need to check for duplicate titles here since there is no ambiguity. The duplicate title check will be done when the visualization is saved and when the visualization is rendered.

BionIT
BionIT previously approved these changes Mar 19, 2024
BionIT
BionIT previously approved these changes Mar 19, 2024
ZilongX
ZilongX previously approved these changes Mar 19, 2024
BionIT
BionIT previously approved these changes Mar 19, 2024
Signed-off-by: Huy Nguyen <[email protected]>
@huyaboo huyaboo dismissed stale reviews from BionIT and ZilongX via c392de4 March 19, 2024 21:13
@bandinib-amzn bandinib-amzn merged commit de978d4 into opensearch-project:main Mar 19, 2024
68 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 19, 2024
* Add MDS support for Vega

Signed-off-by: Huy Nguyen <[email protected]>

* Refactor field to data_source_id

Signed-off-by: Huy Nguyen <[email protected]>

* Add to CHANGELOG.md

Signed-off-by: Huy Nguyen <[email protected]>

* Added test cases and renamed field to use data_source_name

Signed-off-by: Huy Nguyen <[email protected]>

* Add prefix datasource name test case and add example in default hjson

Signed-off-by: Huy Nguyen <[email protected]>

* Move CHANGELOG to appropriate section

Signed-off-by: Huy Nguyen <[email protected]>

* Increased test coverage of search() method

Signed-off-by: Huy Nguyen <[email protected]>

* Add test cases for util function

Signed-off-by: Huy Nguyen <[email protected]>

* Add util function to modify Vega Spec

Signed-off-by: Huy Nguyen <[email protected]>

* Add method to verify Vega saved object type

Signed-off-by: Huy Nguyen <[email protected]>

* Add import saved object support for Vega

Signed-off-by: Huy Nguyen <[email protected]>

* Add unit tests for Vega objects in create and conflict modes

Signed-off-by: Huy Nguyen <[email protected]>

* Refactored utils test file

Signed-off-by: Huy Nguyen <[email protected]>

* Add to CHANGELOG

Signed-off-by: Huy Nguyen <[email protected]>

* Use bulkget instead of single get

Signed-off-by: Huy Nguyen <[email protected]>

* Add datasource references to the specs

Signed-off-by: Huy Nguyen <[email protected]>

* Fix bootstrap errors

Signed-off-by: Huy Nguyen <[email protected]>

* Add edge case where title is potentially undefined

Signed-off-by: Huy Nguyen <[email protected]>

* Address PR comments

Signed-off-by: Huy Nguyen <[email protected]>

* Add more test coverage for checking conflict

Signed-off-by: Huy Nguyen <[email protected]>

* Fix unit test

Signed-off-by: Huy Nguyen <[email protected]>

---------

Signed-off-by: Huy Nguyen <[email protected]>
(cherry picked from commit de978d4)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
@huyaboo huyaboo mentioned this pull request Mar 19, 2024
7 tasks
Flyingliuhub pushed a commit that referenced this pull request Mar 20, 2024
* Add MDS support for Vega

Signed-off-by: Huy Nguyen <[email protected]>

* Refactor field to data_source_id

Signed-off-by: Huy Nguyen <[email protected]>

* Add to CHANGELOG.md

Signed-off-by: Huy Nguyen <[email protected]>

* Added test cases and renamed field to use data_source_name

Signed-off-by: Huy Nguyen <[email protected]>

* Add prefix datasource name test case and add example in default hjson

Signed-off-by: Huy Nguyen <[email protected]>

* Move CHANGELOG to appropriate section

Signed-off-by: Huy Nguyen <[email protected]>

* Increased test coverage of search() method

Signed-off-by: Huy Nguyen <[email protected]>

* Add test cases for util function

Signed-off-by: Huy Nguyen <[email protected]>

* Add util function to modify Vega Spec

Signed-off-by: Huy Nguyen <[email protected]>

* Add method to verify Vega saved object type

Signed-off-by: Huy Nguyen <[email protected]>

* Add import saved object support for Vega

Signed-off-by: Huy Nguyen <[email protected]>

* Add unit tests for Vega objects in create and conflict modes

Signed-off-by: Huy Nguyen <[email protected]>

* Refactored utils test file

Signed-off-by: Huy Nguyen <[email protected]>

* Add to CHANGELOG

Signed-off-by: Huy Nguyen <[email protected]>

* Use bulkget instead of single get

Signed-off-by: Huy Nguyen <[email protected]>

* Add datasource references to the specs

Signed-off-by: Huy Nguyen <[email protected]>

* Fix bootstrap errors

Signed-off-by: Huy Nguyen <[email protected]>

* Add edge case where title is potentially undefined

Signed-off-by: Huy Nguyen <[email protected]>

* Address PR comments

Signed-off-by: Huy Nguyen <[email protected]>

* Add more test coverage for checking conflict

Signed-off-by: Huy Nguyen <[email protected]>

* Fix unit test

Signed-off-by: Huy Nguyen <[email protected]>

---------

Signed-off-by: Huy Nguyen <[email protected]>
(cherry picked from commit de978d4)
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>
Co-authored-by: Ashwin P Chandran <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 20, 2024
* Add MDS support for Vega

Signed-off-by: Huy Nguyen <[email protected]>

* Refactor field to data_source_id

Signed-off-by: Huy Nguyen <[email protected]>

* Add to CHANGELOG.md

Signed-off-by: Huy Nguyen <[email protected]>

* Added test cases and renamed field to use data_source_name

Signed-off-by: Huy Nguyen <[email protected]>

* Add prefix datasource name test case and add example in default hjson

Signed-off-by: Huy Nguyen <[email protected]>

* Move CHANGELOG to appropriate section

Signed-off-by: Huy Nguyen <[email protected]>

* Increased test coverage of search() method

Signed-off-by: Huy Nguyen <[email protected]>

* Add test cases for util function

Signed-off-by: Huy Nguyen <[email protected]>

* Add util function to modify Vega Spec

Signed-off-by: Huy Nguyen <[email protected]>

* Add method to verify Vega saved object type

Signed-off-by: Huy Nguyen <[email protected]>

* Add import saved object support for Vega

Signed-off-by: Huy Nguyen <[email protected]>

* Add unit tests for Vega objects in create and conflict modes

Signed-off-by: Huy Nguyen <[email protected]>

* Refactored utils test file

Signed-off-by: Huy Nguyen <[email protected]>

* Add to CHANGELOG

Signed-off-by: Huy Nguyen <[email protected]>

* Use bulkget instead of single get

Signed-off-by: Huy Nguyen <[email protected]>

* Add datasource references to the specs

Signed-off-by: Huy Nguyen <[email protected]>

* Fix bootstrap errors

Signed-off-by: Huy Nguyen <[email protected]>

* Add edge case where title is potentially undefined

Signed-off-by: Huy Nguyen <[email protected]>

* Address PR comments

Signed-off-by: Huy Nguyen <[email protected]>

* Add more test coverage for checking conflict

Signed-off-by: Huy Nguyen <[email protected]>

* Fix unit test

Signed-off-by: Huy Nguyen <[email protected]>

---------

Signed-off-by: Huy Nguyen <[email protected]>
(cherry picked from commit de978d4)
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>
Co-authored-by: Ashwin P Chandran <[email protected]>
(cherry picked from commit d144637)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
BionIT pushed a commit that referenced this pull request Mar 20, 2024
…6222)

* Add MDS support for Vega



* Refactor field to data_source_id



* Add to CHANGELOG.md



* Added test cases and renamed field to use data_source_name



* Add prefix datasource name test case and add example in default hjson



* Move CHANGELOG to appropriate section



* Increased test coverage of search() method



* Add test cases for util function



* Add util function to modify Vega Spec



* Add method to verify Vega saved object type



* Add import saved object support for Vega



* Add unit tests for Vega objects in create and conflict modes



* Refactored utils test file



* Add to CHANGELOG



* Use bulkget instead of single get



* Add datasource references to the specs



* Fix bootstrap errors



* Add edge case where title is potentially undefined



* Address PR comments



* Add more test coverage for checking conflict



* Fix unit test



---------


(cherry picked from commit de978d4)


# Conflicts:
#	CHANGELOG.md



(cherry picked from commit d144637)

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>
Co-authored-by: Ashwin P Chandran <[email protected]>
@huyaboo huyaboo deleted the import-vega branch March 29, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Vega support with MDS
6 participants