From 08b60e46a75f2f09062d5027835d82095d22c89b Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Tue, 23 Aug 2022 15:12:55 -0400 Subject: [PATCH 1/3] Enables setting the google-project that will be charged for requests that have a "requester pays" component. --- src/main/java/picard/cmdline/CommandLineProgram.java | 10 ++++++++++ src/main/java/picard/nio/GoogleStorageUtils.java | 11 ++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/picard/cmdline/CommandLineProgram.java b/src/main/java/picard/cmdline/CommandLineProgram.java index d37242a610..78e73be579 100644 --- a/src/main/java/picard/cmdline/CommandLineProgram.java +++ b/src/main/java/picard/cmdline/CommandLineProgram.java @@ -134,6 +134,9 @@ public abstract class CommandLineProgram { @Argument(doc="Google Genomics API client_secrets.json file path.", common = true) public String GA4GH_CLIENT_SECRETS="client_secrets.json"; + @Argument(doc="Google project for access to 'requester pays' buckets and objects.", common = true) + public String REQUESTER_PAYS_PROJECT = null; + @ArgumentCollection(doc="Special Arguments that have meaning to the argument parsing system. " + "It is unlikely these will ever need to be accessed by the command line program") public Object specialArgumentsCollection = useLegacyParser() ? @@ -238,6 +241,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) { + System.setProperty("google_project_requester_pays", REQUESTER_PAYS_PROJECT); + Log.getInstance(this.getClass()).info( + String.format("Will use google project %s for gcs requests.", REQUESTER_PAYS_PROJECT)); + } + SamReaderFactory.setDefaultValidationStringency(VALIDATION_STRINGENCY); // Set the compression level everywhere we can think of diff --git a/src/main/java/picard/nio/GoogleStorageUtils.java b/src/main/java/picard/nio/GoogleStorageUtils.java index 1e5f7c2d39..af94adee53 100644 --- a/src/main/java/picard/nio/GoogleStorageUtils.java +++ b/src/main/java/picard/nio/GoogleStorageUtils.java @@ -24,7 +24,6 @@ package picard.nio; - import com.google.cloud.storage.StorageOptions; import com.google.cloud.storage.contrib.nio.CloudStorageConfiguration; import com.google.cloud.storage.contrib.nio.CloudStorageFileSystemProvider; @@ -51,18 +50,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"); + + CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(GoogleStorageUtils.getCloudStorageConfiguration(20, google_project)); CloudStorageFileSystemProvider.setStorageOptions(GoogleStorageUtils.setGenerousTimeouts(StorageOptions.newBuilder()).build()); } /** The config we want to use. **/ - private static CloudStorageConfiguration getCloudStorageConfiguration(int maxReopens, String requesterProject) { - CloudStorageConfiguration.Builder builder = CloudStorageConfiguration.builder() + private static CloudStorageConfiguration getCloudStorageConfiguration(final int maxReopens, final String requesterProject) { + final CloudStorageConfiguration.Builder builder = CloudStorageConfiguration.builder() // if the channel errors out, re-open up to this many times .maxChannelReopens(maxReopens); if (!Strings.isNullOrEmpty(requesterProject)) { // enable requester pays and indicate who pays - builder = builder.autoDetectRequesterPays(true).userProject(requesterProject); + builder.autoDetectRequesterPays(true).userProject(requesterProject); } // this causes the gcs filesystem to treat files that end in a / as a directory From d3198915bced82e29b33bfbed56f0bed942252ba Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Tue, 23 Aug 2022 15:29:49 -0400 Subject: [PATCH 2/3] - argument needs to be optional since the default is null.... --- src/main/java/picard/cmdline/CommandLineProgram.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/picard/cmdline/CommandLineProgram.java b/src/main/java/picard/cmdline/CommandLineProgram.java index 78e73be579..33def4537f 100644 --- a/src/main/java/picard/cmdline/CommandLineProgram.java +++ b/src/main/java/picard/cmdline/CommandLineProgram.java @@ -134,7 +134,7 @@ public abstract class CommandLineProgram { @Argument(doc="Google Genomics API client_secrets.json file path.", common = true) public String GA4GH_CLIENT_SECRETS="client_secrets.json"; - @Argument(doc="Google project for access to 'requester pays' buckets and objects.", common = true) + @Argument(doc="Google project for access to 'requester pays' buckets and objects.", common = true, optional = true) public String REQUESTER_PAYS_PROJECT = null; @ArgumentCollection(doc="Special Arguments that have meaning to the argument parsing system. " + From 54be80571c6284196d8865072c7ca168cec81c32 Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Mon, 29 Aug 2022 12:01:02 -0400 Subject: [PATCH 3/3] responding to review --- src/main/java/picard/cmdline/CommandLineProgram.java | 9 ++++++--- src/main/java/picard/nio/GoogleStorageUtils.java | 1 - 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/picard/cmdline/CommandLineProgram.java b/src/main/java/picard/cmdline/CommandLineProgram.java index 33def4537f..9b8f880c80 100644 --- a/src/main/java/picard/cmdline/CommandLineProgram.java +++ b/src/main/java/picard/cmdline/CommandLineProgram.java @@ -242,10 +242,13 @@ public int instanceMain(final String[] argv) { System.setProperty("ga4gh.client_secrets", GA4GH_CLIENT_SECRETS); } - if (System.getProperty("google_project_requester_pays") == null && REQUESTER_PAYS_PROJECT != null) { - System.setProperty("google_project_requester_pays", REQUESTER_PAYS_PROJECT); + if (PathProvider.GCS.isAvailable){ + 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( - String.format("Will use google project %s for gcs requests.", REQUESTER_PAYS_PROJECT)); + String.format("Will use google project %s for gcs requests.", System.getProperty("google_project_requester_pays"))); } SamReaderFactory.setDefaultValidationStringency(VALIDATION_STRINGENCY); diff --git a/src/main/java/picard/nio/GoogleStorageUtils.java b/src/main/java/picard/nio/GoogleStorageUtils.java index af94adee53..353fbcbebf 100644 --- a/src/main/java/picard/nio/GoogleStorageUtils.java +++ b/src/main/java/picard/nio/GoogleStorageUtils.java @@ -49,7 +49,6 @@ class GoogleStorageUtils { public static void initialize() { - // requester pays support is currently not configured final String google_project = System.getProperty("google_project_requester_pays"); CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(GoogleStorageUtils.getCloudStorageConfiguration(20, google_project));