fix: update Object Write handling to be able to specify an explicit crc32c value #1696
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rc32c value Storage#createFrom and Storage#writer allow specifying an explicit crc32c value which should be passed to GCS for validation. Update our uploads to allow these explicitly defined values to be passed in the `finish_write` message to GCS. Update UnifiedOpts.Crc32cValue to represent its value as an int rather than the b64 string from apiary. Internally when the value is necessary for apiary it will be converted to the b64 string representation. Map gRPC Status `DATA_LOSS` to HTTP 400 Bad Request to mirror the behavior of JSON when a checksum validation fails. Add a new Hasher.constant instant which will function to specify a constant value for the cumulative value of a write. Various plumbing tweaks to allow passing this value all the way down and also attempting to "nullSafeConcat" Add new integration tests to try the negative case to ensue we are getting checksum failures when we pass mis-match values and content.
frankyn
approved these changes
Oct 7, 2022
Contributor
frankyn
left a comment
There was a problem hiding this comment.
LGTM, just have one question about error code mapping to HTTP 1.1 errors
| case ABORTED: // ? | ||
| case CANCELLED: // ? | ||
| case UNKNOWN: // ? | ||
| case DATA_LOSS: // ? |
Contributor
There was a problem hiding this comment.
Are the ? TODO's that need to be finished before cutting a relaese?
Collaborator
Author
There was a problem hiding this comment.
Hopefully, they're being tracked in the tracker. However, ABORTED, CANCELLED, UNKNOWN probably fall into the "connection errors" bucket we've got for http.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Storage#createFrom and Storage#writer allow specifying an explicit crc32c value which should be passed to GCS for validation. Update our uploads to allow these explicitly defined values to be passed in the
finish_writemessage to GCS.Update UnifiedOpts.Crc32cValue to represent its value as an int rather than the b64 string from apiary. Internally when the value is necessary for apiary it will be converted to the b64 string representation.
Map gRPC Status
DATA_LOSSto HTTP 400 Bad Request to mirror the behavior of JSON when a checksum validation fails.Add a new Hasher.constant instant which will function to specify a constant value for the cumulative value of a write.
Various plumbing tweaks to allow passing this value all the way down and also attempting to "nullSafeConcat"
Add new integration tests to try the negative case to ensue we are getting checksum failures when we pass mismatch values and content.