Merge branch 'develop' into WX-1260

broadinstitute · Nov 2, 2023 · 96e7d7d · 96e7d7d
2 parents b1699d5 + e880371
commit 96e7d7d
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 8 deletions.
diff --git a/.../google/batch/src/main/scala/cromwell/backend/google/batch/runnable/RunnableBuilder.scala b/.../google/batch/src/main/scala/cromwell/backend/google/batch/runnable/RunnableBuilder.scala
@@ -4,7 +4,6 @@ import com.google.cloud.batch.v1.Runnable.Container
 import com.google.cloud.batch.v1.{Environment,Runnable, Volume}
 import cromwell.backend.google.batch.models.GcpBatchConfigurationAttributes.GcsTransferConfiguration
 import cromwell.backend.google.batch.models.{BatchParameter, GcpBatchInput, GcpBatchOutput}
-//import cromwell.backend.google.batch.runnable.RunnableLabels._
 import cromwell.core.path.Path
 import mouse.all.anySyntaxMouse
 
@@ -60,8 +59,14 @@ object RunnableBuilder {
     def withVolumes(volumes: List[Volume]): Runnable.Builder = {
       val formattedVolumes = volumes.map { volume =>
         val mountPath = volume.getMountPath
-        val mountOptions = Option(volume.getMountOptionsList).map(_.asScala.toList).getOrElse(List.empty)
-        s"$mountPath:$mountPath:${mountOptions.mkString(",")}"
+
+        val mountOptions = Option(volume.getMountOptionsList)
+          .map(_.asScala)
+          .filter(_.nonEmpty)
+          .map(_.mkString(":", ",", ""))
+          .getOrElse("")
+
+        s"$mountPath:$mountPath$mountOptions"
       }
 
       builder.setContainer(

diff --git a/...gle/batch/src/main/scala/cromwell/backend/google/batch/util/BatchUtilityConversions.scala b/...gle/batch/src/main/scala/cromwell/backend/google/batch/util/BatchUtilityConversions.scala
@@ -8,10 +8,9 @@ import wom.format.MemorySize
 
 trait BatchUtilityConversions {
 
-
   // construct zones string
   def toZonesPath(zones: Vector[String]): String = {
-    "zones/" + zones.mkString(",")
+    zones.map(zone => "zones/" + zone).mkString(" ")
   }
 
   // lowercase text to match gcp label requirements

diff --git a/...gle/batch/src/test/scala/cromwell/backend/google/batch/runnable/RunnableBuilderSpec.scala b/...gle/batch/src/test/scala/cromwell/backend/google/batch/runnable/RunnableBuilderSpec.scala
@@ -76,7 +76,7 @@ class RunnableBuilderSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matc
     runnable.getContainer.getCommandsList.asScala shouldBe expectedCommand
     runnable.getAlwaysRun shouldBe true
     runnable.getLabelsMap shouldBe memoryRetryRunnableExpectedLabels
-    runnable.getContainer.getVolumesList.asScala.toList shouldBe volumes.map(v => s"${v.getMountPath}:${v.getMountPath}:")
+    runnable.getContainer.getVolumesList.asScala.toList shouldBe volumes.map(v => s"${v.getMountPath}:${v.getMountPath}")
   }
 
   it should "return cloud sdk runnable for multiple keys in retry-with-double-memory" in {
@@ -89,6 +89,6 @@ class RunnableBuilderSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matc
     runnable.getContainer.getCommandsList.asScala shouldBe expectedCommand
     runnable.getAlwaysRun shouldBe true
     runnable.getLabelsMap shouldBe memoryRetryRunnableExpectedLabels
-    runnable.getContainer.getVolumesList.asScala.toList shouldBe volumes.map(v => s"${v.getMountPath}:${v.getMountPath}:")
+    runnable.getContainer.getVolumesList.asScala.toList shouldBe volumes.map(v => s"${v.getMountPath}:${v.getMountPath}")
   }
 }
diff --git a/...main/scala/cromwell/backend/google/pipelines/v2beta/api/request/AbortRequestHandler.scala b/...main/scala/cromwell/backend/google/pipelines/v2beta/api/request/AbortRequestHandler.scala
@@ -35,7 +35,7 @@ trait AbortRequestHandler extends LazyLogging { this: RequestHandler =>
   // The Genomics batch endpoint doesn't seem to be able to handle abort requests on V2 operations at the moment
   // For now, don't batch the request and execute it on its own
   def handleRequest(abortQuery: PAPIAbortRequest, batch: BatchRequest, pollingManager: ActorRef)(implicit ec: ExecutionContext): Future[Try[Unit]] = {
-    Future(abortQuery.httpRequest.execute()) map {
+    Future(abortQuery.httpRequest.setThrowExceptionOnExecuteError(false).execute()) map {
       case response if response.isSuccessStatusCode =>
         abortQuery.requester ! PAPIAbortRequestSuccessful(abortQuery.jobId.jobId)
         Success(())