Skip to content

Commit

Permalink
Allow nested inputs when corresponding meta setting is set to true
Browse files Browse the repository at this point in the history
This implements part of the spec change that handles inputs openwdl/wdl#359
  • Loading branch information
rhpvorderman committed Jul 1, 2020
1 parent f93da32 commit aded97f
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@ import cats.syntax.validated._
import common.validation.ErrorOr.{ErrorOr, _}
import wdl.model.draft3.elements.ExpressionElement.{ArrayLiteral, IdentifierLookup, SelectFirst}
import wdl.model.draft3.elements._
import wdl.model.draft3.graph.ExpressionValueConsumer.ops._
import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator}
import wdl.model.draft3.graph.{ExpressionValueConsumer, GeneratedCallFinishedHandle, GeneratedValueHandle, LinkedGraph}
import wdl.shared.transforms.wdlom2wom.WomGraphMakerTools
import wdl.transforms.base.linking.graph.LinkedGraphMaker
import wdl.transforms.base.wdlom2wom.graph.renaming.GraphIdentifierLookupRenamer.ops._
import wdl.transforms.base.wdlom2wom.graph.renaming._
import wdl.transforms.base.wdlom2wom.graph.{GraphNodeMakerInputs, WorkflowGraphElementToGraphNode}
import wom.callable.MetaValueElement.MetaValueElementBoolean
import wom.callable.{Callable, WorkflowDefinition}
import wom.graph.expression.AnonymousExpressionNode
import wom.graph.GraphNodePort.OutputPort
import wom.graph.expression.AnonymousExpressionNode
import wom.graph.{CallNode, GraphNode, WomIdentifier, Graph => WomGraph}
import wom.types.WomType
import wdl.model.draft3.graph.ExpressionValueConsumer.ops._
import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator}
import wdl.transforms.base.wdlom2wom.graph.renaming.GraphIdentifierLookupRenamer.ops._
import wdl.transforms.base.wdlom2wom.graph.renaming._

object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {

Expand All @@ -33,6 +34,14 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {

val a: WorkflowDefinitionConvertInputs = eliminateInputDependencies(b)

val (meta, parameterMeta) = processMetaSections(a.definitionElement.metaSection, a.definitionElement.parameterMetaSection)

val allowNestedInputs: Boolean = {
meta.get("allowNestedInputs").flatMap {
case x: MetaValueElementBoolean => Option(x.value)
case _ => None
}}.getOrElse(false)

// Make the set of workflow graph elements, including:
// - Top-level graph elements
// - Declarations in the inputs section
Expand All @@ -45,6 +54,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {
val innerGraph: ErrorOr[WomGraph] = convertGraphElements(GraphLikeConvertInputs(graphNodeElements, Set.empty, Map.empty, a.typeAliases, a.definitionElement.name,
insideAScatter = false,
convertNestedScatterToSubworkflow = b.convertNestedScatterToSubworkflow,
allowNestedInputs = allowNestedInputs,
a.callables))
// NB: isEmpty means "not isDefined". We specifically do NOT add defaults if the output section is defined but empty.
val withDefaultOutputs: ErrorOr[WomGraph] = if (a.definitionElement.outputsSection.isEmpty) {
Expand All @@ -53,8 +63,6 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {
innerGraph
}

val (meta, parameterMeta) = processMetaSections(a.definitionElement.metaSection, a.definitionElement.parameterMetaSection)

(withDefaultOutputs map {
ig => WorkflowDefinition(a.definitionElement.name, ig, meta, parameterMeta, b.definitionElement.sourceLocation)
}).contextualizeErrors(s"process workflow definition '${a.definitionElement.name}'")
Expand All @@ -67,6 +75,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {
workflowName: String,
insideAScatter: Boolean,
convertNestedScatterToSubworkflow: Boolean,
allowNestedInputs: Boolean,
callables: Map[String, Callable])

def convertGraphElements(a: GraphLikeConvertInputs)
Expand All @@ -84,7 +93,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {

for {
linkedGraph <- LinkedGraphMaker.make(nodes = a.graphElements, seedGeneratedValueHandles ++ finished, typeAliases = a.typeAliases, callables = a.callables)
womGraph <- makeWomGraph(linkedGraph, a.seedNodes, a.externalUpstreamCalls, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables)
womGraph <- makeWomGraph(linkedGraph, a.seedNodes, a.externalUpstreamCalls, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.allowNestedInputs, a.callables)
} yield womGraph
}

Expand All @@ -94,6 +103,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {
workflowName: String,
insideAScatter: Boolean,
convertNestedScatterToSubworkflow : Boolean,
allowNestedInputs: Boolean,
callables: Map[String, Callable])
(implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement],
fileEvaluator: FileEvaluator[ExpressionElement],
Expand All @@ -119,7 +129,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util {

val generatedGraphNodesValidation: ErrorOr[Set[GraphNode]] =
WorkflowGraphElementToGraphNode.convert(
GraphNodeMakerInputs(next, upstreamCallNodes, linkedGraph.consumedValueLookup, availableValues, linkedGraph.typeAliases, workflowName, insideAScatter, convertNestedScatterToSubworkflow, callables))
GraphNodeMakerInputs(next, upstreamCallNodes, linkedGraph.consumedValueLookup, availableValues, linkedGraph.typeAliases, workflowName, insideAScatter, convertNestedScatterToSubworkflow, allowNestedInputs, callables))
generatedGraphNodesValidation map { nextGraphNodes: Set[GraphNode] => currentList ++ nextGraphNodes }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object CallElementToGraphNode {

def supplyableInput(definition: Callable.InputDefinition): Boolean = {
!definition.isInstanceOf[FixedInputDefinitionWithDefault] &&
!definition.name.contains(".") // NB: Remove this check when sub-workflows allow pass-through task inputs
(!definition.name.contains(".") || a.allowNestedInputs)
}

def validInput(name: String, definition: Callable.InputDefinition): Boolean = {
Expand Down Expand Up @@ -224,4 +224,5 @@ case class CallNodeMakerInputs(node: CallElement,
availableTypeAliases: Map[String, WomType],
workflowName: String,
insideAnotherScatter: Boolean,
allowNestedInputs: Boolean,
callables: Map[String, Callable])
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ object IfElementToGraphNode {
val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins, foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName,
insideAScatter = a.insideAnotherScatter,
convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow,
allowNestedInputs = a.allowNestedInputs,
a.callables)
val innerGraph: ErrorOr[Graph] = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs)

Expand All @@ -96,4 +97,5 @@ final case class ConditionalNodeMakerInputs(node: IfElement,
workflowName: String,
insideAnotherScatter: Boolean,
convertNestedScatterToSubworkflow : Boolean,
allowNestedInputs: Boolean,
callables: Map[String, Callable])
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ object ScatterElementToGraphNode {
val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins ++ Set(womInnerGraphScatterVariableInput), foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName,
insideAScatter = true,
convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow,
allowNestedInputs = a.allowNestedInputs,
a.callables)
val innerGraph: ErrorOr[Graph] = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs)

Expand Down Expand Up @@ -131,6 +132,7 @@ object ScatterElementToGraphNode {
val graphLikeConvertInputs = GraphLikeConvertInputs(Set(a.node), subWorkflowInputs, Map.empty, a.availableTypeAliases, a.workflowName,
insideAScatter = false,
convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow,
allowNestedInputs = a.allowNestedInputs,
a.callables)
val subWorkflowGraph = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs)
subWorkflowGraph map { WomGraphMakerTools.addDefaultOutputs(_) }
Expand Down Expand Up @@ -188,4 +190,5 @@ final case class ScatterNodeMakerInputs(node: ScatterElement,
workflowName: String,
insideAnotherScatter: Boolean,
convertNestedScatterToSubworkflow: Boolean,
allowNestedInputs: Boolean,
callables: Map[String, Callable])
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ object WorkflowGraphElementToGraphNode {
result.contextualizeErrors(s"process declaration '${typeElement.toWdlV1} $name = ${expr.toWdlV1}'")

case se: ScatterElement =>
val scatterMakerInputs = ScatterNodeMakerInputs(se, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables)
val scatterMakerInputs = ScatterNodeMakerInputs(se, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.allowNestedInputs, a.callables)
ScatterElementToGraphNode.convert(scatterMakerInputs)

case ie: IfElement =>
val ifMakerInputs = ConditionalNodeMakerInputs(ie, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables)
val ifMakerInputs = ConditionalNodeMakerInputs(ie, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.allowNestedInputs, a.callables)
IfElementToGraphNode.convert(ifMakerInputs)

case ce: CallElement =>
val callNodeMakerInputs = CallNodeMakerInputs(ce, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.callables)
val callNodeMakerInputs = CallNodeMakerInputs(ce, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.allowNestedInputs, a.callables)
CallElementToGraphNode.convert(callNodeMakerInputs)
}

Expand All @@ -81,4 +81,5 @@ final case class GraphNodeMakerInputs(node: WorkflowGraphElement,
workflowName: String,
insideAScatter: Boolean,
convertNestedScatterToSubworkflow : Boolean,
allowNestedInputs: Boolean,
callables: Map[String, Callable])
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
version 1.0

workflow sub_wf {
input {
Int y
}
# Calls foo but doesn't provide an 'x'. That's fine because the input was optional, but now the outer WF cannot override it.
call foo { input: y = y }

# No outputs, but we don't want that to be the error:
output { }
meta {allowNestedInputs: true}
}

task foo {
input {
Int? x
Int y
}
command {
}
output {
Int z = y
}
runtime {
docker: "ubuntu:latest"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"supplied_optional_subwf_sub_inputs.y": 5,
"supplied_optional_subwf_sub_inputs.sub_wf.foo.x": 5
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
version 1.0

import "import_me.wdl"

workflow supplied_optional_subwf_sub_inputs {
input {
Int y
}
# Shouldn't (strictly) be able to call this sub-workflow because the inputs are not passed through
call import_me.sub_wf {input: y=y}
meta {
allowNestedInputs: true
}
}

0 comments on commit aded97f

Please sign in to comment.