Provide sha256 checksum with file updates#1353
Provide sha256 checksum with file updates#1353eullerborges wants to merge 1 commit intos3tools:masterfrom
Conversation
rampageservices
left a comment
rampageservices
left a comment
There was a problem hiding this comment.
Typo needs fixing.
Suggest improvement for readability.
| sha256_hash = checksum_sha256_file(stream, offset, size_total) | ||
| request.body = sha256_hash | ||
|
|
||
| # Provide the checksum with teh request. This is important for buckets that have |
There was a problem hiding this comment.
"the" is spelled incorrectly here, shown as "teh". Minor typo.
|
|
||
| # Provide the checksum with teh request. This is important for buckets that have | ||
| # Object Lock enabled. | ||
| headers['x-amz-checksum-sha256'] = base64.b64encode(sha256_hash.digest()).decode() |
There was a problem hiding this comment.
base64.b64encode(sha256_hash.digest()).decode()
Can this line be broken down? It is doing a lot all at once. It is hard for readability.
Perhaps we canpull out the digest operation into a separate sha256_hash_digest variable?
Or maybe this functionality can be separated into a new function in S3/Crypto.py
I know this exact line is recommended online in tutorials but is in my opinion hard to read.
See: https://debugpointer.com/python/sha256-to-base64-python
|
I think we can close this PR as #1393 has completed this reviews. |
|
Closing as suggested, to be replaced by #1393 |
|
Sorry for not coming back to this, I've been away from GitHub for a while. I'm glad this was picked up and merged in. Thank you! |
4 participants
Fixes #1177. I tried
Content-MD5as well, but that was more involved.