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

Add better handling for empty bucketed aggregation results #70

Merged
merged 1 commit into from
Jun 12, 2018

Conversation

jameslamb
Copy link
Collaborator

Currently, running an aggs query that is valid but returns no results is handled in a kind-of weird way. This PR just addresses it directly, catching the empty case and logging out the fact that it was empty with an INFO level statement.

I don't think this fixes the issue that was reported in #58 (since that turned out to actually be about reverse_nested aggs), but gets us a bit closer.

@codecov-io
Copy link

codecov-io commented Jun 10, 2018

Codecov Report

Merging #70 into master will increase coverage by 0.16%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   85.92%   86.08%   +0.16%     
==========================================
  Files           9        9              
  Lines         604      611       +7     
==========================================
+ Hits          519      526       +7     
  Misses         85       85
Impacted Files Coverage Δ
R/chomp_aggs.R 96.47% <100%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b07ccd9...7031772. Read the comment docs.

@jameslamb jameslamb mentioned this pull request Jun 10, 2018
@@ -102,7 +102,7 @@ cp ${SAMPLE_DATA_FILE} ${TESTDIR}/sample.json
cd ${TESTDIR}

# give the cluster a chance
sleep 10
sleep 15
Copy link
Collaborator

Choose a reason for hiding this comment

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

ha

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

stuff happens, what can I say

Copy link
Collaborator

@austin3dickey austin3dickey left a comment

Choose a reason for hiding this comment

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

LGTM

@jameslamb jameslamb merged commit a11e8da into uptake:master Jun 12, 2018
@jameslamb jameslamb deleted the empty_aggs branch June 23, 2018 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants