Repository navigation
Conversation
S3 only completes a multipart upload created with a ChecksumAlgorithm
if each part in CompleteMultipartUpload carries its checksum for that
algorithm. pipe_file and put_file built their parts from PartNumber
and ETag only, and S3File only forwarded ChecksumSHA256, so uploads
using s3_additional_kwargs={"ChecksumAlgorithm": ...} failed on AWS
once they went multipart.
Build every completed part through one helper that adds the
Checksum<ALGORITHM> value from the upload_part response. The algorithm
is the one s3fs requested, as s3transfer does, with the
CreateMultipartUpload response as a fallback, so servers that don't
echo it back still get the checksums.
This file contains hidden or 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
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.
Fixes #672
#775 fixed this for
S3Filewith SHA256, butpipe_fileandput_filestill complete their multipart uploads with onlyPartNumberandETagper part. if the upload was created with aChecksumAlgorithm(e.g.s3_additional_kwargs={"ChecksumAlgorithm": "SHA256"}), AWS refuses to complete it without the part checksums, which is the error in #672.S3Filealso only forwardedChecksumSHA256, so CRC32 and friends were dropped there.the change is a small
_completed_parthelper incore.pythat all three paths use now. it takes theChecksumAlgorithms3fs asked for (per-call kwargs overs3_additional_kwargs, same precedence_call_s3uses) and copiesChecksum<ALGORITHM>from theupload_partresponse into the part, which is the same rules3transfer'sUploadPartTaskuses. the algorithm in thecreate_multipart_uploadresponse is only a fallback, so servers that don't echo it still get the checksums, and the SHA256 behaviour from #775 stays the same there. uploads created without an algorithm send exactly what they sent before. botocore adds a CRC32 to parts by default, but that's left out of the complete call unless the upload asked for it.test:
test_multipart_parts_carry_checksumsrunspipe_file,put_fileandopen(..., "wb")against moto with SHA256 and CRC32, records theUploadPartresponses and theCompleteMultipartUploadparams through botocore's event hooks, and checks every completed part carries the checksum its upload returned. each case also runs withChecksumAlgorithmstripped from theCreateMultipartUploadresponse, like a server that doesn't echo it. moto itself completes these uploads without the checksums, so the test checks what we send rather than relying on the server to reject it.not touched here: the
upload_part_copypaths (merge, managed copies, append mode inS3File). there are open PRs onmergeand multipart copies, so I left those alone. also, aChecksumAlgorithmpassed straight toput_file/pipe_fileas a call kwarg only reachescreate_multipart_upload, notupload_part. that's a separate kwargs-forwarding thing, this PR covers thes3_additional_kwargsroute from the issue.