-
Notifications
You must be signed in to change notification settings - Fork 966
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
feat: Add MDS support for index pattern validate script #9371
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: tygao <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9371 +/- ##
==========================================
+ Coverage 61.68% 61.70% +0.02%
==========================================
Files 3816 3816
Lines 91829 91830 +1
Branches 14543 14544 +1
==========================================
+ Hits 56649 56668 +19
+ Misses 31521 31506 -15
+ Partials 3659 3656 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks for improving on MDS integration with core features. Could you add more details to the PR to explain what the script feature does, what's the issue, and what benefit does MDS integration bring us? Including a video or gif might help others to better understand! @raintygao
const { dataSourceId } = request.query; | ||
const client = | ||
dataSourceId && context.dataSource | ||
? await context.dataSource.opensearch.getClient(dataSourceId) |
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.
Do we need to handle the error during getClient
?
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 think there may be no need because if the remote client can't be got, it should throw error as expected.
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 may need to wrap the getClient or the http request may give statusCode of 500.
@@ -799,6 +801,7 @@ export class FieldEditor extends PureComponent<FieldEdiorProps, FieldEditorState | |||
script: field.script as string, | |||
indexPatternTitle: indexPattern.title, | |||
http: this.context.services.http, | |||
dataSourceId: this.state.dataSourceId, |
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.
Should we use props.indexPattern.dataSourceRef?.id
? I do not think a dataSourceId state is necessary for this component.
Description
feat: Add MDS support for index pattern validate script
Scripted field is a feature of index pattern that used in visualizations and display in documents. Validation is an API used for validation script when creating scripts.
Before this PR, this API did not support MDS, so it would throw an error when operating the index pattern on a remote data source. After this PR, this error would be fixed.
Screenshot
No UI change
Testing the changes
Turn on/off the MDS, and then visit index pattern script field edit page, the API won't break down any more if editing an index pattern on remote data source.
Changelog
Check List
yarn test:jest
yarn test:jest_integration