Enables setting the google-project that will be charged for requests … #1825
Conversation
|
|
||
| if (System.getProperty("google_project_requester_pays") == null && REQUESTER_PAYS_PROJECT != null) { | ||
| System.setProperty("google_project_requester_pays", REQUESTER_PAYS_PROJECT); | ||
| Log.getInstance(this.getClass()).info( |
There was a problem hiding this comment.
Should this only be output if the google libraries are loaded?
There was a problem hiding this comment.
if the libraries are not loaded, what's the meaning of having "requestor pays"? is there something you'd like it to output in that case?
| @@ -237,6 +240,13 @@ public int instanceMain(final String[] argv) { | |||
| if (System.getProperty("ga4gh.client_secrets") == null) { | |||
| System.setProperty("ga4gh.client_secrets", GA4GH_CLIENT_SECRETS); | |||
| } | |||
|
|
|||
| if (System.getProperty("google_project_requester_pays") == null && REQUESTER_PAYS_PROJECT != null) { | |||
There was a problem hiding this comment.
There's a way to get the default project from the environment I think. I don't know if you want to get that by default?
There was a problem hiding this comment.
does GATK do that?
| @@ -51,18 +50,20 @@ class GoogleStorageUtils { | |||
|
|
|||
| public static void initialize() { | |||
| // requester pays support is currently not configured | |||
| @@ -50,19 +49,20 @@ | |||
| class GoogleStorageUtils { | |||
|
|
|||
| public static void initialize() { | |||
| // requester pays support is currently not configured | |||
| CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(GoogleStorageUtils.getCloudStorageConfiguration(20, null)); | |||
| final String google_project = System.getProperty("google_project_requester_pays"); | |||
There was a problem hiding this comment.
It seems like this is a bit load-order dependent. Have you checked that the commandline argument overrides the setting in the way you want? I think it's also possible to get the default google project so it's not necessary to specify at all. In gatk we instantiate a preferences object and check if it has a value for the project I think, but I'm not sure if it works or has any weirdness.
There was a problem hiding this comment.
how about we get this simple version to work and if people need something else, it can be added later?
There was a problem hiding this comment.
@yfarjoun Looks good I think, although the value looks like it depends on the order things get loaded so I would check that it's working as intended when overriding a system property.
|
@kachulis what's the protocol now? who needs to give this a 👍 so that it can be merged? |
…that have a "requester pays" component.
yfarjoun
force-pushed
the
yf_enable_requester_pays_project
branch
from
cccaf15
to
54be805
Compare
|
@yfarjoun @lbergelson This PR can't be merged as-is because:
|
…that have a "requester pays" component.