diff --git a/backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala b/backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala index 1b183a67646..bbb796220c4 100644 --- a/backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala +++ b/backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala @@ -748,10 +748,10 @@ trait StandardAsyncExecutionActor RuntimeAttributesValidation.extract(FailOnStderrValidation.instance, validatedRuntimeAttributes) /** - * Returns the behavior for continuing on the return code, obtained by converting `returnCodeContents` to an Int. - * - * @return the behavior for continuing on the return code. - */ + * Returns the behavior for continuing on the return code, obtained by converting `returnCodeContents` to an Int. + * + * @return the behavior for continuing on the return code. + */ lazy val continueOnReturnCode: ContinueOnReturnCode = RuntimeAttributesValidation.extract(ContinueOnReturnCodeValidation.instance, validatedRuntimeAttributes) diff --git a/backend/src/main/scala/cromwell/backend/validation/ContinueOnReturnCodeValidation.scala b/backend/src/main/scala/cromwell/backend/validation/ContinueOnReturnCodeValidation.scala index 14dfb369d77..7573909eb69 100644 --- a/backend/src/main/scala/cromwell/backend/validation/ContinueOnReturnCodeValidation.scala +++ b/backend/src/main/scala/cromwell/backend/validation/ContinueOnReturnCodeValidation.scala @@ -3,8 +3,8 @@ package cromwell.backend.validation import cats.data.Validated.{Invalid, Valid} import cats.implicits._ import com.typesafe.config.Config -import cromwell.backend.validation.RuntimeAttributesValidation._ import common.validation.ErrorOr._ +import cromwell.backend.validation.RuntimeAttributesValidation.validateInt import wom.RuntimeAttributesKeys import wom.types._ import wom.values._ @@ -23,8 +23,11 @@ import scala.util.Try * `default` a validation with the default value specified by the reference.conf file. */ object ContinueOnReturnCodeValidation { - lazy val instance: RuntimeAttributesValidation[ContinueOnReturnCode] = new ContinueOnReturnCodeValidation - def default(runtimeConfig: Option[Config]): RuntimeAttributesValidation[ContinueOnReturnCode] = + lazy val instance: RuntimeAttributesValidation[ContinueOnReturnCode] = + new ContinueOnReturnCodeValidation + def default( + runtimeConfig: Option[Config] + ): RuntimeAttributesValidation[ContinueOnReturnCode] = instance.withDefault(configDefaultWdlValue(runtimeConfig) getOrElse WomInteger(0)) def configDefaultWdlValue(runtimeConfig: Option[Config]): Option[WomValue] = instance.configDefaultWomValue(runtimeConfig) @@ -39,6 +42,7 @@ class ContinueOnReturnCodeValidation extends RuntimeAttributesValidation[Continu override def validateValue: PartialFunction[WomValue, ErrorOr[ContinueOnReturnCode]] = { case WomBoolean(value) => ContinueOnReturnCodeFlag(value).validNel case WomString(value) if Try(value.toBoolean).isSuccess => ContinueOnReturnCodeFlag(value.toBoolean).validNel + case WomString(value) if value.equals("*") => ContinueOnReturnCodeFlag(true).validNel case WomString(value) if Try(value.toInt).isSuccess => ContinueOnReturnCodeSet(Set(value.toInt)).validNel case WomInteger(value) => ContinueOnReturnCodeSet(Set(value)).validNel case value @ WomArray(_, seq) => @@ -47,22 +51,25 @@ class ContinueOnReturnCodeValidation extends RuntimeAttributesValidation[Continu case Valid(ints) => ContinueOnReturnCodeSet(ints.toSet).validNel case Invalid(_) => invalidValueFailure(value) } + case value => invalidValueFailure(value) } override def validateExpression: PartialFunction[WomValue, Boolean] = { case WomBoolean(_) => true case WomString(value) if Try(value.toInt).isSuccess => true case WomString(value) if Try(value.toBoolean).isSuccess => true + case WomString(value) if value.equals("*") => true case WomInteger(_) => true case WomArray(WomArrayType(WomStringType), elements) => elements forall { value => Try(value.valueString.toInt).isSuccess } case WomArray(WomArrayType(WomIntegerType), _) => true + case _ => false } - override protected def missingValueMessage: String = s"Expecting $key" + - " runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + override protected def missingValueMessage: String = "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." override def usedInCallCaching: Boolean = true } diff --git a/backend/src/test/scala/cromwell/backend/BackendWorkflowInitializationActorSpec.scala b/backend/src/test/scala/cromwell/backend/BackendWorkflowInitializationActorSpec.scala index 4fc1efd7381..c4688d759f8 100644 --- a/backend/src/test/scala/cromwell/backend/BackendWorkflowInitializationActorSpec.scala +++ b/backend/src/test/scala/cromwell/backend/BackendWorkflowInitializationActorSpec.scala @@ -62,6 +62,11 @@ class BackendWorkflowInitializationActorSpec "read_int(\"bad file\")" ) + val starRow = Table( + "value", + "*" + ) + val invalidWdlValueRows = Table( "womValue", WomString(""), @@ -201,6 +206,21 @@ class BackendWorkflowInitializationActorSpec // NOTE: expressions are never valid to validate } + forAll(starRow) { value => + val womValue = WomString(value) + val result = true + testContinueOnReturnCode(Option(womValue)) should be(result) + ContinueOnReturnCodeValidation.default(optionalConfig).validateOptionalWomValue(Option(womValue)) should be( + result + ) + val valid = + ContinueOnReturnCodeValidation + .default(optionalConfig) + .validate(Map(RuntimeAttributesKeys.ContinueOnReturnCodeKey -> womValue)) + valid.isValid should be(result) + valid.toEither.toOption.get should be(ContinueOnReturnCodeFlag(true)) + } + forAll(invalidWdlValueRows) { womValue => val result = false testContinueOnReturnCode(Option(womValue)) should be(result) @@ -213,7 +233,8 @@ class BackendWorkflowInitializationActorSpec .validate(Map(RuntimeAttributesKeys.ContinueOnReturnCodeKey -> womValue)) valid.isValid should be(result) valid.toEither.swap.toOption.get.toList should contain theSameElementsAs List( - "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } diff --git a/backend/src/test/scala/cromwell/backend/standard/StandardValidatedRuntimeAttributesBuilderSpec.scala b/backend/src/test/scala/cromwell/backend/standard/StandardValidatedRuntimeAttributesBuilderSpec.scala index bd3024eda9a..35c57983e6e 100644 --- a/backend/src/test/scala/cromwell/backend/standard/StandardValidatedRuntimeAttributesBuilderSpec.scala +++ b/backend/src/test/scala/cromwell/backend/standard/StandardValidatedRuntimeAttributesBuilderSpec.scala @@ -136,7 +136,8 @@ class StandardValidatedRuntimeAttributesBuilderSpec val runtimeAttributes = Map("continueOnReturnCode" -> WomString("value")) assertRuntimeAttributesFailedCreation( runtimeAttributes, - "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } @@ -152,7 +153,6 @@ class StandardValidatedRuntimeAttributesBuilderSpec workflowOptions = workflowOptions ) } - } val defaultLogger: Logger = LoggerFactory.getLogger(classOf[StandardValidatedRuntimeAttributesBuilderSpec]) @@ -188,7 +188,7 @@ class StandardValidatedRuntimeAttributesBuilderSpec docker should be(expectedRuntimeAttributes(DockerKey).asInstanceOf[Option[String]]) failOnStderr should be(expectedRuntimeAttributes(FailOnStderrKey).asInstanceOf[Boolean]) continueOnReturnCode should be( - expectedRuntimeAttributes(ContinueOnReturnCodeKey).asInstanceOf[ContinueOnReturnCode] + expectedRuntimeAttributes(ContinueOnReturnCodeKey) ) () } diff --git a/backend/src/test/scala/cromwell/backend/validation/ContinueOnReturnCodeSpec.scala b/backend/src/test/scala/cromwell/backend/validation/ContinueOnReturnCodeSpec.scala index e0ff75d3176..bd102bc27c1 100644 --- a/backend/src/test/scala/cromwell/backend/validation/ContinueOnReturnCodeSpec.scala +++ b/backend/src/test/scala/cromwell/backend/validation/ContinueOnReturnCodeSpec.scala @@ -37,5 +37,13 @@ class ContinueOnReturnCodeSpec extends AnyWordSpecLike with CromwellTimeoutSpec ContinueOnReturnCodeSet(set).continueFor(returnCode) should be(expectedContinue) } } + + "continue on expected return code string" in { + val flagTests = Table(("string", "returnCode", "expectedContinue"), ("*", 0, true), ("*", 1, true)) + + forAll(flagTests) { (flag, returnCode, expectedContinue) => + ContinueOnReturnCodeFlag(flag == "*").continueFor(returnCode) should be(expectedContinue) + } + } } } diff --git a/backend/src/test/scala/cromwell/backend/validation/RuntimeAttributesValidationSpec.scala b/backend/src/test/scala/cromwell/backend/validation/RuntimeAttributesValidationSpec.scala index 8f692d6486a..dafca35e1b6 100644 --- a/backend/src/test/scala/cromwell/backend/validation/RuntimeAttributesValidationSpec.scala +++ b/backend/src/test/scala/cromwell/backend/validation/RuntimeAttributesValidationSpec.scala @@ -184,7 +184,8 @@ class RuntimeAttributesValidationSpec case Valid(_) => fail("A failure was expected.") case Invalid(e) => assert( - e.head == "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + e.head == "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } } @@ -212,7 +213,8 @@ class RuntimeAttributesValidationSpec case Valid(_) => fail("A failure was expected.") case Invalid(e) => assert( - e.head == "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + e.head == "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } } @@ -228,6 +230,18 @@ class RuntimeAttributesValidationSpec } } + "return success when tries to validate a valid returnCodes string entry" in { + val returnCodesValue = Some(WomString("*")) + val result = RuntimeAttributesValidation.validateContinueOnReturnCode( + returnCodesValue, + "Failed to get return code mandatory key from runtime attributes".invalidNel + ) + result match { + case Valid(x) => assert(x == ContinueOnReturnCodeFlag(true)) + case Invalid(e) => fail(e.toList.mkString(" ")) + } + } + "return success when tries to validate a valid Integer memory entry" in { val expectedGb = 1 val memoryValue = Some(WomInteger(1 << 30)) diff --git a/centaur/src/main/resources/standardTestCases/failing_return_code.test b/centaur/src/main/resources/standardTestCases/failing_return_code.test new file mode 100644 index 00000000000..163715222cf --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/failing_return_code.test @@ -0,0 +1,11 @@ +name: failing_return_code +testFormat: workflowfailure + +files { + workflow: failing_return_code/failing_return_code.wdl +} + +metadata { + workflowName: FailingReturnCode + status: Failed +} diff --git a/centaur/src/main/resources/standardTestCases/failing_return_code/failing_return_code.wdl b/centaur/src/main/resources/standardTestCases/failing_return_code/failing_return_code.wdl new file mode 100644 index 00000000000..f393698d2b7 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/failing_return_code/failing_return_code.wdl @@ -0,0 +1,20 @@ +version development-1.1 + +workflow FailingReturnCode { + call FailingReturnCodeSet +} + +task FailingReturnCodeSet { + meta { + volatile: true + } + + command <<< + exit 1 + >>> + + runtime { + returnCodes: [0, 5, 10] + docker: "ubuntu:latest" + } +} diff --git a/centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code.test b/centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code.test new file mode 100644 index 00000000000..8f55af82262 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code.test @@ -0,0 +1,11 @@ +name: invalid_return_codes_and_continue_on_return_code +testFormat: workflowfailure + +files { + workflow: invalid_return_codes_and_continue_on_return_code/invalid_return_codes_and_continue_on_return_code.wdl +} + +metadata { + workflowName: InvalidReturnCodeAndContinueOnReturnCode + status: Failed +} diff --git a/centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code/invalid_return_codes_and_continue_on_return_code.wdl b/centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code/invalid_return_codes_and_continue_on_return_code.wdl new file mode 100644 index 00000000000..ea2233ab355 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code/invalid_return_codes_and_continue_on_return_code.wdl @@ -0,0 +1,21 @@ +version development-1.1 + +workflow InvalidReturnCodeAndContinueOnReturnCode { + call InvalidReturnCodeContinueOnReturnCode +} + +task InvalidReturnCodeContinueOnReturnCode { + meta { + volatile: true + } + + command <<< + exit 1 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [5, 10, 15] + continueOnReturnCode: [1] + } +} diff --git a/centaur/src/main/resources/standardTestCases/return_codes.test b/centaur/src/main/resources/standardTestCases/return_codes.test new file mode 100644 index 00000000000..7528064f4c8 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/return_codes.test @@ -0,0 +1,11 @@ +name: return_codes +testFormat: workflowsuccess + +files { + workflow: return_codes/return_codes.wdl +} + +metadata { + workflowName: ReturnCodeValidation + status: Succeeded +} diff --git a/centaur/src/main/resources/standardTestCases/return_codes/return_codes.wdl b/centaur/src/main/resources/standardTestCases/return_codes/return_codes.wdl new file mode 100644 index 00000000000..21e1da37650 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/return_codes/return_codes.wdl @@ -0,0 +1,69 @@ +version development-1.1 + +workflow ReturnCodeValidation { + call ReturnCodeSet1 + call ReturnCodeSet2 + call ReturnCodeSet3 + call ReturnCodeString +} + +task ReturnCodeSet1 { + meta { + volatile: true + } + + command <<< + exit 1 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [1] + } +} + +task ReturnCodeSet2 { + meta { + volatile: true + } + + command <<< + exit 200 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [1, 123, 200] + } +} + +task ReturnCodeSet3 { + meta { + volatile: true + } + + command <<< + exit 10 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: 10 + } +} + + +task ReturnCodeString { + meta { + volatile: true + } + + command <<< + exit 500 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: "*" + } +} diff --git a/centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version.test b/centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version.test new file mode 100644 index 00000000000..20c07a6e051 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version.test @@ -0,0 +1,11 @@ +name: return_codes_invalid_on_old_wdl_version +testFormat: workflowfailure + +files { + workflow: return_codes_invalid_on_old_wdl_version/return_codes_invalid_on_old_wdl_version.wdl +} + +metadata { + workflowName: ReturnCodesInvalidOnOldWdl + status: Failed +} diff --git a/centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version/return_codes_invalid_on_old_wdl_version.wdl b/centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version/return_codes_invalid_on_old_wdl_version.wdl new file mode 100644 index 00000000000..296dc833f02 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version/return_codes_invalid_on_old_wdl_version.wdl @@ -0,0 +1,18 @@ +workflow ReturnCodesInvalidOnOldWdl { + call ReturnCodesInvalidOnOldWdlTask +} + +task ReturnCodesInvalidOnOldWdlTask { + meta { + volatile: "true" + } + + command <<< + exit 5 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [5, 10, 15] + } +} diff --git a/centaur/src/main/resources/standardTestCases/valid_return_codes_and_continue_on_return_code/valid_return_codes_and_continue_on_return_code.wdl b/centaur/src/main/resources/standardTestCases/valid_return_codes_and_continue_on_return_code/valid_return_codes_and_continue_on_return_code.wdl new file mode 100644 index 00000000000..41acb028221 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/valid_return_codes_and_continue_on_return_code/valid_return_codes_and_continue_on_return_code.wdl @@ -0,0 +1,55 @@ +version development-1.1 + +workflow ValidReturnCodeAndContinueOnReturnCode { + call ReturnCodeContinueOnReturnCode1 + call ReturnCodeContinueOnReturnCode2 + call ReturnCodeContinueOnReturnCode3 +} + +task ReturnCodeContinueOnReturnCode1 { + meta { + volatile: true + } + + command <<< + exit 1 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [1] + continueOnReturnCode: [0] + } +} + +task ReturnCodeContinueOnReturnCode2 { + meta { + volatile: true + } + + command <<< + exit 1 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [1] + continueOnReturnCode: false + } +} + +task ReturnCodeContinueOnReturnCode3 { + meta { + volatile: true + } + + command <<< + exit 1 + >>> + + runtime { + docker: "ubuntu:latest" + returnCodes: [1, 4, 7] + continueOnReturnCode: [1, 3, 5] + } +} diff --git a/centaur/src/main/resources/standardTestCases/valid_return_codes_and_continue_on_return_codes.test b/centaur/src/main/resources/standardTestCases/valid_return_codes_and_continue_on_return_codes.test new file mode 100644 index 00000000000..1a8354b9482 --- /dev/null +++ b/centaur/src/main/resources/standardTestCases/valid_return_codes_and_continue_on_return_codes.test @@ -0,0 +1,11 @@ +name: valid_return_codes_and_continue_on_return_code +testFormat: workflowsuccess + +files { + workflow: valid_return_codes_and_continue_on_return_code/valid_return_codes_and_continue_on_return_code.wdl +} + +metadata { + workflowName: ValidReturnCodeAndContinueOnReturnCode + status: Succeeded +} diff --git a/supportedBackends/aws/src/test/scala/cromwell/backend/impl/aws/AwsBatchRuntimeAttributesSpec.scala b/supportedBackends/aws/src/test/scala/cromwell/backend/impl/aws/AwsBatchRuntimeAttributesSpec.scala index 957126da9ad..f972f4b6f2e 100644 --- a/supportedBackends/aws/src/test/scala/cromwell/backend/impl/aws/AwsBatchRuntimeAttributesSpec.scala +++ b/supportedBackends/aws/src/test/scala/cromwell/backend/impl/aws/AwsBatchRuntimeAttributesSpec.scala @@ -202,7 +202,8 @@ class AwsBatchRuntimeAttributesSpec extends AnyWordSpecLike with CromwellTimeout ) assertAwsBatchRuntimeAttributesFailedCreation( runtimeAttributes, - "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } diff --git a/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/models/GcpBatchRuntimeAttributesSpec.scala b/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/models/GcpBatchRuntimeAttributesSpec.scala index 110dca0e348..b1755a5b3b0 100644 --- a/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/models/GcpBatchRuntimeAttributesSpec.scala +++ b/supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/models/GcpBatchRuntimeAttributesSpec.scala @@ -69,7 +69,8 @@ final class GcpBatchRuntimeAttributesSpec val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "continueOnReturnCode" -> WomString("value")) assertBatchRuntimeAttributesFailedCreation( runtimeAttributes, - "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } diff --git a/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiRuntimeAttributesSpec.scala b/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiRuntimeAttributesSpec.scala index a7a880cb28c..542883743e2 100644 --- a/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiRuntimeAttributesSpec.scala +++ b/supportedBackends/google/pipelines/common/src/test/scala/cromwell/backend/google/pipelines/common/PipelinesApiRuntimeAttributesSpec.scala @@ -99,7 +99,8 @@ final class PipelinesApiRuntimeAttributesSpec val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "continueOnReturnCode" -> WomString("value")) assertPapiRuntimeAttributesFailedCreation( runtimeAttributes, - "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } diff --git a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesRuntimeAttributesSpec.scala b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesRuntimeAttributesSpec.scala index 1fb43503fb6..7189f64a660 100644 --- a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesRuntimeAttributesSpec.scala +++ b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesRuntimeAttributesSpec.scala @@ -157,7 +157,8 @@ class TesRuntimeAttributesSpec extends AnyWordSpecLike with CromwellTimeoutSpec val runtimeAttributes = Map("docker" -> WomString("ubuntu:latest"), "continueOnReturnCode" -> WomString("value")) assertFailure( runtimeAttributes, - "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]" + "Expecting returnCodes/continueOnReturnCode" + + " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]." ) } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/wdlom2wom/package.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/wdlom2wom/package.scala index ef1f719a00e..0169ab5af62 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/wdlom2wom/package.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/wdlom2wom/package.scala @@ -1,7 +1,16 @@ package wdl.transforms.biscayne +import cats.implicits.catsSyntaxValidatedId import common.transforms.CheckedAtoB -import wdl.transforms.base.wdlom2wom.TaskDefinitionElementToWomTaskDefinition.TaskDefinitionElementToWomInputs +import common.validation.ErrorOr.{ErrorOr, ShortCircuitingFlatMapTuple2, _} +import wdl.model.draft3.elements.{ExpressionElement, RuntimeAttributesSectionElement} +import wdl.model.draft3.elements.ExpressionElement.{ArrayLiteral, KvPair, PrimitiveLiteralExpressionElement} +import wdl.model.draft3.graph.ExpressionValueConsumer +import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator} +import wdl.transforms.base.wdlom2wom.TaskDefinitionElementToWomTaskDefinition.{ + eliminateInputDependencies, + TaskDefinitionElementToWomInputs +} import wdl.transforms.base.wdlom2wom.WorkflowDefinitionElementToWomWorkflowDefinition.WorkflowDefinitionConvertInputs import wdl.transforms.base.wdlom2wom.{ FileElementToWomBundle, @@ -15,13 +24,93 @@ import wdl.transforms.biscayne.linking.expression.consumed._ import wdl.transforms.biscayne.linking.expression.files._ import wdl.transforms.biscayne.linking.expression.types._ import wdl.transforms.biscayne.linking.expression.values._ +import wom.values.WomInteger +import wom.{RuntimeAttributes, RuntimeAttributesKeys} package object wdlom2wom { val taskDefinitionElementToWomTaskDefinition: CheckedAtoB[TaskDefinitionElementToWomInputs, CallableTaskDefinition] = - CheckedAtoB.fromErrorOr(TaskDefinitionElementToWomTaskDefinition.convert) + CheckedAtoB.fromErrorOr(convert) val workflowDefinitionElementToWomWorkflowDefinition : CheckedAtoB[WorkflowDefinitionConvertInputs, WorkflowDefinition] = CheckedAtoB.fromErrorOr(WorkflowDefinitionElementToWomWorkflowDefinition.convert) val fileElementToWomBundle: CheckedAtoB[FileElementToWomBundleInputs, WomBundle] = CheckedAtoB.fromCheck(FileElementToWomBundle.convert) + + private def convert(b: TaskDefinitionElementToWomInputs)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], + fileEvaluator: FileEvaluator[ExpressionElement], + typeEvaluator: TypeEvaluator[ExpressionElement], + valueEvaluator: ValueEvaluator[ExpressionElement] + ): ErrorOr[CallableTaskDefinition] = { + val a = eliminateInputDependencies(b)(expressionValueConsumer) + + val conversion = + TaskDefinitionElementToWomTaskDefinition.createTaskGraphAndValidateMetadata(a)(expressionValueConsumer, + fileEvaluator, + typeEvaluator, + valueEvaluator + ) flatMapN { (taskGraph, _) => + val validRuntimeAttributes: ErrorOr[RuntimeAttributes] = a.taskDefinitionElement.runtimeSection match { + case Some(attributeSection) => + TaskDefinitionElementToWomTaskDefinition.createRuntimeAttributes( + RuntimeAttributesSectionElement(getFinalRuntimeAttributes(attributeSection)), + taskGraph.linkedGraph + )( + expressionValueConsumer, + fileEvaluator, + typeEvaluator, + valueEvaluator + ) + case None => RuntimeAttributes(Map.empty).validNel + } + + TaskDefinitionElementToWomTaskDefinition.createCallableTaskDefinition(a, taskGraph, validRuntimeAttributes)( + expressionValueConsumer, + fileEvaluator, + typeEvaluator, + valueEvaluator + ) + } + + conversion.contextualizeErrors(s"process task definition '${b.taskDefinitionElement.name}'") + } + + /** + * Combine `returnCodes` and `continueOnReturnCode` to be a single attribute. The resulting vector will contain + * `continueOnReturnCode` if either `continueOnReturnCode` or `returnCodes` was in the `attributeSection`, it will + * never contain `returnCodes`. The value for the `continueOnReturnCode` key in the new vector will be the value + * associated with `returnCodes` in the original vector if it exists, else it will be the value associated with + * `continueOnReturnCode` in the original vector. + * @param attributeSection list of all runtime attributes and their values + * @return A vector of pairs of runtime attribute keys to their respective values + */ + private def getFinalRuntimeAttributes(attributeSection: RuntimeAttributesSectionElement): Vector[KvPair] = { + val returnCodesAttribute = + attributeSection.runtimeAttributes.toList.find(pair => pair.key.equals(RuntimeAttributesKeys.ReturnCodesKey)) + val continueOnReturnCodeAttribute = + attributeSection.runtimeAttributes.toList.find(pair => + pair.key.equals(RuntimeAttributesKeys.ContinueOnReturnCodeKey) + ) + + val returnCodesNotUnique = (returnCodesAttribute, continueOnReturnCodeAttribute) match { + case (Some(returnCodesValue), Some(continueOnReturnCodeValue)) => + returnCodesValue.value + .equals(continueOnReturnCodeValue.value) || returnCodesValue.value.equals( + ArrayLiteral(Vector(PrimitiveLiteralExpressionElement(WomInteger(0)))) + ) + case _ => false + } + + val finalAttributes = (returnCodesAttribute, returnCodesNotUnique) match { + case (Some(returnCodesValue), false) => + attributeSection.runtimeAttributes.filterNot(attribute => + attribute.key.equals(RuntimeAttributesKeys.ContinueOnReturnCodeKey) + ) ++ Vector( + KvPair(RuntimeAttributesKeys.ContinueOnReturnCodeKey, returnCodesValue.value) + ) + case _ => attributeSection.runtimeAttributes + } + + finalAttributes.filterNot(attribute => attribute.key.equals(RuntimeAttributesKeys.ReturnCodesKey)) + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/wdlom2wom/package.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/wdlom2wom/package.scala index 8933a00cb57..6441c46f09f 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/wdlom2wom/package.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/wdlom2wom/package.scala @@ -1,7 +1,16 @@ package wdl.transforms.cascades +import cats.implicits.catsSyntaxValidatedId import common.transforms.CheckedAtoB -import wdl.transforms.base.wdlom2wom.TaskDefinitionElementToWomTaskDefinition.TaskDefinitionElementToWomInputs +import common.validation.ErrorOr.{ErrorOr, ShortCircuitingFlatMapTuple2, _} +import wdl.model.draft3.elements.ExpressionElement.{ArrayLiteral, KvPair, PrimitiveLiteralExpressionElement} +import wdl.model.draft3.elements.{ExpressionElement, RuntimeAttributesSectionElement} +import wdl.model.draft3.graph.ExpressionValueConsumer +import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator} +import wdl.transforms.base.wdlom2wom.TaskDefinitionElementToWomTaskDefinition.{ + eliminateInputDependencies, + TaskDefinitionElementToWomInputs +} import wdl.transforms.base.wdlom2wom.WorkflowDefinitionElementToWomWorkflowDefinition.WorkflowDefinitionConvertInputs import wdl.transforms.base.wdlom2wom.{ FileElementToWomBundle, @@ -15,13 +24,93 @@ import wdl.transforms.cascades.linking.expression.consumed._ import wdl.transforms.cascades.linking.expression.files._ import wdl.transforms.cascades.linking.expression.types._ import wdl.transforms.cascades.linking.expression.values._ +import wom.values.WomInteger +import wom.{RuntimeAttributes, RuntimeAttributesKeys} package object wdlom2wom { val taskDefinitionElementToWomTaskDefinition: CheckedAtoB[TaskDefinitionElementToWomInputs, CallableTaskDefinition] = - CheckedAtoB.fromErrorOr(TaskDefinitionElementToWomTaskDefinition.convert) + CheckedAtoB.fromErrorOr(convert) val workflowDefinitionElementToWomWorkflowDefinition : CheckedAtoB[WorkflowDefinitionConvertInputs, WorkflowDefinition] = CheckedAtoB.fromErrorOr(WorkflowDefinitionElementToWomWorkflowDefinition.convert) val fileElementToWomBundle: CheckedAtoB[FileElementToWomBundleInputs, WomBundle] = CheckedAtoB.fromCheck(FileElementToWomBundle.convert) + + private def convert(b: TaskDefinitionElementToWomInputs)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], + fileEvaluator: FileEvaluator[ExpressionElement], + typeEvaluator: TypeEvaluator[ExpressionElement], + valueEvaluator: ValueEvaluator[ExpressionElement] + ): ErrorOr[CallableTaskDefinition] = { + val a = eliminateInputDependencies(b)(expressionValueConsumer) + + val conversion = + TaskDefinitionElementToWomTaskDefinition.createTaskGraphAndValidateMetadata(a)(expressionValueConsumer, + fileEvaluator, + typeEvaluator, + valueEvaluator + ) flatMapN { (taskGraph, _) => + val validRuntimeAttributes: ErrorOr[RuntimeAttributes] = a.taskDefinitionElement.runtimeSection match { + case Some(attributeSection) => + TaskDefinitionElementToWomTaskDefinition.createRuntimeAttributes( + RuntimeAttributesSectionElement(getFinalRuntimeAttributes(attributeSection)), + taskGraph.linkedGraph + )( + expressionValueConsumer, + fileEvaluator, + typeEvaluator, + valueEvaluator + ) + case None => RuntimeAttributes(Map.empty).validNel + } + + TaskDefinitionElementToWomTaskDefinition.createCallableTaskDefinition(a, taskGraph, validRuntimeAttributes)( + expressionValueConsumer, + fileEvaluator, + typeEvaluator, + valueEvaluator + ) + } + + conversion.contextualizeErrors(s"process task definition '${b.taskDefinitionElement.name}'") + } + + /** + * Combine `returnCodes` and `continueOnReturnCode` to be a single attribute. The resulting vector will contain + * `continueOnReturnCode` if either `continueOnReturnCode` or `returnCodes` was in the `attributeSection`, it will + * never contain `returnCodes`. The value for the `continueOnReturnCode` key in the new vector will be the value + * associated with `returnCodes` in the original vector if it exists, else it will be the value associated with + * `continueOnReturnCode` in the original vector. + * @param attributeSection list of all runtime attributes and their values + * @return A vector of pairs of runtime attribute keys to their respective values + */ + private def getFinalRuntimeAttributes(attributeSection: RuntimeAttributesSectionElement): Vector[KvPair] = { + val returnCodesAttribute = + attributeSection.runtimeAttributes.toList.find(pair => pair.key.equals(RuntimeAttributesKeys.ReturnCodesKey)) + val continueOnReturnCodeAttribute = + attributeSection.runtimeAttributes.toList.find(pair => + pair.key.equals(RuntimeAttributesKeys.ContinueOnReturnCodeKey) + ) + + val returnCodesNotUnique = (returnCodesAttribute, continueOnReturnCodeAttribute) match { + case (Some(returnCodesValue), Some(continueOnReturnCodeValue)) => + returnCodesValue.value + .equals(continueOnReturnCodeValue.value) || returnCodesValue.value.equals( + ArrayLiteral(Vector(PrimitiveLiteralExpressionElement(WomInteger(0)))) + ) + case _ => false + } + + val finalAttributes = (returnCodesAttribute, returnCodesNotUnique) match { + case (Some(returnCodesValue), false) => + attributeSection.runtimeAttributes.filterNot(attribute => + attribute.key.equals(RuntimeAttributesKeys.ContinueOnReturnCodeKey) + ) ++ Vector( + KvPair(RuntimeAttributesKeys.ContinueOnReturnCodeKey, returnCodesValue.value) + ) + case _ => attributeSection.runtimeAttributes + } + + finalAttributes.filterNot(attribute => attribute.key.equals(RuntimeAttributesKeys.ReturnCodesKey)) + } } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/TaskDefinitionElementToWomTaskDefinition.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/TaskDefinitionElementToWomTaskDefinition.scala index 576ce210da5..d6e0eba7b42 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/TaskDefinitionElementToWomTaskDefinition.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/TaskDefinitionElementToWomTaskDefinition.scala @@ -2,29 +2,29 @@ package wdl.transforms.base.wdlom2wom import cats.data.NonEmptyList import cats.data.Validated.{Invalid, Valid} +import cats.instances.list._ import cats.syntax.apply._ import cats.syntax.traverse._ import cats.syntax.validated._ -import cats.instances.list._ import common.validation.ErrorOr.{ErrorOr, _} import wdl.model.draft3.elements.CommandPartElement.{PlaceholderCommandPartElement, StringCommandPartElement} import wdl.model.draft3.elements.ExpressionElement.{ArrayLiteral, IdentifierLookup, KvPair, SelectFirst} import wdl.model.draft3.elements._ -import wdl.model.draft3.graph.{ExpressionValueConsumer, LinkedGraph} -import wom.callable.Callable._ -import wom.callable.{Callable, CallableTaskDefinition, MetaValueElement} -import wom.expression.WomExpression -import wom.types.{WomOptionalType, WomType} -import wom.{CommandPart, RuntimeAttributes} import wdl.model.draft3.graph.ExpressionValueConsumer.ops._ -import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator} import wdl.model.draft3.graph.expression.WomExpressionMaker.ops._ +import wdl.model.draft3.graph.expression.WomTypeMaker.ops._ +import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator} +import wdl.model.draft3.graph.{ExpressionValueConsumer, LinkedGraph} import wdl.transforms.base.linking.expression._ import wdl.transforms.base.linking.graph.LinkedGraphMaker +import wdl.transforms.base.linking.typemakers._ import wdl.transforms.base.wdlom2wom.expression.renaming.IdentifierLookupRenamer.ops._ import wdl.transforms.base.wdlom2wom.expression.renaming.expressionEvaluator -import wdl.model.draft3.graph.expression.WomTypeMaker.ops._ -import wdl.transforms.base.linking.typemakers._ +import wom.callable.Callable._ +import wom.callable.{Callable, CallableTaskDefinition, MetaValueElement} +import wom.expression.WomExpression +import wom.types.{WomOptionalType, WomType} +import wom.{CommandPart, RuntimeAttributes} object TaskDefinitionElementToWomTaskDefinition extends Util { @@ -39,60 +39,79 @@ object TaskDefinitionElementToWomTaskDefinition extends Util { valueEvaluator: ValueEvaluator[ExpressionElement] ): ErrorOr[CallableTaskDefinition] = { val a = eliminateInputDependencies(b) - val inputElements = a.taskDefinitionElement.inputsSection.map(_.inputDeclarations).getOrElse(Seq.empty) - - val declarations = a.taskDefinitionElement.declarations - val outputElements = a.taskDefinitionElement.outputsSection.map(_.outputs).getOrElse(Seq.empty) - val conversion = ( - createTaskGraph(inputElements, - declarations, - outputElements, - a.taskDefinitionElement.parameterMetaSection, - a.typeAliases - ), - validateParameterMetaEntries(a.taskDefinitionElement.parameterMetaSection, - a.taskDefinitionElement.inputsSection, - a.taskDefinitionElement.outputsSection - ) - ) flatMapN { (taskGraph, _) => + val conversion = createTaskGraphAndValidateMetadata(a) flatMapN { (taskGraph, _) => val validRuntimeAttributes: ErrorOr[RuntimeAttributes] = a.taskDefinitionElement.runtimeSection match { case Some(attributeSection) => createRuntimeAttributes(attributeSection, taskGraph.linkedGraph) case None => RuntimeAttributes(Map.empty).validNel } - - val validCommand: ErrorOr[Seq[CommandPart]] = - expandLines(a.taskDefinitionElement.commandSection.parts).toList - .traverse { parts => - CommandPartElementToWomCommandPart.convert(parts, - taskGraph.linkedGraph.typeAliases, - taskGraph.linkedGraph.generatedHandles - ) - } - .map(_.toSeq) - - val (meta, parameterMeta) = - processMetaSections(a.taskDefinitionElement.metaSection, a.taskDefinitionElement.parameterMetaSection) - - (validRuntimeAttributes, validCommand) mapN { (runtime, command) => - CallableTaskDefinition( - a.taskDefinitionElement.name, - Function.const(command.validNel), - runtime, - meta, - parameterMeta, - taskGraph.outputs, - taskGraph.inputs, - Set.empty, - Map.empty, - sourceLocation = a.taskDefinitionElement.sourceLocation - ) - } + createCallableTaskDefinition(a, taskGraph, validRuntimeAttributes) } conversion.contextualizeErrors(s"process task definition '${b.taskDefinitionElement.name}'") } + def createTaskGraphAndValidateMetadata(a: TaskDefinitionElementToWomInputs)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], + fileEvaluator: FileEvaluator[ExpressionElement], + typeEvaluator: TypeEvaluator[ExpressionElement], + valueEvaluator: ValueEvaluator[ExpressionElement] + ): (ErrorOr[TaskGraph], ErrorOr[Unit]) = { + val inputElements = a.taskDefinitionElement.inputsSection.map(_.inputDeclarations).getOrElse(Seq.empty) + + val declarations = a.taskDefinitionElement.declarations + val outputElements = a.taskDefinitionElement.outputsSection.map(_.outputs).getOrElse(Seq.empty) + + (createTaskGraph(inputElements, + declarations, + outputElements, + a.taskDefinitionElement.parameterMetaSection, + a.typeAliases + ), + validateParameterMetaEntries(a.taskDefinitionElement.parameterMetaSection, + a.taskDefinitionElement.inputsSection, + a.taskDefinitionElement.outputsSection + ) + ) + } + + def createCallableTaskDefinition(a: TaskDefinitionElementToWomInputs, + taskGraph: TaskGraph, + validRuntimeAttributes: ErrorOr[RuntimeAttributes] + )(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], + fileEvaluator: FileEvaluator[ExpressionElement], + typeEvaluator: TypeEvaluator[ExpressionElement], + valueEvaluator: ValueEvaluator[ExpressionElement] + ): ErrorOr[CallableTaskDefinition] = { + val validCommand: ErrorOr[Seq[CommandPart]] = + expandLines(a.taskDefinitionElement.commandSection.parts).toList + .traverse { parts => + CommandPartElementToWomCommandPart.convert(parts, + taskGraph.linkedGraph.typeAliases, + taskGraph.linkedGraph.generatedHandles + ) + } + .map(_.toSeq) + + val (meta, parameterMeta) = + processMetaSections(a.taskDefinitionElement.metaSection, a.taskDefinitionElement.parameterMetaSection) + + (validRuntimeAttributes, validCommand) mapN { (runtime, command) => + CallableTaskDefinition( + a.taskDefinitionElement.name, + Function.const(command.validNel), + runtime, + meta, + parameterMeta, + taskGraph.outputs, + taskGraph.inputs, + Set.empty, + Map.empty, + sourceLocation = a.taskDefinitionElement.sourceLocation + ) + } + } private def validateParameterMetaEntries(parameterMetaSectionElement: Option[ParameterMetaSectionElement], inputs: Option[InputsSectionElement], outputs: Option[OutputsSectionElement] @@ -116,7 +135,7 @@ object TaskDefinitionElementToWomTaskDefinition extends Util { } } - private def eliminateInputDependencies( + def eliminateInputDependencies( a: TaskDefinitionElementToWomInputs )(implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]): TaskDefinitionElementToWomInputs = { case class NewInputElementsSet(original: InputDeclarationElement, @@ -221,9 +240,9 @@ object TaskDefinitionElementToWomTaskDefinition extends Util { } } - final private case class TaskGraph(inputs: List[Callable.InputDefinition], - outputs: List[Callable.OutputDefinition], - linkedGraph: LinkedGraph + final case class TaskGraph(inputs: List[Callable.InputDefinition], + outputs: List[Callable.OutputDefinition], + linkedGraph: LinkedGraph ) private def createTaskGraph(inputs: Seq[InputDeclarationElement], @@ -319,7 +338,7 @@ object TaskDefinitionElementToWomTaskDefinition extends Util { } } - private def createRuntimeAttributes(attributes: RuntimeAttributesSectionElement, linkedGraph: LinkedGraph)(implicit + def createRuntimeAttributes(attributes: RuntimeAttributesSectionElement, linkedGraph: LinkedGraph)(implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], fileEvaluator: FileEvaluator[ExpressionElement], typeEvaluator: TypeEvaluator[ExpressionElement], diff --git a/wom/src/main/scala/wom/RuntimeAttributes.scala b/wom/src/main/scala/wom/RuntimeAttributes.scala index 2c66b6859d3..216d6404dc5 100644 --- a/wom/src/main/scala/wom/RuntimeAttributes.scala +++ b/wom/src/main/scala/wom/RuntimeAttributes.scala @@ -15,6 +15,10 @@ object RuntimeAttributesKeys { val MemoryKey = "memory" val FailOnStderrKey = "failOnStderr" val ContinueOnReturnCodeKey = "continueOnReturnCode" + + // New for WDL 1.1 + // Semantically, this is the same as continueOnReturnCode as the two attributes are combined at the parsing stage + val ReturnCodesKey = "returnCodes" } case class RuntimeAttributes(attributes: Map[String, WomExpression])