Skip to content

Remove unsupported async_search parameters from rest-api-spec #117626

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

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

pquentin
Copy link
Member

Partly reverts commit 2f8bb0b introduced in #117312.

@javanna pointed me to this comment that I had read but failed to fully take into account:

// for simplicity, we share parsing with ordinary search. That means a couple of unsupported parameters, like scroll
// and pre_filter_shard_size get set to the search request although the REST spec don't list
// them as supported. We rely on SubmitAsyncSearchRequest#validate to fail in case they are set.
// Note that ccs_minimize_roundtrips is also set this way, which is a supported option.

And indeed three parameters raise an exception:

public void testValidateKeepAlive() {
SubmitAsyncSearchRequest req = new SubmitAsyncSearchRequest();
req.setKeepAlive(TimeValue.timeValueMillis(randomIntBetween(1, 999)));
ActionRequestValidationException exc = req.validate();
assertNotNull(exc);
assertThat(exc.validationErrors().size(), equalTo(1));
assertThat(exc.validationErrors().get(0), containsString("[keep_alive]"));
}
public void testValidateSuggestOnly() {
SubmitAsyncSearchRequest req = new SubmitAsyncSearchRequest();
req.getSearchRequest().source(new SearchSourceBuilder().suggest(new SuggestBuilder()));
ActionRequestValidationException exc = req.validate();
assertNotNull(exc);
assertThat(exc.validationErrors().size(), equalTo(1));
assertThat(exc.validationErrors().get(0), containsString("suggest"));
}
public void testValidatePreFilterShardSize() {
SubmitAsyncSearchRequest req = new SubmitAsyncSearchRequest();
req.getSearchRequest().setPreFilterShardSize(randomIntBetween(2, Integer.MAX_VALUE));
ActionRequestValidationException exc = req.validate();
assertNotNull(exc);
assertThat(exc.validationErrors().size(), equalTo(1));
assertThat(exc.validationErrors().get(0), containsString("[pre_filter_shard_size]"));

However, I kept ccs_minimize_roundtrips which is explicitly supported (as stated in the comment too):

public void testValidateCssMinimizeRoundtrips() {
SubmitAsyncSearchRequest req = new SubmitAsyncSearchRequest();
req.getSearchRequest().setCcsMinimizeRoundtrips(true);
ActionRequestValidationException exc = req.validate();
assertNull(exc);
}

@pquentin pquentin added >non-issue auto-backport Automatically create backport pull requests when merged :Search Foundations/Search Catch all for Search Foundations v9.0.0 v8.17.0 v8.18.0 labels Nov 27, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Nov 27, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@pquentin pquentin merged commit e19f2b7 into elastic:main Nov 29, 2024
16 checks passed
@pquentin pquentin deleted the remove-async-query-parameters branch November 29, 2024 13:22
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.17
8.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >non-issue :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.17.0 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants