From cc96144c22bb10fd197aedc0180e42543994052f Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 21 Jun 2023 15:22:02 -0400 Subject: [PATCH 1/7] first attempt --- .../scala/cromwell/backend/impl/tes/TesTask.scala | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala index f7674c377e7..c639a438b1e 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala @@ -1,5 +1,4 @@ package cromwell.backend.impl.tes - import common.collections.EnhancedCollections._ import common.util.StringUtil._ import cromwell.backend.impl.tes.OutputMode.OutputMode @@ -72,6 +71,11 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, `type` = Option("FILE") ) + // TES accepts a key/value pair in its backend parameters that specifies + // the directory to use for files related to this task. + private val tesTaskPathPrefix = ("internal_path_prefix", + Option(tesPaths.callExecutionRoot.resolve("tes_data").pathAsString)) + private def writeFunctionFiles: Map[FullyQualifiedName, Seq[WomFile]] = instantiatedCommand.createdFiles map { f => f.file.value.md5SumShort -> List(f.file) } toMap @@ -233,7 +237,8 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, val resources: Resources = TesTask.makeResources( runtimeAttributes, - preferedWorkflowExecutionIdentity + preferedWorkflowExecutionIdentity, + Map(tesTaskPathPrefix) ) val executors = Seq(Executor( @@ -254,7 +259,7 @@ object TesTask { configIdentity.map(_.value).orElse(workflowOptionsIdentity.map(_.value)) } def makeResources(runtimeAttributes: TesRuntimeAttributes, - workflowExecutionId: Option[String]): Resources = { + workflowExecutionId: Option[String], additionalBackendParams: Map[String, Option[String]]): Resources = { // This was added in BT-409 to let us pass information to an Azure // TES server about which user identity to run tasks as. @@ -263,7 +268,8 @@ object TesTask { val backendParameters = runtimeAttributes.backendParameters ++ workflowExecutionId .map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(_)) - .toMap + .toMap ++ + additionalBackendParams val disk :: ram :: _ = Seq(runtimeAttributes.disk, runtimeAttributes.memory) map { case Some(x) => Option(x.to(MemoryUnit.GB).amount) From dd5bcc9c16469ea86a5dc8e028ec39db4b5d3c50 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Thu, 22 Jun 2023 14:26:21 -0400 Subject: [PATCH 2/7] some changes and tests --- .../cromwell/backend/impl/tes/TesTask.scala | 4 +- .../backend/impl/tes/TesTaskSpec.scala | 81 ++++++++++++++++--- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala index c639a438b1e..0b92deaf793 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala @@ -73,8 +73,8 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, // TES accepts a key/value pair in its backend parameters that specifies // the directory to use for files related to this task. - private val tesTaskPathPrefix = ("internal_path_prefix", - Option(tesPaths.callExecutionRoot.resolve("tes_data").pathAsString)) + val tesTaskPathPrefix : (String, Option[String]) = ("internal_path_prefix", + Option(tesPaths.callExecutionRoot.resolve("tes_task").pathAsString)) private def writeFunctionFiles: Map[FullyQualifiedName, Seq[WomFile]] = instantiatedCommand.createdFiles map { f => f.file.value.md5SumShort -> List(f.file) } toMap diff --git a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala index e2c743bb718..450985a252c 100644 --- a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala +++ b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala @@ -31,34 +31,40 @@ class TesTaskSpec false, Map.empty ) + val internalPathPrefix = ("internal_path_prefix", Option("mock/path/to/tes/task")) + val additionalBackendParams = Map(internalPathPrefix) it should "create the correct resources when an identity is passed in WorkflowOptions" in { val wei = Option("abc123") - TesTask.makeResources(runtimeAttributes, wei) shouldEqual - Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"))) + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, + Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), + internalPathPrefix)) ) } it should "create the correct resources when an empty identity is passed in WorkflowOptions" in { val wei = Option("") - TesTask.makeResources(runtimeAttributes, wei) shouldEqual - Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(""))) + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, + Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(""), + internalPathPrefix)) ) } it should "create the correct resources when no identity is passed in WorkflowOptions" in { val wei = None - TesTask.makeResources(runtimeAttributes, wei) shouldEqual - Resources(None, None, None, Option(false), None, Option(Map.empty[String, Option[String]]) - ) + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, Option(Map(internalPathPrefix))) } it should "create the correct resources when an identity is passed in via backend config" in { val weic = Option(WorkflowExecutionIdentityConfig("abc123")) val weio = Option(WorkflowExecutionIdentityOption("def456")) val wei = TesTask.getPreferredWorkflowExecutionIdentity(weic, weio) - TesTask.makeResources(runtimeAttributes, wei) shouldEqual - Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"))) + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), + internalPathPrefix)) ) } @@ -66,11 +72,64 @@ class TesTaskSpec val weic = None val weio = Option(WorkflowExecutionIdentityOption("def456")) val wei = TesTask.getPreferredWorkflowExecutionIdentity(weic, weio) - TesTask.makeResources(runtimeAttributes, wei) shouldEqual - Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("def456"))) + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("def456"), + internalPathPrefix)) ) } + it should "correctly set the internal path prefix when provided as a backend parameter" in { + val wei = Option("abc123") + val internalPathPrefix = ("internal_path_prefix", Option("mock/path/to/tes/task")) + val additionalBackendParams = Map(internalPathPrefix) + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, + Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), + internalPathPrefix)) + ) + } + + it should "correctly resolve the path to .../tes_task and add the k/v pair to backend parameters" in { + val jobLogger = mock[JobLogger] + val emptyWorkflowOptions = WorkflowOptions(JsObject(Map.empty[String, JsValue])) + val workflowDescriptor = buildWdlWorkflowDescriptor(TestWorkflows.HelloWorld, + labels = Labels("foo" -> "bar")) + val jobDescriptor = jobDescriptorFromSingleCallWorkflow(workflowDescriptor, + Map.empty, + emptyWorkflowOptions, + Set.empty) + val tesPaths = TesJobPaths(jobDescriptor.key, + jobDescriptor.workflowDescriptor, + TestConfig.emptyConfig) + + val tesTask = TesTask(jobDescriptor, + TestConfig.emptyBackendConfigDescriptor, + jobLogger, + tesPaths, + runtimeAttributes, + DefaultPathBuilder.build("").get, + "", + InstantiatedCommand("command"), + "", + Map.empty, + "", + OutputMode.ROOT) + + //Assert path is created correctly + val expectedKey = "internal_path_prefix" + val expectedValue = Option(tesPaths.callExecutionRoot.resolve("tes_task").pathAsString) + tesTask.tesTaskPathPrefix shouldBe (expectedKey, expectedValue) + + //Assert path correctly ends up in the resources + val additionalBackendParams = Map(expectedKey -> expectedValue) + val wei = Option("abc123") + TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + Resources(None, None, None, Option(false), None, + Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), + expectedKey -> expectedValue)) + ) + } + it should "copy labels to tags" in { val jobLogger = mock[JobLogger] val emptyWorkflowOptions = WorkflowOptions(JsObject(Map.empty[String, JsValue])) From b7e8ba36d67065313123a64f2d91ac54bdb1c4d6 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Fri, 30 Jun 2023 15:04:15 -0400 Subject: [PATCH 3/7] elevate path --- .../tes/TesAsyncBackendJobExecutionActor.scala | 3 ++- .../cromwell/backend/impl/tes/TesJobPaths.scala | 4 ++++ .../cromwell/backend/impl/tes/TesTask.scala | 10 +++------- .../cromwell/backend/impl/tes/TesTaskSpec.scala | 17 +---------------- 4 files changed, 10 insertions(+), 24 deletions(-) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala index 7ff507e63f2..595dd59e18a 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala @@ -162,7 +162,8 @@ class TesAsyncBackendJobExecutionActor(override val standardParams: StandardAsyn mode) }) - tesTask.map(TesTask.makeTask) + val task = tesTask.map(TesTask.makeTask) + return task } def writeScriptFile(): Future[Unit] = { diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala index 0b75cfc0f3a..4fed6b3d5ca 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala @@ -28,6 +28,10 @@ case class TesJobPaths private[tes] (override val workflowPaths: TesWorkflowPath val callInputsDockerRoot = callDockerRoot.resolve("inputs") val callInputsRoot = callRoot.resolve("inputs") + // TES accepts a key/value pair in its backend parameters that specifies + // the directory to use for files related to this task. It expects "internal_path_prefix" to be the key. + val tesTaskRoot = callExecutionRoot.resolve("tes_task") + // Given an output path, return a path localized to the storage file system def storageOutput(path: String): String = { callExecutionRoot.resolve(path).toString diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala index 0b92deaf793..781e14179b7 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala @@ -70,12 +70,6 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, path = tesPaths.callExecutionDockerRoot.resolve("script").toString, `type` = Option("FILE") ) - - // TES accepts a key/value pair in its backend parameters that specifies - // the directory to use for files related to this task. - val tesTaskPathPrefix : (String, Option[String]) = ("internal_path_prefix", - Option(tesPaths.callExecutionRoot.resolve("tes_task").pathAsString)) - private def writeFunctionFiles: Map[FullyQualifiedName, Seq[WomFile]] = instantiatedCommand.createdFiles map { f => f.file.value.md5SumShort -> List(f.file) } toMap @@ -235,10 +229,12 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, workflowExecutionIdentityOption ) + val internalPathPrefix = ("internal_path_prefix", Option(tesPaths.tesTaskRoot.pathAsString)) + val resources: Resources = TesTask.makeResources( runtimeAttributes, preferedWorkflowExecutionIdentity, - Map(tesTaskPathPrefix) + Map(internalPathPrefix) ) val executors = Seq(Executor( diff --git a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala index 450985a252c..08e50e73f9c 100644 --- a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala +++ b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala @@ -90,7 +90,6 @@ class TesTaskSpec } it should "correctly resolve the path to .../tes_task and add the k/v pair to backend parameters" in { - val jobLogger = mock[JobLogger] val emptyWorkflowOptions = WorkflowOptions(JsObject(Map.empty[String, JsValue])) val workflowDescriptor = buildWdlWorkflowDescriptor(TestWorkflows.HelloWorld, labels = Labels("foo" -> "bar")) @@ -102,23 +101,9 @@ class TesTaskSpec jobDescriptor.workflowDescriptor, TestConfig.emptyConfig) - val tesTask = TesTask(jobDescriptor, - TestConfig.emptyBackendConfigDescriptor, - jobLogger, - tesPaths, - runtimeAttributes, - DefaultPathBuilder.build("").get, - "", - InstantiatedCommand("command"), - "", - Map.empty, - "", - OutputMode.ROOT) - //Assert path is created correctly val expectedKey = "internal_path_prefix" - val expectedValue = Option(tesPaths.callExecutionRoot.resolve("tes_task").pathAsString) - tesTask.tesTaskPathPrefix shouldBe (expectedKey, expectedValue) + val expectedValue = Option(tesPaths.tesTaskRoot.pathAsString) //Assert path correctly ends up in the resources val additionalBackendParams = Map(expectedKey -> expectedValue) From 6579ed9605e9d9c47aa0230eae48ff91fe2abf46 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Fri, 30 Jun 2023 15:59:17 -0400 Subject: [PATCH 4/7] stale comment --- .../src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala index 08e50e73f9c..3df66f83d3c 100644 --- a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala +++ b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala @@ -101,7 +101,6 @@ class TesTaskSpec jobDescriptor.workflowDescriptor, TestConfig.emptyConfig) - //Assert path is created correctly val expectedKey = "internal_path_prefix" val expectedValue = Option(tesPaths.tesTaskRoot.pathAsString) From 9fdca8ebaa34e992aaff303ab8f4f649e4759e92 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 1 Aug 2023 15:56:23 -0400 Subject: [PATCH 5/7] tidy --- .../backend/impl/tes/TesAsyncBackendJobExecutionActor.scala | 3 +-- .../main/scala/cromwell/backend/impl/tes/TesJobPaths.scala | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala index 595dd59e18a..7ff507e63f2 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala @@ -162,8 +162,7 @@ class TesAsyncBackendJobExecutionActor(override val standardParams: StandardAsyn mode) }) - val task = tesTask.map(TesTask.makeTask) - return task + tesTask.map(TesTask.makeTask) } def writeScriptFile(): Future[Unit] = { diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala index 4fed6b3d5ca..6c8ff52dcca 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala @@ -28,8 +28,8 @@ case class TesJobPaths private[tes] (override val workflowPaths: TesWorkflowPath val callInputsDockerRoot = callDockerRoot.resolve("inputs") val callInputsRoot = callRoot.resolve("inputs") - // TES accepts a key/value pair in its backend parameters that specifies - // the directory to use for files related to this task. It expects "internal_path_prefix" to be the key. + // This is the root directory that TES will use for files related to this task. + // We must specify it in the backend parameters sent to TES using "internal_path_prefix" as the key. val tesTaskRoot = callExecutionRoot.resolve("tes_task") // Given an output path, return a path localized to the storage file system From a90f2933b361698fc648ebb0b1c9e42ee1aee571 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 8 Aug 2023 15:22:22 -0400 Subject: [PATCH 6/7] PR feedback --- .../backend/impl/tes/TesJobPaths.scala | 11 ++++-- .../cromwell/backend/impl/tes/TesTask.scala | 35 +++++++++--------- .../impl/tes/TesWorkflowOptionKeys.scala | 1 + .../backend/impl/tes/TesTaskSpec.scala | 36 +++++++++---------- 4 files changed, 45 insertions(+), 38 deletions(-) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala index 6c8ff52dcca..f1797d9bc58 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesJobPaths.scala @@ -28,9 +28,14 @@ case class TesJobPaths private[tes] (override val workflowPaths: TesWorkflowPath val callInputsDockerRoot = callDockerRoot.resolve("inputs") val callInputsRoot = callRoot.resolve("inputs") - // This is the root directory that TES will use for files related to this task. - // We must specify it in the backend parameters sent to TES using "internal_path_prefix" as the key. - val tesTaskRoot = callExecutionRoot.resolve("tes_task") + /* + * tesTaskRoot: This is the root directory that TES will use for files related to this task. + * We provide it to TES as a k/v pair where the key is "internal_path_prefix" (specified in TesWorkflowOptionKeys.scala) + * and the value is a blob path. + * This is not a standard TES feature, but rather related to the Azure TES implementation that Terra uses. + * While passing it outside of terra won't do any harm, we could consider making this optional and/or configurable. + */ + val tesTaskRoot : Path = callExecutionRoot.resolve("tes_task") // Given an output path, return a path localized to the storage file system def storageOutput(path: String): String = { diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala index 781e14179b7..08af9a207fe 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala @@ -229,14 +229,6 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, workflowExecutionIdentityOption ) - val internalPathPrefix = ("internal_path_prefix", Option(tesPaths.tesTaskRoot.pathAsString)) - - val resources: Resources = TesTask.makeResources( - runtimeAttributes, - preferedWorkflowExecutionIdentity, - Map(internalPathPrefix) - ) - val executors = Seq(Executor( image = dockerImageUsed, command = Seq(jobShell, commandScript.path), @@ -246,6 +238,12 @@ final case class TesTask(jobDescriptor: BackendJobDescriptor, stdin = None, env = None )) + + val resources: Resources = TesTask.makeResources( + runtimeAttributes, + preferedWorkflowExecutionIdentity, + Option(tesPaths.tesTaskRoot.pathAsString) + ) } object TesTask { @@ -255,17 +253,22 @@ object TesTask { configIdentity.map(_.value).orElse(workflowOptionsIdentity.map(_.value)) } def makeResources(runtimeAttributes: TesRuntimeAttributes, - workflowExecutionId: Option[String], additionalBackendParams: Map[String, Option[String]]): Resources = { - - // This was added in BT-409 to let us pass information to an Azure - // TES server about which user identity to run tasks as. - // Note that we validate the type of WorkflowExecutionIdentity - // in TesInitializationActor. - val backendParameters = runtimeAttributes.backendParameters ++ + workflowExecutionId: Option[String], internalPathPrefix: Option[String]): Resources = { + /* + * workflowExecutionId: This was added in BT-409 to let us pass information to an Azure + * TES server about which user identity to run tasks as. + * Note that we validate the type of WorkflowExecutionIdentity in TesInitializationActor. + * + * internalPathPrefix: Added in WX-1156 to support the azure TES implementation. Specifies + * a working directory that the TES task can use. + */ + val backendParameters : Map[String, Option[String]] = runtimeAttributes.backendParameters ++ workflowExecutionId .map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(_)) .toMap ++ - additionalBackendParams + internalPathPrefix + .map(TesWorkflowOptionKeys.InternalPathPrefix -> Option(_)) + .toMap val disk :: ram :: _ = Seq(runtimeAttributes.disk, runtimeAttributes.memory) map { case Some(x) => Option(x.to(MemoryUnit.GB).amount) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala index 0f116815edf..b935689c737 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala @@ -4,4 +4,5 @@ object TesWorkflowOptionKeys { // Communicates to the TES server which identity the tasks should execute as val WorkflowExecutionIdentity = "workflow_execution_identity" val DataAccessIdentity = "data_access_identity" + val InternalPathPrefix = "internal_path_prefix" } diff --git a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala index 3df66f83d3c..5bfa916086d 100644 --- a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala +++ b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala @@ -31,40 +31,40 @@ class TesTaskSpec false, Map.empty ) - val internalPathPrefix = ("internal_path_prefix", Option("mock/path/to/tes/task")) - val additionalBackendParams = Map(internalPathPrefix) + val internalPathPrefix = Option("mock/path/to/tes/task") + val expectedTuple = "internal_path_prefix" -> internalPathPrefix it should "create the correct resources when an identity is passed in WorkflowOptions" in { val wei = Option("abc123") - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + TesTask.makeResources(runtimeAttributes, wei, internalPathPrefix) shouldEqual Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), - internalPathPrefix)) + expectedTuple)) ) } it should "create the correct resources when an empty identity is passed in WorkflowOptions" in { val wei = Option("") - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + TesTask.makeResources(runtimeAttributes, wei, internalPathPrefix) shouldEqual Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(""), - internalPathPrefix)) + expectedTuple)) ) } it should "create the correct resources when no identity is passed in WorkflowOptions" in { val wei = None - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual - Resources(None, None, None, Option(false), None, Option(Map(internalPathPrefix))) + TesTask.makeResources(runtimeAttributes, wei, internalPathPrefix) shouldEqual + Resources(None, None, None, Option(false), None, Option(Map(expectedTuple))) } it should "create the correct resources when an identity is passed in via backend config" in { val weic = Option(WorkflowExecutionIdentityConfig("abc123")) val weio = Option(WorkflowExecutionIdentityOption("def456")) val wei = TesTask.getPreferredWorkflowExecutionIdentity(weic, weio) - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + TesTask.makeResources(runtimeAttributes, wei, internalPathPrefix) shouldEqual Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), - internalPathPrefix)) + expectedTuple)) ) } @@ -72,21 +72,20 @@ class TesTaskSpec val weic = None val weio = Option(WorkflowExecutionIdentityOption("def456")) val wei = TesTask.getPreferredWorkflowExecutionIdentity(weic, weio) - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + TesTask.makeResources(runtimeAttributes, wei, internalPathPrefix) shouldEqual Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("def456"), - internalPathPrefix)) + expectedTuple)) ) } it should "correctly set the internal path prefix when provided as a backend parameter" in { val wei = Option("abc123") - val internalPathPrefix = ("internal_path_prefix", Option("mock/path/to/tes/task")) - val additionalBackendParams = Map(internalPathPrefix) - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + val internalPathPrefix = Option("mock/path/to/tes/task") + TesTask.makeResources(runtimeAttributes, wei, internalPathPrefix) shouldEqual Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), - internalPathPrefix)) - ) + "internal_path_prefix" -> internalPathPrefix) + )) } it should "correctly resolve the path to .../tes_task and add the k/v pair to backend parameters" in { @@ -105,9 +104,8 @@ class TesTaskSpec val expectedValue = Option(tesPaths.tesTaskRoot.pathAsString) //Assert path correctly ends up in the resources - val additionalBackendParams = Map(expectedKey -> expectedValue) val wei = Option("abc123") - TesTask.makeResources(runtimeAttributes, wei, additionalBackendParams) shouldEqual + TesTask.makeResources(runtimeAttributes, wei, expectedValue) shouldEqual Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123"), expectedKey -> expectedValue)) From 8395db47bc38a9233771b4c7b3c7279c07a526eb Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Mon, 14 Aug 2023 17:02:24 -0400 Subject: [PATCH 7/7] move key --- .../tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala | 3 ++- .../cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala index 08af9a207fe..66f0f508d77 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala @@ -262,12 +262,13 @@ object TesTask { * internalPathPrefix: Added in WX-1156 to support the azure TES implementation. Specifies * a working directory that the TES task can use. */ + val internalPathPrefixKey = "internal_path_prefix" val backendParameters : Map[String, Option[String]] = runtimeAttributes.backendParameters ++ workflowExecutionId .map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(_)) .toMap ++ internalPathPrefix - .map(TesWorkflowOptionKeys.InternalPathPrefix -> Option(_)) + .map(internalPathPrefixKey -> Option(_)) .toMap val disk :: ram :: _ = Seq(runtimeAttributes.disk, runtimeAttributes.memory) map { case Some(x) => diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala index b935689c737..0f116815edf 100644 --- a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala +++ b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesWorkflowOptionKeys.scala @@ -4,5 +4,4 @@ object TesWorkflowOptionKeys { // Communicates to the TES server which identity the tasks should execute as val WorkflowExecutionIdentity = "workflow_execution_identity" val DataAccessIdentity = "data_access_identity" - val InternalPathPrefix = "internal_path_prefix" }