-
Notifications
You must be signed in to change notification settings - Fork 89
feat: Blob/Object get #1403
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
feat: Blob/Object get #1403
Conversation
BenWhitehead
left a comment
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.
Overall looks pretty good. A few changes about base64 handling need to be addressed, but other than that looks pretty good.
google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
Outdated
Show resolved
Hide resolved
| s -> | ||
| BaseEncoding.base64() | ||
| .encode(Hashing.md5().hashBytes(s.getBytes()).asBytes()))) | ||
| .as( |
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 can be done in a later PR if you feel like it.
This will need to changes from a combine of two arbitraries, to a single arbitrary which yields both crc32c and md5 values.
We want to ensure the value for those fields is generated base on the same bytes in the general case. Later we can investigate if we want to add an edge case where the md5 does not correspond to crc32c of the same input data.
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.
ah I see makes sense. I will look into it.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.