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

Buckets can now be serialized outside of an Aggregation #8113

Merged
merged 1 commit into from Oct 17, 2014
Merged

Buckets can now be serialized outside of an Aggregation #8113

merged 1 commit into from Oct 17, 2014

Conversation

colings86
Copy link
Contributor

This change means that buckets can now be serialised to JSON and serialized and deserialized to the transport API outside of the aggregation that contains them. This is a required change for #8110 (Reducers framework) but should make sense on it's own since object should really take care of their own serialization rather than relying on their parent object.

@jpountz
Copy link
Contributor

jpountz commented Oct 16, 2014

This looks good to me overall but I have some questions:

  • do we actually need to register streams for bucket in Reducers - Post processing of aggregation results #8110 (I see ithe BucketStreams.stream method is not used in this PR)?
  • if yes would it make sense to make the aggregation streams and bucket streams (with different method names for aggs and buckets obviously)

@jpountz jpountz removed the review label Oct 16, 2014
@jpountz
Copy link
Contributor

jpountz commented Oct 17, 2014

Just discussed it with @colings86 and his current impl actually makes more sense. :) So LGTM

This change means that buckets can now be serialised to JSON and serialized and deserialized to the transport API outside of the aggregation that contains them.  This is a required change for #8110 (Reducers framework) but should make sense on it's own since object should really take care of their own serialization rather than relying on their parent object.
@colings86 colings86 merged commit 4723c2a into elastic:master Oct 17, 2014
@colings86 colings86 deleted the feature/bucketSerialization branch November 25, 2014 11:02
@clintongormley clintongormley changed the title Aggregations: Buckets can now be serialized outside of an Aggregation Buckets can now be serialized outside of an Aggregation Jun 7, 2015
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.

None yet

3 participants