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

Fix unexpected data length in aio buffer transport #102

Merged
merged 3 commits into from
Nov 27, 2019
Merged

Conversation

ethe
Copy link
Member

@ethe ethe commented Nov 27, 2019

No description provided.

@ethe ethe mentioned this pull request Nov 27, 2019
@aiudirog
Copy link
Collaborator

It passes my tests with one change: rest_len = len(ret) - sz should be rest_len = sz - len(ret) since sz will be bigger.

However, if you use max for the read length, TAsyncSocket can't use readexactly otherwise we're back to the timeout errors. This means TAsyncSocket has to be wrapped in the buffered transport and couldn't be used as transport on its own because it's read method doesn't guarantee the number of bytes. If that's okay, then this PR should be it.

@ethe
Copy link
Member Author

ethe commented Nov 27, 2019

Yes, we should use read rather than readexactly here.

@aiudirog
Copy link
Collaborator

Yeah, looking at TSocket it does the same thing. Sounds good! I'll close the other PR.

@codecov
Copy link

codecov bot commented Nov 27, 2019

Codecov Report

Merging #102 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
+ Coverage   80.87%   80.91%   +0.03%     
==========================================
  Files          35       35              
  Lines        3529     3536       +7     
==========================================
+ Hits         2854     2861       +7     
  Misses        675      675
Impacted Files Coverage Δ
thriftpy2/contrib/aio/transport/buffered.py 94% <100%> (+0.38%) ⬆️
thriftpy2/transport/buffered/__init__.py 95.12% <100%> (+0.52%) ⬆️

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 931930e...40e01d5. Read the comment docs.

@ethe ethe merged commit 2c41c1e into master Nov 27, 2019
@ethe ethe deleted the bugfix/buffer-trans branch November 10, 2022 11:32
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.

2 participants