[v2] S3 high level checksums #8933
Conversation
aemous
force-pushed
the
s3-high-level-checksums
branch
from
a479b9c
to
0201a02
Compare
There was a problem hiding this comment.
Looks like a solid start.
My main feedback here is that --checksum-algorithm should be supported for copies (ie s3s3). I can definitely see strong use cases for wanting to copy the object checksums between different S3 paths (or even generating different checksums). In order to support this, we're going to need to port this PR for the vended s3transfer package. Let's make this port a separate PR to reduce noise in this one.
We should also prefer f-strings for string formatting in all new code. ie f"var: {my_var}" instead of "var: %s" % myvar.
| } | ||
|
|
||
| CHECKSUM_ALGORITHM = { | ||
| 'name': 'checksum-algorithm', 'choices': ['CRC32', 'SHA256', 'SHA1', 'CRC32C'], |
There was a problem hiding this comment.
non-blocking: We should consider retrieving the choices from the service model instead of hardcoding the values here.
There was a problem hiding this comment.
Thanks for the feedback; could be worth looking into in a future PR.
aemous
force-pushed
the
s3-high-level-checksums
branch
2 times, most recently
from
90dfb1f
to
fcc9b99
Compare
There was a problem hiding this comment.
Playing around with it some more, I'm not sure --checksum-mode ENABLED provides anything useful without the debug flag:
aws s3 cp s3://hssyoo-brawn-test/README.rst ./ --checksum-mode ENABLED
download: s3://hssyoo-brawn-test/README.rst to ./README.rst
Compare this to the low-level get-object:
aws s3api get-object --bucket hssyoo-brawn-test --key README.rst --checksum-mode ENABLED ./README.rst
{
...
"ChecksumCRC32C": "ac1Isw==",
...
}
With the current changes, the --checksum-mode parameter in high-level S3 commands:
- Doesn't do client-side validation of the object
- Doesn't output the checksum the user could use to validate the object
We should decide what behavior we want to support.
EDIT: Sorry, validation is automatically supported with botocore. Ignore.
aemous
force-pushed
the
s3-high-level-checksums
branch
from
fcc9b99
to
82286ca
Compare
Issue #, if available:
Low-level
aws s3apicommands support checksums other than MD5 for verifying end-to-end data integrity. This pull request ports over similar functionality to high-levelaws s3commands.Description of changes:
--checksum-algorithmflag for file uploads usingaws s3 cp,aws s3 sync, andaws s3 mv.--checksum-modeflag for file downloads usingaws s3 cp,aws s3 sync, andaws s3 mv.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.