-
Notifications
You must be signed in to change notification settings - Fork 87
feat: add UploadPartRequest.crc32c property and requisite plumbing #3395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
10c3ce2 to
4359491
Compare
...-storage/src/main/java/com/google/cloud/storage/multipartupload/model/UploadPartRequest.java
Outdated
Show resolved
Hide resolved
...-storage/src/main/java/com/google/cloud/storage/multipartupload/model/UploadPartRequest.java
Outdated
Show resolved
Hide resolved
...-cloud-storage/src/main/java/com/google/cloud/storage/MultipartUploadHttpRequestManager.java
Show resolved
Hide resolved
...-storage/src/test/java/com/google/cloud/storage/ITMultipartUploadHttpRequestManagerTest.java
Show resolved
Hide resolved
|
I've rephrased the title of this PR to be more clear about what the feature is, as the title will appear as-is in the changelog/release notes. |
| } | ||
|
|
||
| /** | ||
| * Returns the CRC32C checksum of the part to upload. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the encoding related information. It should be clear to the user what encoding it needs to do on the crc32c value.
| } | ||
| } | ||
|
|
||
| private void addChecksumHeader(@Nullable String crc32c, HttpHeaders headers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crc32c won't be null given the 2 flows calling it are sending non-null value. @nullable is not required.
| } | ||
|
|
||
| private void addChecksumHeader(@Nullable String crc32c, HttpHeaders headers) { | ||
| if (crc32c != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check is also redundant
…3395) Co-authored-by: Dhriti Chopra <[email protected]>
| * @since 2.61.0 This new api is in preview and is subject to breaking changes. | ||
| */ | ||
| @BetaApi | ||
| public Builder crc32c(@Nullable String crc32c) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar setup is required in the completeMultipartUpload as well
Adding ability to provide custom crc32c for the Upload Request for MPU.
Refer: 3348