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

Requester Pays on NIO #3221

Closed
Horneth opened this issue May 1, 2018 · 13 comments
Closed

Requester Pays on NIO #3221

Horneth opened this issue May 1, 2018 · 13 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@Horneth
Copy link

Horneth commented May 1, 2018

There is an option on BlobSourceOption and BlobWriteOption to specify the billing project to use when accessing a requester pays bucket, but there doesn't seem to be a way to wire that through NIO.
Am I missing it ? And if not is this something that could be considered ?

@Horneth
Copy link
Author

Horneth commented May 15, 2018

A more specific suggestion would be to add the relevant options for userProject: BlobSourceOption, BlobGetOption, BlobWriteOption
to all calls to the Storage object such that the billing project is always explicitly set. Would there be any objection to that ? I'm happy to make a PR.
I'm thinking here, and where necessary in CloudStorageFileSystemProvider.

This is problematic for us because it prevents using of any requester pays enabled buckets through NIO.

@jean-philippe-martin any thoughts ? :)

@jean-philippe-martin
Copy link

That's a good idea, I would welcome a PR yes. Make sure to mention me in it so I can find it easily!

@droazen
Copy link

droazen commented Jun 5, 2018

@Horneth Do you have a PR for this feature ready to go, or could you easily make one?

@JustinBeckwith JustinBeckwith added triage me I really want to be triaged. 🚨 This issue needs some love. labels Jun 8, 2018
@yihanzhen yihanzhen added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. labels Jun 11, 2018
@JustinBeckwith JustinBeckwith removed the 🚨 This issue needs some love. label Jun 11, 2018
@jean-philippe-martin
Copy link

Code is now in, the feature should be available. Let me know how it goes, and mention me in a new bug if you run into any difficulty!

@jean-philippe-martin
Copy link

@hzyi-google, would you mind please closing this issue? I am not able to do it myself.

@yihanzhen
Copy link
Contributor

Sure. Thanks for contributing!

@jean-philippe-martin
Copy link

Thank you, @hzyi-google !

@yihanzhen yihanzhen reopened this Jul 11, 2018
@yihanzhen
Copy link
Contributor

@jean-philippe-martin There are integration tests failing. Can you double check?

@jean-philippe-martin
Copy link

Yes I can look into it @hzyi-google . Do you have something a bit more specific? Do you mean the ITStorageTest test suite?

@yihanzhen
Copy link
Contributor

From my side, testCantCreateWithoutUserProject, testCantReadWithoutUserProject, testCantCopyWithoutUserProject and testFileExistsRequesterPaysNoUserProject are failing (the last one is an error).

@jean-philippe-martin
Copy link

Odd, I synced to head and those all work for me. These tests require GOOGLE_APPLICATION_CREDENTIALS to be set, but if that's it then I'd expect the failure to happen early, in the beforeClass method.

I wonder if it's something special about the test machine configuration, or the chosen project.
Requester-pays buckets require either the billing project to be set, or the project to have the resourcemanager.projects.createBillingAssignment permission. Could it be that this permission is enabled? Then the accesses would pass, causing the test to fail.

Can you share the error for the last test (either here or via email if you prefer)? Perhaps that'll give me a hint about how to reproduce the issue.

@yihanzhen
Copy link
Contributor

@jean-philippe-martin Yes you are right. Our testing project do have the permission to the method in resourcemanager api. And the test with errors is for the same reason. I'll create a separate github issue for this and ignore the tests for now. Thanks for the clarification :)

@jean-philippe-martin
Copy link

@hzyi-google great, I'm glad the mystery is solved!

Hopefully there is some way you can use a project without these permissions, so you can restore the tests on your side. Or split the integration tests into two parts, one with that permission and one without.

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 Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

5 participants