-
Notifications
You must be signed in to change notification settings - Fork 918
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]Optional workspaces params in repository #5949
[Workspace]Optional workspaces params in repository #5949
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5949 +/- ##
==========================================
+ Coverage 67.01% 67.04% +0.02%
==========================================
Files 3310 3311 +1
Lines 63647 63737 +90
Branches 10165 10194 +29
==========================================
+ Hits 42656 42731 +75
- Misses 18522 18526 +4
- Partials 2469 2480 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ed4126a
to
720e5ba
Compare
@kavilla @Flyingliuhub @ashwin-pc , could you please take a look on this PR? This PR introduces many fundamental features for Workspace and are totally controlled behind a |
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.
@SuZhou-Joe would you add the new config to OSD.yaml file
With disable by default and comment out.
Also add a few words to describe the feature flag
Thanks and done for that. |
Signed-off-by: SuZhou-Joe <[email protected]>
We'd like to backport to 2.x until other down stream PRs get merged, thanks for the call out. |
@@ -557,6 +564,7 @@ export class SavedObjectsService | |||
this.logger, | |||
migrationsRetryDelay | |||
), | |||
opensearchDashboardsRawConfig: this.opensearchDashboardsRawConfig as Config, |
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 about just passing a minimum set of config which is only required down to the functions instead of passing the entire config object? Similar to savedObjectsConfig
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.
I'd prefer to keep as it is because:
- If we do some filter here, once we introduce more config to control the mappings dynamically, we need to change two places, filter and the
build_active_mapping
- For now the configService.atPath does not provide functionality to subscribe to multiple paths(even though it accepts string[], it is more like a lodash.get(obj, path)).
.map((object) => { | ||
const { type, id } = object; | ||
/** | ||
* It requires a check when overwriting objects to target workspaces |
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.
Q: how this comment relates to the code next?
); | ||
} | ||
public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { | ||
const createWithWorkspaceConflictCheck = async <T = unknown>( |
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.
Would be nice to leave a comment to explain from high level of why a wrapper is necessary for create
and bulkCreate
...plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts
Outdated
Show resolved
Hide resolved
...core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.ts
Show resolved
Hide resolved
@ruanyl Will raise a following PR to address your comment. |
…ct#5949) * refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe <[email protected]> * fix: some error Signed-off-by: SuZhou-Joe <[email protected]> * feat: fix test error Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless config in test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: add more unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in import_saved_objects.test.ts Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe <[email protected]> * feat: add type Signed-off-by: SuZhou-Joe <[email protected]> * fix: bootstrap type error Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code and add comment Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> * fix: integration test fail Signed-off-by: SuZhou-Joe <[email protected]> * feat: add missing code Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code Signed-off-by: SuZhou-Joe <[email protected]> * Add permissions field to the mapping only if the permission control is enabled Signed-off-by: gaobinlong <[email protected]> * Fix test failure Signed-off-by: gaobinlong <[email protected]> * feat: modify unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: bulk create error Signed-off-by: SuZhou-Joe <[email protected]> * fix: bulk create error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add new config in yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: add new config in yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: update yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: fix unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: do not skip migration when doing integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: change flag variable Signed-off-by: SuZhou-Joe <[email protected]> * feat: add test cases Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Signed-off-by: gaobinlong <[email protected]> Co-authored-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
* feat: cherry-pick pr opensearch-project#5949 Signed-off-by: SuZhou-Joe <[email protected]> * fix: bootstrap error Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]>
* feat: temp save Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
…ct#6012) * feat: temp save Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
…ct#6012) * feat: temp save Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
…ct#6012) (#276) * feat: temp save * fix: unit test * feat: add some comment --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
…ct#6012) (#277) * feat: temp save * fix: unit test * feat: add some comment --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5949-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c6b4c34956d51c9fc60210ae9b0b3477d58eee5f
# Push it to GitHub
git push --set-upstream origin backport/backport-5949-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
…ct#5949) * refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe <[email protected]> * fix: some error Signed-off-by: SuZhou-Joe <[email protected]> * feat: fix test error Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless config in test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: add more unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in import_saved_objects.test.ts Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe <[email protected]> * feat: add type Signed-off-by: SuZhou-Joe <[email protected]> * fix: bootstrap type error Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code and add comment Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> * fix: integration test fail Signed-off-by: SuZhou-Joe <[email protected]> * feat: add missing code Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code Signed-off-by: SuZhou-Joe <[email protected]> * Add permissions field to the mapping only if the permission control is enabled Signed-off-by: gaobinlong <[email protected]> * Fix test failure Signed-off-by: gaobinlong <[email protected]> * feat: modify unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: bulk create error Signed-off-by: SuZhou-Joe <[email protected]> * fix: bulk create error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add new config in yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: add new config in yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: update yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: fix unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: do not skip migration when doing integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: change flag variable Signed-off-by: SuZhou-Joe <[email protected]> * feat: add test cases Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Signed-off-by: gaobinlong <[email protected]> Co-authored-by: gaobinlong <[email protected]> (cherry picked from commit c6b4c34)
…ct#6012) * feat: temp save Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
) (#6387) * [Workspace]Optional workspaces params in repository (#5949) * refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe <[email protected]> * fix: some error Signed-off-by: SuZhou-Joe <[email protected]> * feat: fix test error Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless config in test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add CHANGELOG Signed-off-by: SuZhou-Joe <[email protected]> * feat: add more unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in import_saved_objects.test.ts Signed-off-by: SuZhou-Joe <[email protected]> * feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe <[email protected]> * feat: add type Signed-off-by: SuZhou-Joe <[email protected]> * fix: bootstrap type error Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code and add comment Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> * fix: integration test fail Signed-off-by: SuZhou-Joe <[email protected]> * feat: add missing code Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code Signed-off-by: SuZhou-Joe <[email protected]> * Add permissions field to the mapping only if the permission control is enabled Signed-off-by: gaobinlong <[email protected]> * Fix test failure Signed-off-by: gaobinlong <[email protected]> * feat: modify unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: bulk create error Signed-off-by: SuZhou-Joe <[email protected]> * fix: bulk create error Signed-off-by: SuZhou-Joe <[email protected]> * feat: add new config in yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: add new config in yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: update yml file Signed-off-by: SuZhou-Joe <[email protected]> * feat: fix unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: do not skip migration when doing integration test Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: change flag variable Signed-off-by: SuZhou-Joe <[email protected]> * feat: add test cases Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Signed-off-by: gaobinlong <[email protected]> Co-authored-by: gaobinlong <[email protected]> (cherry picked from commit c6b4c34) * [Workspace]Following pr for #5949 (#6012) * feat: temp save Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: add some comment Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>
Description
As workspace will be a new concept to organize saved objects, this PR add a new field
workspaces
to indicate which workspaces the objects belong to and consume the field in related methods.Issues Resolved
Partial #4944
Screenshot
As all of the changes happens in server side, currently do not have screenshot for this PR.
Testing the changes
As all of the changes happens in server side, can not test from browser, so we can run the integration_test in
src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts
. The test covers the cases:Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr