ESQL: Run tests with allow_partial_results set to false
#129271
Conversation
This switches spec-based integration and YAML tests to run with `"allow_partial_results": false` to conteract the default, set to `true`.
bpintea
added
>test
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
elasticsearchmachine
added
the
Team:Analytics
| var apiCallSection = doSection.getApiCallSection(); | ||
| if (apiCallSection.getApi().equals("esql.query")) { | ||
| if (apiCallSection.getParams().containsKey("allow_partial_results") == false) { | ||
| apiCallSection.addParam("allow_partial_results", "false"); // we want any error to fail the test |
There was a problem hiding this comment.
Should we do this in the other esql yaml subclasses?
I'm a bit worried that this makes our tests unlike production. If we're hacking the yaml files, maybe we should add an assertion that there aren't any partial failures instead of swapping this key.
@dnhatn, you wanted some time for this setting to test, what do you think?
There was a problem hiding this comment.
Should we do this in the other esql yaml subclasses?
I've extended the setting to the async tests too, as they're not really fetching chunks of partial results. Thanks.
I'm a bit worried that this makes our tests unlike production. If we're hacking the yaml files, maybe we should add an assertion that there aren't any partial failures instead of swapping this key.
I've opened #129293 as an alternative to this approach. Some of the KNN tests seem to fail with both approaches, will follow up.
There was a problem hiding this comment.
Thanks for the ping. I will take a look at the new PR.
|
Superseded by #129293 |
4 participants
This switches spec-based and YAML integration tests to run with
"allow_partial_results": falseto conteract the default, set to
true.Closes #129256