Skip to content
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

fix: use projectId from CloudStorageConfig #429

Merged
merged 1 commit into from Feb 12, 2021

Conversation

frankyn
Copy link
Member

@frankyn frankyn commented Feb 11, 2021

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:

  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)

Fixes #352 ☕️

@frankyn frankyn requested a review from a team as a code owner February 11, 2021 06:07
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 11, 2021
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage-nio API. label Feb 11, 2021
@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #429 (0a02af3) into master (3a3d465) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #429      +/-   ##
============================================
+ Coverage     72.29%   72.57%   +0.27%     
  Complexity      500      500              
============================================
  Files            29       29              
  Lines          1664     1670       +6     
  Branches        277      276       -1     
============================================
+ Hits           1203     1212       +9     
+ Misses          336      335       -1     
+ Partials        125      123       -2     
Impacted Files Coverage Δ Complexity Δ
...le/cloud/storage/contrib/nio/CloudStoragePath.java 78.63% <100.00%> (+2.95%) 52.00 <0.00> (+1.00)
...ud/storage/contrib/nio/testing/FakeStorageRpc.java 70.96% <0.00%> (-0.47%) 47.00% <0.00%> (-1.00%)
...ge/contrib/nio/CloudStorageFileSystemProvider.java 63.23% <0.00%> (ø) 76.00% <0.00%> (ø%)
...storage/contrib/nio/CloudStorageConfiguration.java 88.46% <0.00%> (+2.56%) 14.00% <0.00%> (ø%)

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 3a3d465...0f64570. Read the comment docs.

import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** Unit tests for {@link CloudStorageFileSystemProvider} late initialization. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I don't think this comment is true

Copy link
Member Author

Choose a reason for hiding this comment

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

lol.... copy paste disaster over here!

fileSystem.provider().getProject() == null
? null
: Storage.BlobListOption.userProject(fileSystem.provider().getProject()));
String userProject = fileSystem.config().userProject();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Am I understanding correctly, the bug was reading projectId from the ServiceOptions instead of from the CloudStorageConfig? If so, can we update the commit comment to explain this fact?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, that's much clearer.

@frankyn frankyn changed the title fix: incorrectly used userProject fix: use projectId from CloudStorageConfig Feb 12, 2021
@frankyn frankyn added the automerge Merge the pull request once unit tests and other checks pass. label Feb 12, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit b6ec240 into master Feb 12, 2021
@gcf-merge-on-green gcf-merge-on-green bot deleted the fix-user-project-failure branch February 12, 2021 01:22
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage-nio API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when we try to check if a file is a directory on google buckets
2 participants