-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(storage): add Autoclass support #6828
Conversation
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.
Nice start on this Cathy!
@@ -60,6 +60,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) { | |||
Encryption: &BucketEncryption{DefaultKMSKeyName: "key"}, |
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.
Let's make sure we have test coverage for the proto converters as well. Also, you should make an integration test to cover the roundtrip with the service (both create and update).
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.
Yep was meaning to write an integration test... also added 2 tests for the proto converters. @tritone PTAL, thanks!
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.
Thanks for adding these additional tests; didn't realize we were missing these unit tests for proto converter methods. We should consider auditing and filling others in if they are missing. @noahdietz @BrennaEpp
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.
One test comment, otherwise LGTM pending feature release.
Fixes #2707 Requires googleapis/google-cloud-go#6828 Tests pass locally with library feature support
Fixes #2707 Requires googleapis/google-cloud-go#6828 Tests pass locally with library feature support
Add support for Autoclass, which allows for automatic selection of the best storage class based on object access patterns
Fixes #6592