8000 [Storage] Add support for UniformBucketLevelAccess by frankyn · Pull Request #5899 · googleapis/google-cloud-java · GitHub
[go: up one dir, main page]
Skip to content

[Storage] Add support for UniformBucketLevelAccess #5899

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.

Sign up for GitHub

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

Jump to bottom
Merged
JesseLovelace merged 16 commits into from
Nov 4, 2019
Merged

[Storage] Add support for UniformBucketLevelAccess #5899

JesseLovelace merged 16 commits into from
Nov 4, 2019

Conversation

@frankyn
8000 Copy link
Contributor
@frankyn frankyn commented Jul 26, 2019 •
edited
Loading
  • Design implementation to support UBLA with BPO as deprecated alias methods.
  • Update documentation.
  • Review from GCS eng team(@thanhld94).
  • Review from GCS cDPE team (@JesseLovelace).
  • Unit tests.
  • Integration Tests.
  • Update Bucket Policy Only samples to use UBLA methods instead.
  • Sample Tests.
  • Fix pending for backend bug: 134695162. Remove workaround.

@frankyn frankyn added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: storage Issues related to the Cloud Storage API. needs work This is a pull request that needs a little love. labels Jul 26, 2019
@frankyn frankyn requested a review from JesseLovelace July 26, 2019 18:58
thanhld94
thanhld94 reviewed Jul 26, 2019
View reviewed changes
...le-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java Outdated

/**
* Sets whether BucketPolicyOnly is enabled for this bucket. When this is enabled, access to
* Sets whether Uniform Bucket Level Access is enabled for this bucket. When this is enabled, access to
Copy link
@thanhld94 thanhld94 Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC the name is not trade marked, and is a descriptive name. So uniform bucket-level access?

thanhld94
thanhld94 reviewed Jul 26, 2019
View reviewed changes
...le-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java Outdated
* The Bucket's IAM Configuration.
*
* @see <a href="https://cloud.google.com/storage/docs/bucket-policy-only">Bucket Policy Only</a>
* @see <a href="https://cloud.google.com/storage/docs/uniform-bucket-level-access">Uniform Bucket Level Access</a>
Copy link
@thanhld94 thanhld94 Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 26, 2019
thanhld94
thanhld94 approved these changes Jul 27, 2019
View reviewed changes
Copy link
@thanhld94 thanhld94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@codecov
Copy link
codecov bot commented Jul 27, 2019 •
edited
Loading

Codecov Report

Merging #5899 into master will decrease coverage by 0.65%.
The diff coverage is 48%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5899      +/-   ##
============================================
- Coverage     46.35%   45.69%   -0.66%     
+ Complexity    27995    26494    -1501     
============================================
  Files          2613     2613              
  Lines        288066   282531    -5535     
  Branches      33776    32712    -1064     
============================================
- Hits         133524   129109    -4415     
+ Misses       144303   143330     -973     
+ Partials      10239    10092     -147
Impacted Files Coverage Δ Complexity Δ
...main/java/com/google/cloud/storage/BucketInfo.java 83% <48%> (-1.37%) 84 <0> (ø) 8000
.../cloud/datastore/testing/LocalDatastoreHelper.java 80.59% <0%> (-4.48%) 17% <0%> (ø)
...in/java/com/google/cloud/storage/StorageBatch.java 88% <0%> (-4%) 13% <0%> (ø)
...m/google/cloud/vision/v1/ImageAnnotatorClient.java 58.69% <0%> (-3.31%) 15% <0%> (-4%)
...e/cloud/vision/v1p4beta1/ImageAnnotatorClient.java 58.69% <0%> (-3.31%) 15% <0%> (-4%)
...e/cloud/vision/v1p3beta1/ImageAnnotatorClient.java 47.05% <0%> (-2.95%) 9% <0%> (-2%)
...e/cloud/vision/v1p2beta1/ImageAnnotatorClient.java 47.05% <0%> (-2.95%) 9% <0%> (-2%)
...d/webrisk/v1beta1/WebRiskServiceV1Beta1Client.java 63.41% <0%> (-2.5%) 12% <0%> (-3%)
...ogle/cloud/texttospeech/v1/TextToSpeechClient.java 55.88% <0%> (-2.46%) 9% <0%> (-2%)
...cloud/texttospeech/v1beta1/TextToSpeechClient.java 55.88% <0%> (-2.46%) 9% <0%> (-2%)
... and 664 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43ca154...eecf8f2. Read the comment docs.

@frankyn frankyn added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2019
@frankyn frankyn added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 29, 2019
@frankyn frankyn changed the title [WIP] [Storage] Add support for UniformBucketLevelAccess [Storage] Add support for UniformBucketLevelAccess Jul 29, 2019
JesseLovelace
JesseLovelace suggested changes Oct 7, 2019
View reviewed changes
...le-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java Outdated
* This field can then be disabled until the time specified, after which it will become
* immutable and calls to change it will fail. If this is enabled, calls to access legacy ACL
* information will fail.
* Sets whether uiform bucket-level access is enabled for this bucket. When this is enabled,
Copy link
Contributor
@JesseLovelace JesseLovelace Oct 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uiform->uniform

This was referenced Oct 16, 2019
Closed
Closed
@frankyn
Copy link
Contributor Author
frankyn commented Oct 30, 2019

@JesseLovelace is moving this to completion. Clarifying expectations for PR.

@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 31, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 31, 2019
@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 31, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 31, 2019
@JesseLovelace JesseLovelace merged commit 85244a8 into master Nov 4, 2019
@JesseLovelace JesseLovelace deleted the ubla branch November 4, 2019 19:13
This was referenced Nov 6, 2019
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@thanhld94 thanhld94 thanhld94 approved these changes

@JesseLovelace JesseLovelace JesseLovelace approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JesseLovelace JesseLovelace

Labels

api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing. needs work This is a pull request that needs a little love. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@frankyn @thanhld94 @JesseLovelace @googlebot @yoshi-kokoro
0