Skip to content

Commit

Permalink
Include optional strings in backend_parameters [BT-551] (broadinstitu…
Browse files Browse the repository at this point in the history 
…te#6673)

* Support differentiation between empty string and null in backend_parameters

* Pass optional string values through in backend_parameters

* Whitespace

* Update supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesRuntimeAttributesSpec.scala

Co-authored-by: Miguel Covarrubias <mcovarr@users.noreply.github.com>

Co-authored-by: Miguel Covarrubias <mcovarr@users.noreply.github.com>
  • Loading branch information
@jgainerdewar @mcovarr
jgainerdewar and mcovarr authored Feb 14, 2022
1 parent bfef756 commit c785a0a
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 11 deletions.
12 changes: 9 additions & 3 deletions supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesRuntimeAttributes.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import eu.timepit.refined.api.Refined
import eu.timepit.refined.numeric.Positive
import wom.RuntimeAttributesKeys
import wom.format.MemorySize
import wom.types.WomStringType
import wom.values._

case class TesRuntimeAttributes(continueOnReturnCode: ContinueOnReturnCode,
Expand All @@ -19,7 +20,7 @@ case class TesRuntimeAttributes(continueOnReturnCode: ContinueOnReturnCode,
memory: Option[MemorySize],
disk: Option[MemorySize],
preemptible: Boolean,
backendParameters: Map[String, String])
backendParameters: Map[String, Option[String]])

object TesRuntimeAttributes {

Expand Down Expand Up @@ -55,12 +56,17 @@ object TesRuntimeAttributes {

def makeBackendParameters(runtimeAttributes: Map[String, WomValue],
keysToExclude: Set[String],
config: TesConfiguration): Map[String, String] = {
config: TesConfiguration): Map[String, Option[String]] = {

if (config.useBackendParameters)
runtimeAttributes
.filterKeys(k => !keysToExclude.contains(k))
.collect { case (key, strValue: WomString) => (key, strValue.value)}
.flatMap( _ match {
case (key, WomString(s)) => Option((key, Option(s)))
case (key, WomOptionalValue(WomStringType, Some(WomString(optS)))) => Option((key, Option(optS)))
case (key, WomOptionalValue(WomStringType, None)) => Option((key, None))
case _ => None
})
else
Map.empty
}
Expand Down
4 changes: 2 additions & 2 deletions supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ object TesTask {
.workflowOptions
.get(TesWorkflowOptionKeys.WorkflowExecutionIdentity)
.toOption
.map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> _)
.map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option(_))
.toMap

val disk :: ram :: _ = Seq(runtimeAttributes.disk, runtimeAttributes.memory) map {
Expand Down Expand Up @@ -298,7 +298,7 @@ final case class Resources(cpu_cores: Option[Int],
disk_gb: Option[Double],
preemptible: Option[Boolean],
zones: Option[Seq[String]],
backend_parameters: Option[Map[String, String]])
backend_parameters: Option[Map[String, Option[String]]])

final case class OutputFileLog(url: String,
path: String,
Expand Down
19 changes: 16 additions & 3 deletions ...ortedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesRuntimeAttributesSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class TesRuntimeAttributesSpec extends AnyWordSpecLike with CromwellTimeoutSpec
val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "preemptible" -> WomString("yes"))
assertFailure(runtimeAttributes, "Expecting preemptible runtime attribute to be a Boolean or a String with values of 'true' or 'false'")
}

"validate a valid continueOnReturnCode entry" in {
val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "continueOnReturnCode" -> WomInteger(1))
val expectedRuntimeAttributes = expectedDefaultsPlusUbuntuDocker.copy(continueOnReturnCode = ContinueOnReturnCodeSet(Set(1)))
Expand Down Expand Up @@ -160,13 +160,26 @@ class TesRuntimeAttributesSpec extends AnyWordSpecLike with CromwellTimeoutSpec

"turn unknown string attributes into backend parameters" in {
val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "foo" -> WomString("bar"))
val expectedRuntimeAttributes = expectedDefaults.copy(backendParameters = Map("foo" -> "bar"))
val expectedRuntimeAttributes = expectedDefaults.copy(backendParameters = Map("foo" -> Option("bar")))
assertSuccess(runtimeAttributes, expectedRuntimeAttributes, tesConfig = mockTesConfigWithBackendParams)
}

"exclude unknown non-string attributes from backend parameters" in {
val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "foo" -> WomInteger(5), "bar" -> WomString("baz"))
val expectedRuntimeAttributes = expectedDefaults.copy(backendParameters = Map("bar" -> "baz"))
val expectedRuntimeAttributes = expectedDefaults.copy(backendParameters = Map("bar" -> Option("baz")))
assertSuccess(runtimeAttributes, expectedRuntimeAttributes, tesConfig = mockTesConfigWithBackendParams)
}


"turn populated optional unknown string attributes into backend parameters" in {
val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "foo" -> WomOptionalValue(WomString("bar")))
val expectedRuntimeAttributes = expectedDefaults.copy(backendParameters = Map("foo" -> Option("bar")))
assertSuccess(runtimeAttributes, expectedRuntimeAttributes, tesConfig = mockTesConfigWithBackendParams)
}

"turn unpopulated optional unknown string attributes into backend parameters" in {
val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "foo" -> WomOptionalValue.none(WomStringType))
val expectedRuntimeAttributes = expectedDefaults.copy(backendParameters = Map("foo" -> None))
assertSuccess(runtimeAttributes, expectedRuntimeAttributes, tesConfig = mockTesConfigWithBackendParams)
}
}
Expand Down
6 changes: 3 additions & 3 deletions supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,21 @@ class TesTaskSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers wit
it should "create the correct resources when an identity is passed in WorkflowOptions" in {
val wd = workflowDescriptorWithIdentity(Option("abc123"))
TesTask.makeResources(runtimeAttributes, wd) shouldEqual
Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> "abc123"))
Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("abc123")))
)
}

it should "create the correct resources when an empty identity is passed in WorkflowOptions" in {
val wd = workflowDescriptorWithIdentity(Option(""))
TesTask.makeResources(runtimeAttributes, wd) shouldEqual
Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> ""))
Resources(None, None, None, Option(false), None, Option(Map(TesWorkflowOptionKeys.WorkflowExecutionIdentity -> Option("")))
)
}

it should "create the correct resources when no identity is passed in WorkflowOptions" in {
val wd = workflowDescriptorWithIdentity(None)
TesTask.makeResources(runtimeAttributes, wd) shouldEqual
Resources(None, None, None, Option(false), None, Option(Map.empty[String, String])
Resources(None, None, None, Option(false), None, Option(Map.empty[String, Option[String]])
)
}
}

0 comments on commit c785a0a

Please sign in to comment.