Support all seek whence values#102
Conversation
|
@kyleknap any chance you could take a look at this? It would be really nice to be able to use this library from python3. |
|
Ping. @kyleknap is there anything I can do to facilitate getting this merged? |
|
It would be really great to have this merged. Any update? |
|
ping |
|
It's been over a year and this makes s3transfer pretty unreliable with python3. What will it take to get this merged? I've been using this patch in our projects for @endlessm and have not seen any issues with it. Would you prefer a patch to make s3transfer fail when imported on python3? |
|
Any plans on merging this? |
|
Ran into this seek issue today, looks like the original fix was put up in a PR a couple of years ago, and this PR from January accounts for the review comments originally made... pinging some of the recent maintainers to see if anything else needs to happen before it can be merged: @jamesls @kyleknap @JordonPhillips |
nateprewitt
force-pushed
the
fix-zero-len-files
branch
from
5838de9 to
d7529a6
Compare
Bug: aws/aws-cli#2403 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1696800 As described in the bug reports referenced above, trying to copy a zero-length file to S3 fails (with some version combinations of python3, awscli, s3transfer, and requests) like this: $ aws s3 cp emptyfile s3://somebucket upload failed: ./emptyfile to s3://somebucket/emptyfile seek() takes 2 positional arguments but 3 were given ... due to the incomplete implementation of seek() -- missing the optional 'whence' argument -- in s3transfer's ReadFileChunk method. This patch adds the whence argument to all of s3transfer's seek implementations.
Check that seek from current position (whence = 1) or seek from end (whence = 2) work properly with both ReadFileChunk implementations.
nateprewitt
force-pushed
the
fix-zero-len-files
branch
from
d7529a6 to
fe4f945
Compare
|
Alright, I think we're finally ready to ship this! I've added some additional changes onto the original commits from @dbnicholson and @kamalmostafa to prevent some edge case failures. We'd expect this to go out in the next patch version release of s3transfer. Thanks for the feedback and contributions here everyone! |
kyleknap
left a comment
kyleknap
left a comment
There was a problem hiding this comment.
Looks good to me. 🚢 Thanks everyone for helping out with this.
8 participants
This supersedes #88 and takes into account the comments in #88 (review). In particular, it handles the
ReadFileChunkabstraction as suggested by @kyleknap and adds some tests for seeking with non-0 whence values. I squashed myReadFileChunkchanges in @kamalmostafa's patch.