Skip to content

Percentiles aggregation: disallow specifying same percentile values twice #52257

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Feb 26, 2020
Merged

Percentiles aggregation: disallow specifying same percentile values twice #52257

merged 6 commits into from
Feb 26, 2020

Conversation

@hendrikmuhs
Copy link

@hendrikmuhs hendrikmuhs commented Feb 12, 2020

disallows specifying same percentile values twice and throws an exception.

Note: As this is a breaking change, it goes only into 8.0
Related: #51871

@hendrikmuhs hendrikmuhs requested a review from nik9000 February 12, 2020 08:30
@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 12, 2020

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

@hendrikmuhs hendrikmuhs changed the title Percentiles aggregation disallow specifying same percentile values twice Percentiles aggregation: disallow specifying same percentile values twice Feb 12, 2020
@timroes timroes mentioned this pull request Feb 12, 2020
Closed
@hendrikmuhs
Copy link
Author

hendrikmuhs commented Feb 21, 2020

@nik9000 would be nice if you can have a look, I requested you because you also reviewed the original PR. Thanks!

nik9000
nik9000 approved these changes Feb 21, 2020
View reviewed changes
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It has been a while since I last did it, but I remember you used to have to add something to the docs about the breaking change. Is that still a thing?

@hendrikmuhs hendrikmuhs force-pushed the percentile-aggregation-dups branch from 53d6c4e to e2bf597 Compare February 21, 2020 13:23
hendrikmuhs
hendrikmuhs commented Feb 21, 2020
View reviewed changes
@hendrikmuhs hendrikmuhs force-pushed the percentile-aggregation-dups branch from e2bf597 to 9e3ad0e Compare February 25, 2020 10:24
lcawl
lcawl reviewed Feb 25, 2020
View reviewed changes
@hendrikmuhs hendrikmuhs merged commit 854f698 into elastic:master Feb 26, 2020
@hendrikmuhs hendrikmuhs deleted the percentile-aggregation-dups branch February 26, 2020 12:40
@hendrikmuhs hendrikmuhs mentioned this pull request Nov 19, 2020
Merged
hendrikmuhs pushed a commit that referenced this pull request Nov 30, 2020
Deprecate specifying the same percentile multiple times in percentiles (
32bfb4b 
#65252)

Deprecate specifying the same percentile multiple times in percentiles aggregation

Regression of: #52257
Fixes: #65240
@jakelandis jakelandis mentioned this pull request Feb 22, 2021
Closed
@masseyke masseyke mentioned this pull request Aug 9, 2021
Closed
80 tasks
@hkulekci hkulekci mentioned this pull request Feb 17, 2022
Merged
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcawl lcawl lcawl left review comments

@nik9000 nik9000 nik9000 approved these changes

Assignees

No one assigned

Labels

:Analytics/Aggregations Aggregations >breaking v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@hendrikmuhs @elasticmachine @nik9000 @lcawl @jakelandis