feat(storage): add support for RPO (turbo replication). #14407
Conversation
29e8c73 to
5289d8e
Compare
0252267 to
6114bbd
Compare
google-cloud-storage/test/google/cloud/storage/bucket_rpo_test.rb
Outdated
Show resolved
Hide resolved
|
LGTM except for nit about copyright year. |
6114bbd to
f9c833b
Compare
|
Hi @ddelgrosso1 , this PR is ready to be merged now. Please let me know when I can merge it. |
Done. |
There was a problem hiding this comment.
The getter above can return nil. Is it possible for this setter to set it to nil?
There was a problem hiding this comment.
We cannot set it to nil , we get the following error for any value other than DEFAULT or ASYNC_TURBO
Google::Cloud::InvalidArgumentError (invalid: Invalid value for: is not a valid value)
There was a problem hiding this comment.
Probably because the code says @gapi.rpo = new_rpo.to_s which would attempt to set it to the empty string if you pass nil for new_rpo. What if you changed it to @gapi.rpo = new_rpo&.to_s to preserve nils?
f9c833b to
9ff8744
Compare
There was a problem hiding this comment.
s/metdata/metadata/
Or maybe "attribute" is a better term?
There was a problem hiding this comment.
Probably because the code says @gapi.rpo = new_rpo.to_s which would attempt to set it to the empty string if you pass nil for new_rpo. What if you changed it to @gapi.rpo = new_rpo&.to_s to preserve nils?
9ff8744 to
df3d105
Compare
|
The acceptance test case is failing for a different reason which is being tracked with issue #16365 |
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:
bundle exec rake ciin the gem subdirectory.closes: #14143