Feat: Add "auto" checksum option and make default - #1383
Conversation
Sorry, something went wrong.
cojenco
left a comment
There was a problem hiding this comment.
LGTM, thanks for adding clear documentation too! Just a question on adding a parametrized checksum "auto" to the system test.
Sorry, something went wrong.
| downloads and None for most uploads. Note that ranged downloads ("start" or | ||
| "end" set) still do not support any checksumming, and some features in | ||
| `transfer_manager.py` still support crc32c only. | ||
|
|
There was a problem hiding this comment.
Thanks for adding this! This will make 3.0 release notes really clear
Sorry, something went wrong.
| if self.checksum == "auto": | ||
| self.checksum = ( | ||
| "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5" | ||
| ) |
There was a problem hiding this comment.
I like how this is handled in the constructor 🎉
Sorry, something went wrong.
| assert upload.upload_url == MULTIPART_URL | ||
| assert upload._headers == {} | ||
| assert upload._checksum_type is None | ||
| assert upload._checksum_type == "crc32c" # converted from "auto" |
There was a problem hiding this comment.
nice 😄
Sorry, something went wrong.
|
|
||
| EMPTY_MD5 = base64.b64encode(hashlib.md5(b"").digest()).decode("utf-8") | ||
| crc32c = _helpers._get_crc32c_object() | ||
| crc32c = google_crc32c.Checksum() |
There was a problem hiding this comment.
For system tests, maybe we can add a parametrized checksum "auto" here?
python-storage/tests/resumable_media/system/requests/test_download.py
Lines 272 to 273 in d17a15d
Can do the same for an upload integration test here as well
python-storage/tests/resumable_media/system/requests/test_upload.py
Lines 234 to 235 in d17a15d
Sorry, something went wrong.
No description provided.