Skip to content

Storage: Fix locationType field is added in bucket #5725

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 5 commits into from
Jul 17, 2019
Merged

Storage: Fix locationType field is added in bucket #5725

JesseLovelace merged 5 commits into from
Jul 17, 2019

Conversation

@abhinav-qlogic
Copy link

@abhinav-qlogic abhinav-qlogic commented Jul 11, 2019

Fixes #5442

Due to LocationType is a read-only field, So Have not added test case specific for dual-region or region locationType.

@abhinav-qlogic abhinav-qlogic requested a review from a team as a code owner July 11, 2019 06:46
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 11, 2019
@pmakani pmakani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 11, 2019
@yoshi-kokoro yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 11, 2019
@codecov
Copy link

codecov bot commented Jul 11, 2019 •
edited
Loading

Codecov Report

Merging #5725 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5725      +/-   ##
============================================
+ Coverage     47.08%   47.09%   +0.01%     
- Complexity    24995    25066      +71     
============================================
  Files          2382     2389       +7     
  Lines        259281   259754     +473     
  Branches      29370    29406      +36     
============================================
+ Hits         122075   122331     +256     
- Misses       128263   128462     +199     
- Partials       8943     8961      +18
Impacted Files Coverage Δ Complexity Δ
...src/main/java/com/google/cloud/storage/Bucket.java 81.81% <100%> (+0.13%) 34 <0> (ø) ⬇️
...main/java/com/google/cloud/storage/BucketInfo.java 84.86% <100%> (+0.26%) 81 <1> (+3) ⬆️
...n/java/com/google/cloud/bigquery/BigQueryImpl.java 66.78% <0%> (-13.22%) 57% <0%> (ø)
...e/cloud/bigquery/testing/RemoteBigQueryHelper.java 61.4% <0%> (-1.1%) 6% <0%> (ø)
.../main/java/com/google/cloud/bigquery/BigQuery.java 77.55% <0%> (-0.9%) 0% <0%> (ø)
.../google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java 5.23% <0%> (-0.87%) 2% <0%> (ø)
...gle/cloud/bigtable/data/v2/BigtableDataClient.java 89.58% <0%> (ø) 32% <0%> (ø) ⬇️
...bigtable/admin/v2/BigtableInstanceAdminClient.java 99.13% <0%> (ø) 44% <0%> (ø) ⬇️
...ud/bigtable/admin/v2/BigtableTableAdminClient.java 80% <0%> (ø) 27% <0%> (ø) ⬇️
...main/java/com/google/cloud/bigquery/RoutineId.java 76.92% <0%> (ø) 13% <0%> (?)
... and 8 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 ab6a85b...50c2d21. Read the comment docs.

@chingor13 chingor13 changed the title Storage:Fix locationType field is added in bucket Storage: Fix locationType field is added in bucket Jul 11, 2019
frankyn
frankyn suggested changes Jul 11, 2019
View reviewed changes
...le-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java Outdated
}

@Override
public Builder setLocationType(String locationType) {
Copy link
Contributor

@frankyn frankyn Jul 11, 2019

Choose a reason for hiding this comment

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

I think this should be a private setLocationType due to read-only.

google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java Outdated
}

@Override
public Builder setLocationType(String locationType) {
Copy link
Contributor

@frankyn frankyn Jul 11, 2019

Choose a reason for hiding this comment

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

Private here as well.

@JesseLovelace JesseLovelace self-requested a review July 11, 2019 23:36
@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 15, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 15, 2019
damienlog
damienlog approved these changes Jul 17, 2019
View reviewed changes
Copy link

@damienlog damienlog left a comment

Choose a reason for hiding this comment

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

Duplicate of #

@frankyn
Copy link
Contributor

frankyn commented Jul 17, 2019 via email

@JesseLovelace JesseLovelace merged commit ec43a94 into googleapis:master Jul 17, 2019
@JesseLovelace JesseLovelace mentioned this pull request Jul 17, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@frankyn frankyn frankyn approved these changes

@JesseLovelace JesseLovelace JesseLovelace approved these changes

@damienlog damienlog damienlog approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JesseLovelace JesseLovelace

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.

Storage: Support bucket.locationType

7 participants

@abhinav-qlogic @frankyn @JesseLovelace @damienlog @googlebot @pmakani @yoshi-kokoro