Allow nested inputs when corresponding meta setting is set to true #5562
Conversation
This implements part of the spec change that handles inputs openwdl/wdl#359
rhpvorderman
added
Comply with language spec
WDL
Community Contribution
|
Thanks for the updated PR, we will take a look (should we close #5523?) |
|
@aednichols #5523 can be closed if this is merged. Maybe keep the branch around in case we want to adhere to the spec for WDL 2.0? |
| val allowNestedInputs: Boolean = { | ||
| meta.get("allowNestedInputs").flatMap { | ||
| case x: MetaValueElementBoolean => Option(x.value) | ||
| case _ => None |
There was a problem hiding this comment.
TOL (Talking Out Loud, not gating approval of this PR): Is it the case that specifying
meta {allowNestedInputs: "I like turtles"}
would silently set allowNestedInputs to false? Should there be a warning or error if the wrong type (something other than MetaValueElementBoolean) was specified?
There was a problem hiding this comment.
meta {allowNestedInputs: "I like turtles"} Will indeed set allowNestedInputs to false. I see no problem here, as it is clearly not the intention of the WDL writer to enable a feature.
Booleans are always true or false. I do not think extra code is needed to evaluate cases such as "true", "false", 0, 1 etc. If people start complaining at this behavior we can easily change it later. For now, I see no reason for the extra effort and code complexity (also I am lazy 😉).
|
@rhpvorderman this is good to merge, you can do it yourself or we will do it for you if we don't hear back by, say, Thursday evening |
|
Thanks all for the reviews! |
This implements part of the spec change that handles inputs openwdl/wdl#359
A rewrite from #5523 .
This foregoes the restriction that the specs requires that all required inputs should be set at the top-level workflow.
It does enable using bubbled up inputs when the meta flag is set.
The restrictions should be enabled on biscayne. But I have no idea how to implement that in this code base. Some help is needed. For now implementing this in the new base at least implements part of the spec.