ESQL: Make esql version required in REST requests#107433
ESQL: Make esql version required in REST requests#107433alex-spies merged 28 commits intoelastic:mainfrom
Conversation
This will make the CI blow up and show us what needs updating.
4dc60c1 to
83bb2f6
Compare
This enables using the version from outside the esql project, esp. in integration tests.
This reverts commit 949c652.
In the ESQL quests for upgrading from older versions of Elasticsearch this will pretend to be and old version of the elasticsearch client and not send a `version` in the body. Old clients will instead default to the first version of ESQL. This should allow us to run these tests even when `version` is required. Tests against newer versions of Elasticsearch will then specify a `version`. Co-authored-by: Nik Everett <nik9000@gmail.com>
| // TODO: make this required | ||
| // "https://github.com/elastic/elasticsearch/issues/104890" | ||
| // validationException = addValidationError(invalidVersion("is required"), validationException); | ||
| validationException = addValidationError(invalidVersion("is required"), validationException); |
There was a problem hiding this comment.
I want to revert this before merging and create a separate PR for it, so that the actual - breaking - API change becomes a single, small PR that can be easily reverted.
There was a problem hiding this comment.
Nevermind, discussed with @nik9000 : we can still revert just this in case something goes wrong.
There was a problem hiding this comment.
We're going to keep this in to save the few hours of build time. If something goes wrong we can back out just this line if we have to.
This reverts commit cb1bb4f.
Co-authored-by: Nik Everett <nik9000@gmail.com>
| EsqlSpecTestCase.assertRequestBreakerEmpty(); | ||
| } | ||
|
|
||
| public static Iterable<Object[]> updateEsqlQueryDoSections(Iterable<Object[]> parameters, Function<DoSection, ExecutableSection> modify) |
There was a problem hiding this comment.
Refactored x-pack/plugin/esql/qa/server/single-node/src/yamlRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlClientYamlAsyncIT.java a little bit, but failed moving this into a place to be used by multiple yaml tests from :/
| if (version != null) { | ||
| builder.field("version", version); | ||
| } |
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java
Show resolved
Hide resolved
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java
Show resolved
Hide resolved
| // TODO: make this required | ||
| // "https://github.com/elastic/elasticsearch/issues/104890" | ||
| // validationException = addValidationError(invalidVersion("is required"), validationException); | ||
| validationException = addValidationError(invalidVersion("is required"), validationException); |
There was a problem hiding this comment.
We're going to keep this in to save the few hours of build time. If something goes wrong we can back out just this line if we have to.
/_queryin integration tests.For mixed cluster tests, impersonates a language client with version 8.13, the only exception in which we do not require the version to be sent (based on #107497 by @nik9000 ).
Fix #107403
Main part of #104890