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

v86 GCPBATCH errors when have multiple zones in the config #7232

Closed
garlandcrow opened this issue Oct 4, 2023 · 3 comments
Closed

v86 GCPBATCH errors when have multiple zones in the config #7232

garlandcrow opened this issue Oct 4, 2023 · 3 comments

Comments

@garlandcrow
Copy link

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
Copy link
Author

garlandcrow commented Oct 4, 2023

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.

@dspeck1
Copy link
Collaborator

dspeck1 commented Oct 20, 2023

Thanks for providing code recommendation! Submitted PR to update.

@aednichols
Copy link
Collaborator

PR merged, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants