From 5bc7ac314d1320e0a68c4fae7920993a711a80fe Mon Sep 17 00:00:00 2001 From: dspeck1 Date: Wed, 1 Nov 2023 08:42:19 -0500 Subject: [PATCH 1/2] WX-1340 GCP Batch: Mount with extra colon issue and multiple zones support (#7240) Co-authored-by: Adam Nichols --- .../google/batch/runnable/RunnableBuilder.scala | 11 ++++++++--- .../google/batch/util/BatchUtilityConversions.scala | 3 +-- .../google/batch/runnable/RunnableBuilderSpec.scala | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/runnable/RunnableBuilder.scala b/supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/runnable/RunnableBuilder.scala index c68e3dbdd29..7f35528fad3 100644 --- a/supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/runnable/RunnableBuilder.scala +++ b/supportedBackends/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/supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/util/BatchUtilityConversions.scala b/supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/util/BatchUtilityConversions.scala index 87b4762f5a5..612c95651e9 100644 --- a/supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/util/BatchUtilityConversions.scala +++ b/supportedBackends/google/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/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/runnable/RunnableBuilderSpec.scala b/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/runnable/RunnableBuilderSpec.scala index d0e4eac0b87..04d2c7d43f5 100644 --- a/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/runnable/RunnableBuilderSpec.scala +++ b/supportedBackends/google/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}") } } \ No newline at end of file From e880371efd2f5f85bc4c9ef99b3cec78a3e94447 Mon Sep 17 00:00:00 2001 From: Adam Nichols Date: Wed, 1 Nov 2023 20:40:40 -0400 Subject: [PATCH 2/2] WX-1339 Make `throwExceptionOnExecuteError` false for PAPI aborts (#7245) --- .../pipelines/v2beta/api/request/AbortRequestHandler.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/api/request/AbortRequestHandler.scala b/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/api/request/AbortRequestHandler.scala index 974c8710766..b4d9421b451 100644 --- a/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/api/request/AbortRequestHandler.scala +++ b/supportedBackends/google/pipelines/v2beta/src/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(())