Skip to content

Conversation

@dimitris-athanasiou
Copy link
Contributor

This commit exposes public getters for the aggregations in
AggregatorFactories.Builder. The reason is that it allows to
parse the aggregation object from elsewhere (e.g. a plugin) and then
be able to get the aggregation builders in order to set them in
a SearchSourceBuilder.

The alternative would have been to expose a setter for the
AggregatorFactories.Builder object. But that would be making
the API a bit trappy.

This commit exposes public getters for the aggregations in
AggregatorFactories.Builder. The reason is that it allows to
parse the aggregation object from elsewhere (e.g. a plugin) and then
be able to get the aggregation builders in order to set them in
a SearchSourceBuilder.

The alternative would have been to expose a setter for the
AggregatorFactories.Builder object. But that would be making
the API a bit trappy.
@dimitris-athanasiou
Copy link
Contributor Author

Pinging @colings86 @jpountz

@jpountz jpountz self-requested a review December 17, 2016 17:43
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

LGTM2

@dimitris-athanasiou dimitris-athanasiou merged commit b58bbb9 into elastic:master Dec 19, 2016
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.

3 participants