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

signal end-of-chunking #270

Merged
merged 2 commits into from
Feb 10, 2017
Merged

signal end-of-chunking #270

merged 2 commits into from
Feb 10, 2017

Conversation

andyflury
Copy link

@andyflury andyflury commented Jan 13, 2017

When there are no more QueryResults coming in through chunking we should signal this to the consumer in some way. I suggest that we use some sort of "poison pill" for this. I added a special error message (with text "DONE") to the final QueryResult. I'm open to different options if someone has a better idea.

Thanks!

@codecov-io
Copy link

codecov-io commented Jan 13, 2017

Codecov Report

Merging #270 into master will decrease coverage by -0.31%.

@@             Coverage Diff              @@
##             master     #270      +/-   ##
============================================
- Coverage      71.2%   70.89%   -0.31%     
  Complexity      123      123              
============================================
  Files            11       11              
  Lines           698      701       +3     
  Branches         76       76              
============================================
  Hits            497      497              
- Misses          159      162       +3     
  Partials         42       42
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/influxdb/impl/InfluxDBImpl.java 68.15% <ø> (-1.04%) 37 <ø> (ø)

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 a7fa63a...854f101. Read the comment docs.

@andyflury
Copy link
Author

Hello guys, did you get a chance to look at this one? I believe both changes are necessary. Open to suggestions if someone has a better solution. Thanks!

@andyflury andyflury changed the title initial commit chunking improvements Jan 20, 2017
@ghost
Copy link

ghost commented Feb 6, 2017

+1

@majst01
Copy link
Collaborator

majst01 commented Feb 7, 2017

Hi,

I would take the first LinkedBlockingQueue enhancement, but you must add some unit tests to cover the additional code. Your second modification is a different topic and should therefore be discussed in a different PR.

@andyflury andyflury changed the title chunking improvements signal end-of-chunking Feb 9, 2017
@andyflury
Copy link
Author

Not sure why codecoverage fails, I added test cases for the additional lines. Can CodeCov tests be restartet?

@andyflury
Copy link
Author

Great! Thanks Stefan!

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.

3 participants