cromwell icon indicating copy to clipboard operation
cromwell copied to clipboard

v86 GCPBATCH errors when have multiple zones in the config

Open garlandcrow opened this issue 1 year ago • 2 comments

Thought I would check out the new GCPBATCH support in the latest cromwell. Used the example config for the newly supported GCPBATCH with my projects settings added and it errored because of multiple zones were configured:

default-runtime-attributes {
    ...
    zones: ["us-central1-a", "us-central1-b"]
}

I changed it to just ["us-central1-a"] and worked fine and ran until completion so I guess it is how the parser is combining these zones into whatever underlying batch command needs to be executed. (I also tried the value "us-central1-a us-central1-b" as a string instead of an array cause I thought I read somewhere that was supported, but that didn't work either)

Here is the error thrown by cromwell:

Screenshot 2023-10-04 at 13 11 01

garlandcrow avatar Oct 04 '23 05:10 garlandcrow

It seems this line in the cromwell code is building the zones string like "zones/us-central1-a,uscentral1-b", where the spec according to google https://cloud.google.com/java/docs/reference/google-cloud-batch/latest/com.google.cloud.batch.v1.AllocationPolicy.LocationPolicy.Builder#com_google_cloud_batch_v1_AllocationPolicy_LocationPolicy_Builder_addAllowedLocations_java_lang_String_ is to have "zones/" before each one when it is used here in the code.

So the likely fix should be:

def toZonesPath(zones: Vector[String]): String = {
  zones.map(zone => "zones/" + zone).mkString(",")
}

but I don't have the ability to build and test a cromwell executable.

garlandcrow avatar Oct 04 '23 06:10 garlandcrow

Thanks for providing code recommendation! Submitted PR to update.

dspeck1 avatar Oct 20 '23 18:10 dspeck1

PR merged, closing.

aednichols avatar May 09 '24 14:05 aednichols