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
Support MDS on ListDetectors page #716
Support MDS on ListDetectors page #716
Changes from all commits
d3de573
8258d5c
c21d6c7
e0333ec
8a94a9b
fdc8636
44723e7
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.
it isn't scalable or consistent to add this ID as part of this particular interface that's used just for the
getDetectors()
server-side call. We'll need it for all server-side fns, including all of the AD ones, alerting ones, and general cluster ones. Can you explore a common pattern that could be added to all routes, for example, maybe appending or prepending the data source ID in the path itself and parsing out similar to howdetectorID
is done for some of them is a possibility. All plugins will have the same problem of how to propagate the data source ID from public -> server-side, is there a common pattern being done by others?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'm adding the dataSourceId as port of this particular interface just for this list page. Because for list page, it uses a specific interface for its query params. Since dataSourceId also serves as a query param on this page, so I'm just adding it to this interface. For other pages, I'm appending the data source id the path itself. Please check the pr for detailed page - https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/pull/719/files#diff-cc594889ac44cb130233969adf3da57a18f360a2a77c57ee273aad5f39d6d733
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.
Understood, and cool, looks like some are adjusted that way there. Let's make all of them consistent through the node path the first time though. This will make it much cleaner and avoid making changes to this data model which doesn't logically fit.