feat(storage): setting default checksum #30878
Conversation
shubhangi-google
requested review from
a team and
yoshi-approver
as code owners
shubhangi-google
changed the title
google-cloud-storage/test/google/cloud/storage/lazy/bucket_test.rb
Outdated
Show resolved
Hide resolved
|
FYI: @cpriti-os the KOKORO failures is not related to changes done by us |
There was a problem hiding this comment.
I think we need to modify this documentation for the new default checksumming, make appropriate changes here, to the method summary and examples
There was a problem hiding this comment.
made changes for doc but not sure what example to add for elaborating a default functionality
| @@ -1640,10 +1640,7 @@ def file path, | |||
| # * `crc32c` - Calculate and provide a checksum using the CRC32c hash. | |||
| # * `all` - Calculate and provide checksums for all available verifications. | |||
| # | |||
| # Optional. The default is `nil`. Do not provide if also providing a | |||
| # corresponding `crc32c` or `md5` argument. See | |||
| # [Validation](https://cloud.google.com/storage/docs/hashes-etags) | |||
There was a problem hiding this comment.
updates the doc again
Clarify the default checksum behavior in documentation.
| @@ -1805,6 +1804,8 @@ def create_file file, | |||
| path ||= file.path if file.respond_to? :path | |||
| path ||= file if file.is_a? String | |||
| raise ArgumentError, "must provide path" if path.nil? | |||
| # setting crc32c as default checksum algorithm if none provided for integrity check | |||
| checksum = :crc32c if checksum.nil? && crc32c.nil? && md5.nil? | |||
There was a problem hiding this comment.
how can a customer disable checksumming all together, earlier the option was do not send anything and we will consider it nil but now if the customer doesn't send anything or sends nil, we will overwrite with crc32c right? I dont think this is ideal.
Main concern: There's no way to distinguish between customer not sending anything and sending nil explicitly.
There was a problem hiding this comment.
if customer is not sending anything the default will be nil
and here we are setting CRC32c in case of nil
There was a problem hiding this comment.
yes, and that is the problem, we are not providing the user with the option to disable all kinds of checksumming.
| # corresponding `crc32c` or `md5` argument. See | ||
| # [Validation](https://cloud.google.com/storage/docs/hashes-etags) | ||
| # for more information. | ||
| # |
|
closing this PR as its getting corrupted after merging main opening new PR |
4 participants
This change sets Checksum CRC32C as default when we are uploading a file to bucket