Remove unsupported group_shard_failures parameter#33208
Merged
javanna merged 2 commits intoelastic:masterfrom Aug 29, 2018
Merged
Remove unsupported group_shard_failures parameter#33208javanna merged 2 commits intoelastic:masterfrom
javanna merged 2 commits intoelastic:masterfrom
Conversation
We have had support for the `group_shard_failures` parameter in our code for a while, since we introduced failures grouping. When we introduced validation of parameters at REST, we seem to have forgotten to expose such parameter. Given that the parameter is effectively not supported for many months now, that no user has complained about that and that grouping is the expected behaviour, this commit removes support for the parameter.
javanna
added
>enhancement
review
:Search/Search
Collaborator
|
Pinging @elastic/es-search-aggs |
Contributor
Author
|
retest this please |
colings86
approved these changes
Aug 29, 2018
Contributor
colings86
left a comment
colings86
left a comment
There was a problem hiding this comment.
LGTM, left a small note but fine to merge
| builder.field("phase", phaseName); | ||
| final boolean group = params.paramAsBoolean("group_shard_failures", true); // we group by default | ||
| builder.field("grouped", group); // notify that it's grouped | ||
| builder.field("grouped", true); // notify that it's grouped |
Contributor
There was a problem hiding this comment.
I assume this is only here for backwards compatibility?
Contributor
Author
There was a problem hiding this comment.
yea I did not want to change the output format at this stage. Also, given that the flag was not exposed, we would always return true anyways even before the change. We should remove this at some point though.
dnhatn
added a commit
that referenced
this pull request
Aug 29, 2018
* master: Painless: Add Bindings (#33042) Update version after client credentials backport Fix forbidden apis on FIPS (#33202) Remote 6.x transport BWC Layer for `_shrink` (#33236) Test fix - Graph HLRC tests needed another field adding to randomisation exception list HLRC: Add ML Get Records API (#33085) [ML] Fix character set finder bug with unencodable charsets (#33234) TESTS: Fix overly long lines (#33240) Test fix - Graph HLRC test was missing field name to be excluded from randomisation logic Remove unsupported group_shard_failures parameter (#33208) Update BucketUtils#suggestShardSideQueueSize signature (#33210) Parse PEM Key files leniantly (#33173) INGEST: Add Pipeline Processor (#32473) Core: Add java time xcontent serializers (#33120) Consider multi release jars when running third party audit (#33206) Update MSI documentation (#31950) HLRC: create base timed request class (#33216) [DOCS] Fixes command page titles HLRC: Move ML protocol classes into client ml package (#33203) Scroll queries asking for rescore are considered invalid (#32918) Painless: Fix Semicolon Regression (#33212) ingest: minor - update test to include dissect (#33211) Switch remaining LLREST usage to new style Requests (#33171) HLREST: add reindex API (#32679)
javanna
added a commit
that referenced
this pull request
Sep 18, 2018
We have had support for the `group_shard_failures` parameter in our code for a while, since we introduced failures grouping. When we introduced validation of parameters at REST, we seem to have forgotten to expose such parameter. Given that the parameter is effectively not supported for many months now, that no user has complained about that and that grouping is the expected behaviour, this commit removes support for the parameter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We have had support for the
group_shard_failuresparameter in our code for a while, since we introduced failures grouping. When we introduced validation of parameters at REST, we seem to have forgotten to expose such parameter. Given that the parameter is effectively not supported for many months now, that no user has complained about that and that grouping is the expected behaviour, this commit removes support for the parameter.Relates to #32598