Fix data-corruption issue with s3boto and s3boto3 multipart uploads #504
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
part upload.
Fixes jschneier#364, similar issue to jschneier#160 for s3boto3. Inspired by
vinayinvicible's fix for jschneier#160.
* `At least two spaces before inline comment (E261)` * `Imports are incorrectly sorted.`
Codecov Report
@@ Coverage Diff @@
## master #504 +/- ##
=========================================
+ Coverage 76.1% 76.41% +0.3%
=========================================
Files 11 11
Lines 1578 1590 +12
=========================================
+ Hits 1201 1215 +14
+ Misses 377 375 -2
Continue to review full report at Codecov.
|
This was referenced May 31, 2018
|
Taking a look at this one now. |
This was referenced Jun 2, 2018
|
@jschneier, thanks for looking at this so quickly. You're going to hate me, but these changes introduce a memory leak because the temporary file is never truncated. I'm working on another PR right now to correct the problem. Sorry! |
|
No hate here! Best to get all the bugs out. Seems like a pain to write a test to cover that.
… On Jun 3, 2018, at 7:26 PM, jnm ***@***.***> wrote:
@jschneier <https://github.com/jschneier>, thanks for looking at this so quickly. You're going to hate me, but these changes introduce a memory leak because the temporary file is never truncated. I'm working on another PR right now to correct the problem. Sorry!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#504 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACJB2MQgwqrURAlvgCQVx2hy-efZpraqks5t5HC_gaJpZM4UUKot>.
|
jnm
added a commit
to jnm/django-storages
that referenced
this pull request
Jun 4, 2018
by truncating the buffer after uploading it. Follows the approach of jschneier#169.
|
Thanks. Here's what I've got: #506. This time, I tried creating a 10GB ZIP on the fly and streaming it to S3 before making the PR :) It seems to work well. |
nitely
pushed a commit
to satellogic/django-storages
that referenced
this pull request
Jul 30, 2018
…schneier#504) * fix for files with size more than buffer size fixes jschneier#160 * fix(s3bot3): spool buffer file to end of all uploaded parts after each part upload. Fixes jschneier#364, similar issue to jschneier#160 for s3boto3. Inspired by vinayinvicible's fix for jschneier#160. * Fix style issues flagged by flake8 * `At least two spaces before inline comment (E261)` * `Imports are incorrectly sorted.` * Fix s3boto3 test incompatibility with Python 3.4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a consolidation of #197 and #365 with merge conflicts resolved and
flake8niggles addressed. All credit for the fixes belongs to @vinayinvicible and @tommwatson.Since the data-corruption problem is long-standing and severe, please consider merging these fixes and their included unit tests. If there are any issues with the proposed changes that make you hesitate to merge, please bring them up candidly—I'll work to address them ASAP!
I believe that this PR should:
s3boto3.pyare corrupt. #449Additionally, #450 is related but cannot be closed by this PR because it also addresses #383, which is a separate issue about file reading.