-
Notifications
You must be signed in to change notification settings - Fork 884
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] Support workspace in saved objects client in server side. #6365
[Workspace] Support workspace in saved objects client in server side. #6365
Conversation
…-project#293) * feat: POC implementation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert dependency Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: address one TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: address TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <[email protected]> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <[email protected]> * feat: improve code coverage Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <[email protected]> * feat: use unified types Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove null Signed-off-by: SuZhou-Joe <[email protected]> * feat: address comments Signed-off-by: SuZhou-Joe <[email protected]> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * refact: update types declaration Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
@@ -78,6 +87,12 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl | |||
); | |||
this.proxyWorkspaceTrafficToRealHandler(core); | |||
|
|||
core.savedObjects.addClientWrapper( | |||
-2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this -2 do? can we give it more meaningful var name? https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/DEVELOPER_GUIDE.md#avoid-magic-numbersstrings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let me assign it to a const and add some comment on that.
/** | ||
* workspaces the new created objects belong to | ||
*/ | ||
workspaces?: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the intent to remove this field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As there is already a workspaces
in SavedObjectsBaseOptions and we can delete this duplicate one.
// By declaring workspaces as null, | ||
// workspaces won't be appended automatically into the options. | ||
// or workspaces can not be found because workspace object do not have `workspaces` field. | ||
workspaces: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if workspaces param is not needed, why do we set it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We set workspaces as null explicitly to make sure the workspace_id_consumer wrapper won't append workspaces into the query.
if (options?.hasOwnProperty('workspaces')) { | ||
finalWorkspaces = workspaceIdsInUserOptions || []; | ||
} else if (workspaceIdParsedFromRequest) { | ||
finalWorkspaces = [workspaceIdParsedFromRequest]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the difference of workspace inside options.workspaces vs get from request? do they co-exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are cases that they may co-exist. Using data source for example, in 2.14 all the data sources will be created globally(which means no workspaces info will be stored within data source object), and we will set options.workspaces
as null to declare that we want to retrieve all the global data sources instead of the data sources within the request.workspaceId.
Signed-off-by: SuZhou-Joe <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6365 +/- ##
===========================================
- Coverage 55.58% 34.96% -20.63%
===========================================
Files 1199 1909 +710
Lines 24259 36928 +12669
Branches 4087 6775 +2688
===========================================
- Hits 13485 12911 -574
- Misses 10133 23160 +13027
- Partials 641 857 +216
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the context: the first round of the PR review for this change was here ruanyl#293
I have no more concerns now, thanks :)
…opensearch-project#6365) * Support workspace in saved objects client in server side. (opensearch-project#293) * feat: POC implementation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert dependency Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: address one TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: address TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <[email protected]> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <[email protected]> * feat: improve code coverage Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <[email protected]> * feat: use unified types Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove null Signed-off-by: SuZhou-Joe <[email protected]> * feat: address comments Signed-off-by: SuZhou-Joe <[email protected]> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * refact: update types declaration Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * fix: import error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: use consts and add comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: change the priority value Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]>
…objects client in server side (opensearch-project#6365) (#327) * [Workspace] Support workspace in saved objects client in server side. (opensearch-project#6365) * Support workspace in saved objects client in server side. (#293) * feat: POC implementation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert dependency Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: address one TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: address TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <[email protected]> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <[email protected]> * feat: improve code coverage Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <[email protected]> * feat: use unified types Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove null Signed-off-by: SuZhou-Joe <[email protected]> * feat: address comments Signed-off-by: SuZhou-Joe <[email protected]> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * refact: update types declaration Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * fix: import error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: use consts and add comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: change the priority value Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]>
…objects client in server side (opensearch-project#6365) (#327) * [Workspace] Support workspace in saved objects client in server side. (opensearch-project#6365) * Support workspace in saved objects client in server side. (opensearch-project#293) * feat: POC implementation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert dependency Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: address one TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: address TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <[email protected]> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <[email protected]> * feat: improve code coverage Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <[email protected]> * feat: use unified types Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove null Signed-off-by: SuZhou-Joe <[email protected]> * feat: address comments Signed-off-by: SuZhou-Joe <[email protected]> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * refact: update types declaration Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * fix: import error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: use consts and add comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: change the priority value Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]>
…#6365) * Support workspace in saved objects client in server side. (#293) * feat: POC implementation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert dependency Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: address one TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: address TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <[email protected]> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <[email protected]> * feat: improve code coverage Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <[email protected]> * feat: use unified types Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove null Signed-off-by: SuZhou-Joe <[email protected]> * feat: address comments Signed-off-by: SuZhou-Joe <[email protected]> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * refact: update types declaration Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * fix: import error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: use consts and add comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: change the priority value Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> (cherry picked from commit 3b03fa9) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
…#6365) (#6573) * Support workspace in saved objects client in server side. (#293) * feat: POC implementation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert dependency Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: address one TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: address TODO Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <[email protected]> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <[email protected]> * feat: improve code coverage Signed-off-by: SuZhou-Joe <[email protected]> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <[email protected]> * feat: use unified types Signed-off-by: SuZhou-Joe <[email protected]> * feat: update comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove null Signed-off-by: SuZhou-Joe <[email protected]> * feat: address comments Signed-off-by: SuZhou-Joe <[email protected]> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <[email protected]> * refact: update types declaration Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * fix: import error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: use consts and add comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: change the priority value Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> (cherry picked from commit 3b03fa9) 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>
Description
Support workspace in saved objects client in server side to unified the way to append workspace id in request payload.
Issues Resolved
closes #6130
Screenshot
Testing the changes
workspace.enabled
to trueyarn start --no-base-path
to make sure no random base path will be appendedhttp://localhost:5601/w/foo/app/management/opensearch-dashboards/objects
Check List
yarn test:jest
yarn test:jest_integration