Skip to content
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

Replace bespoke parser for significance heuristics #50623

Merged
merged 1 commit into from
Jan 6, 2020

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Jan 3, 2020

This replaces the hand written xcontent parsers for significance
heristics with ObjectParser and parsing named xcontent.

As a happy accident, this was the last user of ParseFieldRegistry so
this PR entirely removes that class.

Closes #25519

This replaces the hand written xcontent parsers for significance
heristics with `ObjectParser` and parsing named xcontent.

As a happy accident, this was the last user of `ParseFieldRegistry` so
this PR entirely removes that class.

Closes elastic#25519
@elasticmachine
Copy link
Collaborator

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

@nik9000
Copy link
Member Author

nik9000 commented Jan 3, 2020

+160 −404 Yay!

@nik9000
Copy link
Member Author

nik9000 commented Jan 3, 2020

I've added breaking-java because his requires plugins that provide significance heuristics to declare them in a different way. It doesn't change how they implement that heuristic though, so it should be a pretty light lift. Changes like this have traditionally been ok in minor releases.

@polyfractal
Copy link
Contributor

@markharwood mind having a look at this? I gave it a skim and it LGTM, but not super familiar with the guts of Sig* aggs myself

Copy link
Contributor

@markharwood markharwood left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Nik

@nik9000
Copy link
Member Author

nik9000 commented Jan 6, 2020

Thanks @markharwood and @polyfractal!

@nik9000 nik9000 merged commit 92503ca into elastic:master Jan 6, 2020
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Jan 6, 2020
This replaces the hand written xcontent parsers for significance
heristics with `ObjectParser` and parsing named xcontent.

As a happy accident, this was the last user of `ParseFieldRegistry` so
this PR entirely removes that class.

Closes elastic#25519
nik9000 added a commit that referenced this pull request Jan 6, 2020
This replaces the hand written xcontent parsers for significance
heristics with `ObjectParser` and parsing named xcontent.

As a happy accident, this was the last user of `ParseFieldRegistry` so
this PR entirely removes that class.

Closes #25519
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
This replaces the hand written xcontent parsers for significance
heristics with `ObjectParser` and parsing named xcontent.

As a happy accident, this was the last user of `ParseFieldRegistry` so
this PR entirely removes that class.

Closes elastic#25519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Construct the significant terms aggregation builder's parser statically
5 participants