Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WX-1468] Implement returnCodes runtime attribute #7389

Merged
merged 28 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand All @@ -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)
Expand All @@ -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) =>
Expand All @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ class BackendWorkflowInitializationActorSpec
"read_int(\"bad file\")"
)

val starRow = Table(
"value",
"*"
)

val invalidWdlValueRows = Table(
"womValue",
WomString(""),
Expand Down Expand Up @@ -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)
Expand All @@ -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]."
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]."
)
}

Expand All @@ -152,7 +153,6 @@ class StandardValidatedRuntimeAttributesBuilderSpec
workflowOptions = workflowOptions
)
}

}

val defaultLogger: Logger = LoggerFactory.getLogger(classOf[StandardValidatedRuntimeAttributesBuilderSpec])
Expand Down Expand Up @@ -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)
)
()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package cromwell.backend.validation

import common.assertion.CromwellTimeoutSpec
import org.scalatest.BeforeAndAfterAll
import org.scalatest.matchers.should.Matchers
import org.scalatest.prop.TableDrivenPropertyChecks._
import org.scalatest.wordspec.AnyWordSpecLike

class ReturnCodesSpec extends AnyWordSpecLike with CromwellTimeoutSpec with Matchers with BeforeAndAfterAll {
"Checking for return codes" should {
"continue on expected return code flags" in {
val flagTests = Table(("flag", "returnCode", "expectedContinue"), ("*", 0, true), ("*", 1, true))

forAll(flagTests) { (flag, returnCode, expectedContinue) =>
ContinueOnReturnCodeFlag(true).continueFor(returnCode) should be(expectedContinue)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]."
)
}
}
Expand Down Expand Up @@ -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]."
)
}
}
Expand All @@ -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))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: failing_return_code
testFormat: workflowfailure

files {
workflow: failing_return_code/failing_return_code.wdl
}

metadata {
workflowName: FailingReturnCode
status: Failed
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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]
}
}
11 changes: 11 additions & 0 deletions centaur/src/main/resources/standardTestCases/return_codes.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: return_codes
testFormat: workflowsuccess

files {
workflow: return_codes/return_codes.wdl
}

metadata {
workflowName: ReturnCodeValidation
status: Succeeded
}
Original file line number Diff line number Diff line change
@@ -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: "*"
}
}
Loading
Loading