feat: add configurable checksumming for blob uploads and downloads #246
Conversation
google-cla
bot
added
the
cla: yes
| response = upload.transmit_next_chunk(transport, timeout=timeout) | ||
| except resumable_media.DataCorruption: | ||
| # Attempt to delete the corrupted object. | ||
| self.delete() |
There was a problem hiding this comment.
If this fails for any reason, both exceptions will be reported, using the Python exception pattern "During handling of the above exception, another exception occurred"
There was a problem hiding this comment.
What occurs here if a valid previous generation of the blob exists?
There was a problem hiding this comment.
In the default case, if versioning is off, there is no way to recover the older version. Unfortunately this can only be resolved by backend changes to make server-side checksumming possible for resumable uploads.
In the case where versioning is on, the previous generation still exists, but is not live. This is consistent with behavior in nodejs, which I modeled this code after. We could potentially try to roll back and make the previous version live, but we should also modify nodejs and other languages to be consistent across languages. What do you think?
There was a problem hiding this comment.
I think this is more of a documentation issue until there's a better fix in the backend to support checksums better.
I'd prefer consistency here with clear acknowledgment that these cases can occur when using checksumming. It's not done automatically so it doesn't change behavior for uploads.
| :type checksum: str | ||
| :param checksum: | ||
| (Optional) The type of checksum to compute to verify | ||
| the integrity of the object. If the upload is completed in a single |
There was a problem hiding this comment.
This "if the upload is completed in a single request" / "if the upload is too large" is undeniably ugly and hopefully we can implement server-side handling of resumable upload checksums to resolve that soon
andrewsg
changed the title
|
Recommend hold off on merging until google-resumable-media releases to 1.0 (and bump up the dependency version number in setup.py here) |
product-auto-label
bot
added
the
api: storage
| response = upload.transmit_next_chunk(transport, timeout=timeout) | ||
| except resumable_media.DataCorruption: | ||
| # Attempt to delete the corrupted object. | ||
| self.delete() |
There was a problem hiding this comment.
I think this is more of a documentation issue until there's a better fix in the backend to support checksums better.
I'd prefer consistency here with clear acknowledgment that these cases can occur when using checksumming. It's not done automatically so it doesn't change behavior for uploads.
|
googleapis/google-resumable-media-python#142 is the PR for bumping GRMP to 1.0. |
|
Bumped version number. This should be good to merge as soon as final review comes through. Thanks all |
|
@tseaver PTAL soon if you are available, I've been asked to release. Thanks! |
…oogleapis#246) Co-authored-by: Tres Seaver <tseaver@palladion.com> Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
…oogleapis#246) Co-authored-by: Tres Seaver <tseaver@palladion.com> Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
No description provided.