-
Notifications
You must be signed in to change notification settings - Fork 0
Add bucket_selector pipeline aggregation support to QueryBuilder #50
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
Conversation
…ilder This change adds support for Elasticsearch's `bucket_selector` pipeline aggregation to the QueryBuilder DSL. The new aggregation allows filtering buckets based on computed metrics (e.g. retaining only those buckets where a sum or average exceeds a threshold), a capability not previously exposed through the DSL.
e8f27b9 to
469719e
Compare
| attr_reader :buckets_path, :script, :gap_policy | ||
|
|
||
| # @param [String] name The name used by Elasticsearch to identify the aggregation. | ||
| # @param [Hash,String] buckets_path Path(s) to the metrics in parent aggs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after the comma:
| # @param [Hash,String] buckets_path Path(s) to the metrics in parent aggs. | |
| # @param [Hash, String] buckets_path Path(s) to the metrics in parent aggs. |
| self.class.new( | ||
| name, | ||
| buckets_path: buckets_path.is_a?(Hash) ? buckets_path.dup : buckets_path, | ||
| script: script, # Script is immutable-ish, ok to reuse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take care of the linter warning on this line.
|
|
||
| let(:name) { 'only_slow_tests' } | ||
| let(:buckets_path) { { total: 'total_duration_ms' } } | ||
| let(:script) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Please add an empty line between lines 519 and 520
| JayAPI::Elasticsearch::QueryBuilder::Script | ||
| ) | ||
| end | ||
| let(:gap_policy) { nil } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Please add an empty line between lines 524 and 525
|
|
||
| it 'creates the BucketSelector instance with the expected parameters' do | ||
| expect(JayAPI::Elasticsearch::QueryBuilder::Aggregations::BucketSelector) | ||
| .to receive(:new).with( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat this to avoid the RSpec/ExampleLength linter warning:
| .to receive(:new).with( | |
| .to receive(:new).with(name, buckets_path: buckets_path, script: script, gap_policy: gap_policy) |
|
|
||
| it 'creates the BucketSelector instance with the expected parameters' do | ||
| expect(JayAPI::Elasticsearch::QueryBuilder::Aggregations::BucketSelector) | ||
| .to receive(:new).with( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise here, please reflow the code to avoid the linter warning.
Add support for the bucket_selector pipeline aggregation to the QueryBuilder DSL. This allows filtering buckets based on computed metrics.
Example use case: group test executions by test name, compute total duration per test, and keep only tests that exceed a minimum total duration.
DSL usage:
Generated JSON:
Motivation: the previous DSL had no way to express post-aggregation filtering, forcing either client-side filtering or dropping to raw JSON. This feature enables pipeline filtering directly through the DSL.