Skip to content

Storage: Fix Bucket logging configuration using library#6327

Merged
frankyn merged 3 commits intogoogleapis:masterfrom
pmakani:api-storage-2911
Oct 14, 2019
Merged

Storage: Fix Bucket logging configuration using library#6327
frankyn merged 3 commits intogoogleapis:masterfrom
pmakani:api-storage-2911

Conversation

@pmakani
Copy link

@pmakani pmakani commented Sep 24, 2019

Fixes #2911

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 24, 2019
@codecov
Copy link

codecov bot commented Sep 24, 2019 •
edited
Loading

Codecov Report

Merging #6327 into master will decrease coverage by 0.55%.
The diff coverage is 74.41%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6327      +/-   ##
============================================
- Coverage     46.85%    46.3%   -0.56%     
+ Complexity    28120    27924     -196     
============================================
  Files          2601     2613      +12     
  Lines        287258   287669     +411     
  Branches      33359    33739     +380     
============================================
- Hits         134595   133199    -1396     
- Misses       142429   144263    +1834     
+ Partials      10234    10207      -27
Impacted Files Coverage Δ Complexity Δ
...src/main/java/com/google/cloud/storage/Bucket.java 81.22% <0%> (-0.6%) 34 <0> (ø)
...main/java/com/google/cloud/storage/BucketInfo.java 84.36% <78.04%> (-0.5%) 84 <1> (+3)
...ud/bigtable/admin/v2/BigtableTableAdminClient.java 61.33% <0%> (-18.67%) 27% <0%> (ø)
...in/java/com/google/cloud/storage/PolicyHelper.java 95.23% <0%> (-4.77%) 7% <0%> (+1%)
...c/main/java/com/google/cloud/bigquery/Dataset.java 88.7% <0%> (-2.96%) 16% <0%> (ø)
.../main/java/com/google/cloud/compute/v1/UrlMap.java 30.7% <0%> (-1.48%) 22% <0%> (+3%)
...va/com/google/cloud/compute/v1/ForwardingRule.java 35.88% <0%> (-1.25%) 29% <0%> (+1%)
...va/com/google/cloud/compute/v1/BackendService.java 32.24% <0%> (-0.81%) 34% <0%> (+4%)
...va/com/google/cloud/compute/v1/AuditLogConfig.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
.../java/com/google/cloud/compute/v1/PathMatcher.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
... and 40 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 6ba66e6...5a4a98f. Read the comment docs.

@pmakani
Copy link
Author

pmakani commented Sep 25, 2019

@frankyn PTAL.

@pmakani pmakani requested review from yihanzhen and removed request for JesseLovelace September 26, 2019 05:38
@yihanzhen yihanzhen requested review from JesseLovelace and removed request for yihanzhen September 26, 2019 17:56
frankyn
frankyn suggested changes Oct 1, 2019
View reviewed changes
Copy link
Contributor

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

Small nits.

...ud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java Outdated
storage.create(
BucketInfo.newBuilder(logsBucket)
.setLocation("us")
.setRetentionPeriod(RETENTION_PERIOD)
Copy link
Contributor

@frankyn frankyn Oct 1, 2019

Choose a reason for hiding this comment

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

Remove retention period from new bucket.

Copy link
Author

@pmakani pmakani Oct 1, 2019

Choose a reason for hiding this comment

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

Done

...ud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java Outdated
.setLocation("us")
.setRetentionPeriod(RETENTION_PERIOD)
.build()));
Acl acl = Acl.of(User.ofAllAuthenticatedUsers(), Role.WRITER);
Copy link
Contributor

@frankyn frankyn Oct 1, 2019

Choose a reason for hiding this comment

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

Use IAM policy instead.

Copy link
Author

@pmakani pmakani Oct 1, 2019

Choose a reason for hiding this comment

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

Done.

@pmakani pmakani requested a review from frankyn October 10, 2019 09:43
@frankyn frankyn merged commit a7431af into googleapis:master Oct 14, 2019
@chingor13 chingor13 mentioned this pull request Oct 16, 2019
Merged
@pmakani pmakani deleted the api-storage-2911 branch October 17, 2019 05:45
@codyoss codyoss mentioned this pull request Nov 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JesseLovelace JesseLovelace Awaiting requested review from JesseLovelace

1 more reviewer

@frankyn frankyn frankyn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bucket logging configuration using library

3 participants

@pmakani @frankyn @googlebot