-
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
[MDS] remove datasourceEnabled in the request #6961
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6961 +/- ##
=======================================
Coverage 67.43% 67.43%
=======================================
Files 3445 3445
Lines 67818 67816 -2
Branches 11028 11027 -1
=======================================
+ Hits 45732 45733 +1
+ Misses 19420 19417 -3
Partials 2666 2666
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
}: SavedObjectsImportOptions): Promise<SavedObjectsImportResponse> { | ||
let errorAccumulator: SavedObjectsImportError[] = []; | ||
const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); | ||
|
||
const dataSourceEnabled = supportedTypes.includes('data-source'); |
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.
can we get it from config? Not sure if I understand what we try to achieve 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.
typeRegistry is from the config, this is the savedObjectConfig: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/server/saved_objects/saved_objects_service.ts#L372
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 are checking a config not a supported type, right?
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.
yeah, the supported type is from 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.
How is supported type configured? How is it related to data source enabled 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.
yeah, when constructing the saved object client, it will read config from the raw YAML file, and register it, if data source is enabled, it will support data-source type
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.
So the existing meaning of dataSourceEnabled
was that it's a supported type, and now we're just reading it directly?
@yujin-emma Is this still being targeted for 2.16 ? |
Description
#6557
Issues Resolved
#6557
Screenshot
non MDS
MDS
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration