-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Update elasticsearch client version #97360
Conversation
💔 Build Failed
Failed CI Steps
Metrics [docs]
To update your PR or re-run it, just comment with: |
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.
app services changes LGTM
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.
Kibana app changes look great to me! Code review only (removing ts-expect-error)
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.
Task manager changes LGTM!
@@ -40,7 +40,7 @@ describe('ElasticIndex', () => { | |||
return elasticsearchClientMock.createSuccessTransportRequestPromise({ | |||
[index]: { | |||
aliases: { foo: index }, | |||
mappings: { dynamic: 'strict', properties: { a: 'b' } }, | |||
mappings: { dynamic: 'strict', properties: { a: 'b' } as any }, |
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 fix the mocked result instead of casting it as any
? { a: 'b' }
doesn't seem to fit any mapping definition.
@@ -813,7 +812,7 @@ export const updateAndPickupMappings = ( | |||
.putMapping({ | |||
index, | |||
timeout: DEFAULT_TIMEOUT, | |||
body: mappings, | |||
body: mappings as estypes.TypeMapping, |
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 @elastic/elasticsearch
? The reason it's failing is that GenericProperty
expects all properties to be mandatory.
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 fixing these errors from the last review!
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.
ES UI changes LGTM
closed in favour of #98266 |
Summary
Update
@elastic/elaticsearch
to the nextcanary
version and fix type errors.