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

[Workspace] Add duplicate saved objects API #6288

Merged
merged 26 commits into from
Apr 12, 2024

Conversation

gaobinlong
Copy link
Contributor

@gaobinlong gaobinlong commented Mar 28, 2024

Description

This PR adds a new server api named duplicate_saved_objects which can be used to duplicate saved objects among workspaces, the implementation is simple, we leverage the code of the existing export api and import api to implement this functionality. In addition, same to export api and import api, the duplicate api will also be under the control of the setting savedObjects.maxImportExportSize(defaults to 10000) and savedObjects.maxImportPayloadBytes (defaults to 26214400).

For this new api, the usage is:
Request:

curl -X POST "localhost:5601/api/workspaces/_duplicate_saved_objects" -H 'osd-xsrf: true' -H 'Content-Type: application/json' -d '{"objects":[{"type":"index-pattern","id":"619cc200-ecd0-11ee-95b1-e7363f9e289d"}],"targetWorkspace":"9gt4lB"}'

Response:

{"successCount":1,"success":true,"successResults":[{"type":"index-pattern","id":"619cc200-ecd0-11ee-95b1-e7363f9e289d","meta":{"title":"test*","icon":"indexPatternApp"},"destinationId":"2ffee5da-55b3-49b4-b9e1-c3af5d1adbd3"}]}

Issues Resolved

#6286

Screenshot

No UI change in this PR, the UI change will be in the following PR, here is the video about it:

2024-04-07.10.55.45.mov

Testing the changes

  1. Clone the latest code and run yarn osd bootstrap
  2. Modify config/opensearch_dashboards.yml, add workspace.enabled: true
  3. Run yarn start --no-base-path
  4. Create a workspace and create some saved objects like index patterns, visualizations, etc.
  5. Call the copy api following the above usage.

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: gaobinlong <[email protected]>
Signed-off-by: gaobinlong <[email protected]>
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.62%. Comparing base (85df662) to head (8c9a08d).
Report is 7 commits behind head on main.

❗ Current head 8c9a08d differs from pull request most recent head 7172f55. Consider uploading reports for the commit 7172f55 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6288      +/-   ##
==========================================
+ Coverage   55.58%   55.62%   +0.03%     
==========================================
  Files        1199     1199              
  Lines       24259    24259              
  Branches     4087     4087              
==========================================
+ Hits        13485    13493       +8     
+ Misses      10133    10124       -9     
- Partials      641      642       +1     
Flag Coverage Δ
Linux_2 55.62% <ø> (?)
Windows_2 ?

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.

Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

Can we add docs for the new API changes? Als can we document the existing API as a part of this change?

@gaobinlong
Copy link
Contributor Author

Can we add docs for the new API changes? Als can we document the existing API as a part of this change?

I'm glad to do that, and how could we add documents for the server apis?

@ashwin-pc
Copy link
Member

Here are all the Readme's in the repo that document various features. https://opensearch-project.github.io/OpenSearch-Dashboards/docs/index.html#/. I think adding it to the main saved object plugin readme should be good enough. https://opensearch-project.github.io/OpenSearch-Dashboards/docs/index.html#/../src/plugins/saved_objects/README

@gaobinlong
Copy link
Contributor Author

Here are all the Readme's in the repo that document various features. https://opensearch-project.github.io/OpenSearch-Dashboards/docs/index.html#/. I think adding it to the main saved object plugin readme should be good enough. https://opensearch-project.github.io/OpenSearch-Dashboards/docs/index.html#/../src/plugins/saved_objects/README

I've added the documentation for all saved objects API, the format refers to OpenSearch API's document, please help to take a look, thanks!

Signed-off-by: gaobinlong <[email protected]>
SuZhou-Joe
SuZhou-Joe previously approved these changes Apr 9, 2024
@gaobinlong
Copy link
Contributor Author

Hi @gaobinlong,

I am taking one step back and thinking logic again after watching video and going over PR again - Why we need to introduce new API? You already retrieve and load data on page. You just need to create new saved object with new workspace. Why you are exporting and importing again?

And if we still want to use export import workflow, instead of adding new api, we can call directly existing export and import api, right? With new approach I observed you need to add validation logic which we already have in export api.

Thanks @bandinib-amzn , I think all of your comments above have been addressed yet, please take a look at the latest change, thank you!

For why we add this new API, firstly, the loaded data on the page are not complete, we need to fetch all the details of the references before creating them in the target workspace, secondly, instead of call export API and then import API, this new API makes it easier for users to duplicate saved objects when they just call the server APIs to complete their work, I saw many users asked about how to call the OSD's API in OpenSearch's forum, so I think this new API may reduce the complexity when they need.

Signed-off-by: gaobinlong <[email protected]>
SuZhou-Joe
SuZhou-Joe previously approved these changes Apr 10, 2024
Copy link
Member

@SuZhou-Joe SuZhou-Joe left a comment

Choose a reason for hiding this comment

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

LGTM

@bandinib-amzn
Copy link
Member

Build and test / Build and Verify on Linux (ciGroup3) CI is failing due to #6360 . PR is open #6411 to fix issue.

@SuZhou-Joe SuZhou-Joe merged commit 7eda01a into opensearch-project:main Apr 12, 2024
65 of 66 checks passed
gaobinlong added a commit to gaobinlong/OpenSearch-Dashboards that referenced this pull request Apr 12, 2024
* Add copy saved objects API

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

* Modify change log

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

* Add documents for all saved objects APIs

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

* Revert the yml file change

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

* Move the duplicate api to workspace plugin

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

* Modify change log

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

* Modify api doc

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

* Check target workspace exists or not

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

* Remove unused import

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

* Fix test failure

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

* Modify change log

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

* Modify workspace doc

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

* Add more unit tests

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

* Some minor change

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

* Fix test failure

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

* Modify test description

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

* Optimize test description

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

* Modify test case

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

* Minor change

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

---------

Signed-off-by: gaobinlong <[email protected]>
SuZhou-Joe pushed a commit to ruanyl/OpenSearch-Dashboards that referenced this pull request Apr 12, 2024
…#326)

* Add copy saved objects API



* Modify change log



* Add documents for all saved objects APIs



* Revert the yml file change



* Move the duplicate api to workspace plugin



* Modify change log



* Modify api doc



* Check target workspace exists or not



* Remove unused import



* Fix test failure



* Modify change log



* Modify workspace doc



* Add more unit tests



* Some minor change



* Fix test failure



* Modify test description



* Optimize test description



* Modify test case



* Minor change



---------

Signed-off-by: gaobinlong <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 19, 2024
* Add copy saved objects API

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

* Modify change log

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

* Add documents for all saved objects APIs

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

* Revert the yml file change

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

* Move the duplicate api to workspace plugin

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

* Modify change log

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

* Modify api doc

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

* Check target workspace exists or not

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

* Remove unused import

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

* Fix test failure

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

* Modify change log

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

* Modify workspace doc

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

* Add more unit tests

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

* Some minor change

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

* Fix test failure

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

* Modify test description

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

* Optimize test description

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

* Modify test case

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

* Minor change

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

---------

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

# Conflicts:
#	CHANGELOG.md
SuZhou-Joe pushed a commit that referenced this pull request Apr 22, 2024
* Add copy saved objects API

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

* Modify change log

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

* Add documents for all saved objects APIs

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

* Revert the yml file change

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

* Move the duplicate api to workspace plugin

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

* Modify change log

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

* Modify api doc

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

* Check target workspace exists or not

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

* Remove unused import

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

* Fix test failure

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

* Modify change log

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

* Modify workspace doc

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

* Add more unit tests

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

* Some minor change

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

* Fix test failure

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

* Modify test description

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

* Optimize test description

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

* Modify test case

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

* Minor change

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

---------

Signed-off-by: gaobinlong <[email protected]>
(cherry picked from commit 7eda01a)
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>
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.

[Workspace] Allow copy/duplicate saved objects among workspaces
8 participants