Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[MD]Allow create and distinguish index pattern with same name but from different datasources #3604
[MD]Allow create and distinguish index pattern with same name but from different datasources #3604
Changes from 3 commits
085fda9
c402636
f29f3ac
0767a48
8fc0dfc
1f2721c
0d2faa9
02b3521
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Doing an explicit type check within the generalized
saved_object
plugin seems like an antipattern. Ideally, any logic unique to a particular saved object should exist in that saved object definition (index-pattern
, in this case).Luckily, there are better patterns available to do what we're doing here. One option is to provide a custom
title
getter method in the index-pattern saved object definition, if we always want the title to have the appended data source. Alternatively, if we sometimes need just the index pattern title, and other times need the appended version, we can just define a new custom property, such as "displayTitle" and have this logic in the getter for that.As an example, see how the
visualize
plugin provides its own custom finder method as part of the saved object loader: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/visualizations/public/saved_visualizations/saved_visualizations.tsThere 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.
Prepending datasource to title requires fetching datasource name first, which replies on a saved object client to make a call. This should happen where there's a client to use.
saved object management definition
doesn't provide client , passing it tomagement -> getTitlte()
will change the interface that shard by other saved object. And to pass it for index-pattern type only, again need explicit check.index-pattern saved object
can't provide client, because unfortunately unlikesavedVis
that you referred to, there's noSavedIndexPattern extends SavedObjectClass
exists. Don't think it's worth to create one only for prepending datasource to the title for display purpose. Comparing to visualization, index patterns are comparatively simpler objectsBesides, there's an issue under discussion about having an OUI component that can be used to select datasource then index pattern when creating Visualzaiton/Dashboards/Discover. #2841. The logic we have here may need be refactored after that discussion is settled.
Thanks for bringing this up, this might not be a good pattern to use, but it might not be a blocker for this PR. Can we create an issue to track and address in the future improvements? Does it sound good to you? @joshuarrrr
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.
Similar to my other comment above, this logic shouldn't be here, it should be accounted for in the saved object management definition: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data/server/saved_objects/index_patterns.ts#L42-L44