Skip to content

Commit

Permalink
Hotfix 30.1 cherry picks (#3081)
Browse files Browse the repository at this point in the history
* Imperfect retries for GCS requests

* Include OGINs for if and scatter expressions in GraphNode accumulation

* Readable error message for invalid indexing

* Fixup coercion into the WomNonEmptyArrayType

* Power through non-findable terminals in error messages

* Allow 0 shard scatters

* Show optional inputs in womtool (#3050)

* Use call IO functions for evaluating runtime attributes

* Allow member accesses to parse

* Unstuck workflows and GCS retries
  • Loading branch information
cjllanwarne authored Dec 20, 2017
1 parent 70eb8ee commit 047d48f
Show file tree
Hide file tree
Showing 71 changed files with 932 additions and 390 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cromwell.backend

import _root_.wdl._
import cromwell.core.{NoIoFunctionSet, WorkflowOptions}
import cromwell.core.WorkflowOptions
import cromwell.util.JsonFormatting.WomValueJsonFormatter
import common.validation.ErrorOr.ErrorOr
import wom.callable.Callable.InputDefinition
Expand All @@ -25,7 +25,7 @@ object RuntimeAttributeDefinition {
evaluatedInputs: Map[InputDefinition, WomValue]): ErrorOr[Map[String, WomValue]] = {
import common.validation.ErrorOr._
val inputsMap = evaluatedInputs map { case (x, y) => x.name -> y }
unevaluated.attributes.traverseValues(_.evaluateValue(inputsMap, NoIoFunctionSet))
unevaluated.attributes.traverseValues(_.evaluateValue(inputsMap, wdlFunctions))
}

def buildMapBasedLookup(evaluatedDeclarations: Map[InputDefinition, Try[WomValue]])(identifier: String): WomValue = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ trait StandardAsyncExecutionActor extends AsyncBackendJobExecutionActor with Sta
customPollStatusFailure orElse {
case (_: ExecutionHandle, exception: Exception) if isFatal(exception) =>
// Log exceptions and return the original handle to try again.
jobLogger.warn(s"Fatal exception polling for status. Job will fail.")
jobLogger.warn(s"Fatal exception polling for status. Job will fail.", exception)
FailedNonRetryableExecutionHandle(exception)
case (handle: ExecutionHandle, exception: Exception) =>
// Log exceptions and return the original handle to try again.
Expand Down
11 changes: 10 additions & 1 deletion centaur/src/it/scala/centaur/CentaurTestSuite.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package centaur

import centaur.api.CentaurCromwellClient
import centaur.test.standard.CentaurTestCase
import centaur.test.standard.CentaurTestFormat.{InstantAbort, RestartFormat, ScheduledAbort}
import org.scalatest.{BeforeAndAfterAll, ParallelTestExecution, Suites}

import scala.sys.ShutdownHookThread
Expand All @@ -17,12 +19,19 @@ object CentaurTestSuite {
}

val cromwellBackends = CentaurCromwellClient.backends.get.supportedBackends.map(_.toLowerCase)

def runSequential(testCase: CentaurTestCase) = testCase.testFormat match {
case _: RestartFormat| _: ScheduledAbort | InstantAbort => true
case _ => testCase.workflow.data.workflowType.exists(_.equalsIgnoreCase("cwl"))
}

def runParallel(testCase: CentaurTestCase) = !runSequential(testCase)
}

/**
* The main centaur test suites, runs sub suites in parallel, but allows better control over the way each nested suite runs.
*/
class CentaurTestSuite extends Suites(new RestartTestCaseSpec(), new StandardTestCaseSpec()) with ParallelTestExecution with BeforeAndAfterAll {
class CentaurTestSuite extends Suites(new SequentialTestCaseSpec(), new StandardTestCaseSpec()) with ParallelTestExecution with BeforeAndAfterAll {
private var shutdownHook: Option[ShutdownHookThread] = _

override def beforeAll() = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
package centaur

import centaur.test.standard.CentaurTestFormat.RestartFormat
import org.scalatest.{DoNotDiscover, Matchers}

/**
* All test cases that trigger a Cromwell restart. Note that this suite does not mix in ParallelTestExecution
* such that the restarting tests execute sequentially to avoid a mayhem of Cromwell restarts
*/
@DoNotDiscover
class RestartTestCaseSpec(cromwellBackends: List[String]) extends AbstractCentaurTestCaseSpec(cromwellBackends) with Matchers {
class SequentialTestCaseSpec(cromwellBackends: List[String]) extends AbstractCentaurTestCaseSpec(cromwellBackends) with Matchers {

def this() = this(CentaurTestSuite.cromwellBackends)

allTestCases.filter( _.testFormat match {
case _: RestartFormat => true
case _ => false
}
) foreach executeStandardTest
allTestCases.filter(CentaurTestSuite.runSequential) foreach executeStandardTest

}
9 changes: 2 additions & 7 deletions centaur/src/it/scala/centaur/StandardTestCaseSpec.scala
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
package centaur

import centaur.test.standard.CentaurTestFormat.RestartFormat
import org.scalatest._

@DoNotDiscover
class StandardTestCaseSpec(cromwellBackends: List[String]) extends AbstractCentaurTestCaseSpec(cromwellBackends) with ParallelTestExecution {

def this() = this(CentaurTestSuite.cromwellBackends)

allTestCases.filter( _.testFormat match {
case _: RestartFormat => false
case _ => true
}
) foreach executeStandardTest

allTestCases.filter(CentaurTestSuite.runParallel) foreach executeStandardTest
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import "chain_fail_import.wdl" as sub

workflow chain_fail {
call fail

Array[Boolean] is = [ fail.o ]

# This scatter will never be runnable because the fail task will... fail
scatter (j in is) {
call fail as scatter_fail
}

if (fail.o) {
call fail as conditional_fail
}

call sub.wf_hello { input: wf_hello_input = fail.o }

output {
Array[Boolean] scatter_o = scatter_fail.o
Boolean? conditional_o = conditional_fail.o
String sub_workflow_o = wf_hello.salutation
}
}


task fail {
command {
echo false
exit 1
}
runtime {
docker: "ubuntu:latest"
}
output {
Boolean o = read_boolean(stdout())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
task hello {
Boolean addressee
command {
echo "Hello ${addressee}!"
}
runtime {
docker: "ubuntu:latest"
}
output {
String salutation = read_string(stdout())
}
}

workflow wf_hello {
Boolean wf_hello_input = true

call hello { input: addressee = wf_hello_input }

output {
String salutation = hello.salutation
}
}
11 changes: 11 additions & 0 deletions centaur/src/main/resources/standardTestCases/chainfail.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Test that if a scatter / conditional / sub workflow depends on *something* that fails,
# the workflow fails properly even if later nodes depend on the scatter / conditional / sub workflow.
name: chainfail
testFormat: workflowfailure

files {
wdl: chain_fail/chain_fail.wdl
imports: [
chain_fail/chain_fail_import.wdl
]
}
13 changes: 13 additions & 0 deletions centaur/src/main/resources/standardTestCases/empty_scatter.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: empty_scatter
testFormat: workflowsuccess

files {
wdl: empty_scatter/empty_scatter.wdl
}

metadata {
workflowName: empty_scatter
status: Succeeded
"outputs.empty_scatter.task_outs_length" : "0"
"outputs.empty_scatter.decls_length" : "0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
workflow empty_scatter {
Array[Int] xs = []

scatter (x in xs) {
Int decl = 75 + x
call do_nothing { input: whoa = decl }
}

output {
Int task_outs_length = length(do_nothing.str)
Int decls_length = length(decl)
}
}

# We should never invoke this since we're scattering over an empty array
task do_nothing {
Int whoa
command {
# doesn't matter, won't be run anyway:
exit 500
}
output {
String str = "str"
}
runtime {
docker: "ubuntu:latest"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ files {
metadata {
workflowName: test
status: Succeeded
"outputs.test.left_out": "bar 2 left"
"outputs.test.left_out": "bar 2 left 27"
"outputs.test.triple_left": "4"
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ task echo_str {
command {
echo ${s[0]} ${s[1]} ${s[2]}
}
output { Pair[String, Int] left = (read_string(stdout()), 27) }
output {
Pair[String, Int] left = (read_string(stdout()), 27)
}
runtime {
docker: "ubuntu:latest"
}
Expand All @@ -19,9 +21,13 @@ workflow test {
input: s = [a[1], m["c"], p.left]
}

call echo_str as echo_str_2 {
input: s = [ echo_str.left.left, echo_str.left.right, echo_str.left.left ]
}

output {
# This gets the 'left' output from task echo_str, and then a member lookup for the pair's 'left' element
String left_out = echo_str.left.left
String left_out = echo_str.left.left + " " + echo_str_2.left.right

# Oh hey, nested Pair lookups also works now! Magic!
Int triple_left = triple.left.left + triple.right.left
Expand Down
16 changes: 16 additions & 0 deletions centaur/src/main/resources/standardTestCases/object_access.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: object_access
testFormat: workflowsuccess

files {
wdl: object_access/object_access.wdl
}

metadata {
workflowName: object_access
status: Succeeded
"outputs.object_access.lines.0": "1",
"outputs.object_access.lines.1": "2",
"outputs.object_access.lines.2": "3",
"outputs.object_access.int_a": "1",
"outputs.object_access.int_b": "2",
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
workflow object_access {
call mk_object
call use_object { input: obj_in = mk_object.out }
call use_field as get_a { input: int_in = mk_object.out.a }

Object temp = mk_object.out
call use_field as get_b { input: int_in = temp.b }

output {
Array[String] lines = use_object.lines
Int int_a = get_a.int_out
Int int_b = get_b.int_out
}
}

task mk_object {
command {
echo -e "a\tb\tc"
echo -e "1\t2\t3"
}
output {
Object out = read_object(stdout())
}
runtime {
docker: "ubuntu:latest"
}
}

task use_object {
Object obj_in
command {
echo ${obj_in.a}
echo ${obj_in.b}
echo ${obj_in.c}
}
output {
Array[String] lines = read_lines(stdout())
}
runtime {
docker: "ubuntu:latest"
}
}

task use_field {
Int int_in
command {
echo ${int_in}
}
output {
Int int_out = read_int(stdout())
}
runtime {
docker: "ubuntu:latest"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ testFormat: workflowsuccess
files {
wdl: runtime_attribute_expressions/runtime_attribute_expressions.wdl
}

metadata {
"outputs.runtime_attribute_expressions.out": "Hello World"
}
Original file line number Diff line number Diff line change
@@ -1,26 +1,42 @@

task expression {
String version
File memory_sizer

command {
echo "Hello world!"
echo "Hello World"
}

output {
String out = stdout()
String out = read_string(stdout())
}

runtime {
# Not a literal WdlString
docker: "ubuntu:" + version

# Uses IO functions to evaluate the 'size':
memory: ceil(size(memory_sizer)) + "GB"
}
}

task make_2_byte_file {
command {
# One byte comes from the newline:
echo "a" > ab.txt
}
output {
File f = "ab.txt"
}
runtime { docker: "ubuntu:latest" }
}

workflow runtime_attribute_expressions {

call expression { input: version="latest" }
call make_2_byte_file
call expression { input: version="latest", memory_sizer = make_2_byte_file.f }

output {
expression.out
String out = expression.out
}
}
Loading

0 comments on commit 047d48f

Please sign in to comment.