[ES 9.0] Remove body workaround (@elastic/response-ops)
#217216
Conversation
afharo
changed the title
@elastic/response-ops@elastic/response-ops)
afharo
added
release_note:skip
…ody-workaround/elastic/response-ops
|
Pinging @elastic/response-ops (Team:ResponseOps) |
| // @ts-expect-error According to the docs, sort should be a comma-separated list of fields and goes in the querystring. | ||
| // However, this one is using a "body" format? | ||
| body: { sort }, |
There was a problem hiding this comment.
I know there are some APIs that are permissive about putting some query parameters in the body instead, but the spec may be more strict, intentionally or not. @pquentin I know this has come up a couple times recently in the spec; do we have a preference on how the spec handles things that can go in query or body?
There was a problem hiding this comment.
The spec should reflect reality: if both are accepted it means that Elasticsearch has specific code for both, and we should encode it in the spec.
Then it's up to the clients to choose which one to use, usually the body form.
Also, note that in 9.x the documentation for endpoints is generated from the spec, which will avoid those discrepancies.
| query, | ||
| // @ts-expect-error According to the docs, sort should be a comma-separated list of fields and goes in the querystring. | ||
| // However, this one is using a "body" format? | ||
| body: { sort }, |
There was a problem hiding this comment.
Is this saying we're formatting the sort wrong or this function is expecting the sort in the body field?
ymao1
left a comment
ymao1
left a comment
There was a problem hiding this comment.
LGTM. Verified alerts and event log indices are installed and reinstalled successfully for both serverless and server-ful across restarts. Verified task manager update by queries continue to claim tasks as expected.
💚 Build Succeeded
Metrics [docs]
History
cc @afharo |
6 participants
Summary
Follow up to #213375: The latest version of the ES client fixed the issue elastic/elasticsearch-js#2584.
We should be able to remove all usages of
// @ts-expect-error [email protected] https://github.com/elastic/elasticsearch-js/issues/2584.Checklist