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

Streaming encoder: keep track of encoded bytes that weren't written. #92

Merged
merged 1 commit into from Jan 25, 2019

Conversation

marshallpierce
Copy link
Owner

They'll be retried on subsequent writes, but this plays poorly with
write_all. See rust-lang/rust#56889.

Hat tip to #90.

They'll be retried on subsequent writes, but this plays poorly with
write_all. See rust-lang/rust#56889.

Hat tip to #90.
@codecov-io
Copy link

codecov-io commented Dec 23, 2018

Codecov Report

Merging #92 into master will decrease coverage by 0.33%.
The diff coverage is 88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   96.52%   96.19%   -0.34%     
==========================================
  Files          12       12              
  Lines        1412     1471      +59     
==========================================
+ Hits         1363     1415      +52     
- Misses         49       56       +7
Impacted Files Coverage Δ
src/write/encoder.rs 88.78% <87.5%> (+0.68%) ⬆️
src/write/encoder_tests.rs 95.66% <88.63%> (-1.43%) ⬇️

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 61c2005...d30c85e. Read the comment docs.

@marshallpierce
Copy link
Owner Author

@dignifiedquire any thoughts on this implementation? Does this address your issue?

@dignifiedquire
Copy link

My current code relies heavily on write_all, so I can't easily test it, but it does look like it achieves what it needs to.

@marshallpierce
Copy link
Owner Author

Would it be feasible in your codebase to have a write_all_safe defined for some WriteExtension trait that doesn't choke on Ok(0) and use that instead? I'm wondering if that's maybe something I could suggest in the docs for how to get past the write_all limitations while the rust core team figures out what a better path forward would be.

@dignifiedquire
Copy link

dignifiedquire commented Jan 7, 2019 via email

@nox
Copy link
Contributor

nox commented Jan 23, 2019

There is something I'm confused about, why is it an issue for write_all to return WriteZero? Shouldn't callers just ignore that kind of error in that case?

@marshallpierce
Copy link
Owner Author

You mean, why does write_all behave the way it does? It doesn't make sense to me either.

@marshallpierce marshallpierce merged commit e2d51f0 into master Jan 25, 2019
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

4 participants