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

Specify that runtime attributes can be declared as inputs #313

Closed
wants to merge 1 commit into from

Conversation

geoffjentry
Copy link
Member

This PR replaces #301. The primary difference is that this PR only refers to things which are already part of the spec, i.e. references to input json format remain while references to input json files have been elided

@illusional
Copy link
Contributor

+1 for this PR :)

@@ -2647,6 +2648,23 @@ In JSON, the inputs to the workflow in the previous section might be:

It's important to note that the type in JSON must be coercible to the WDL type. For example `wf.int_val` expects an integer, but if we specified it in JSON as `"wf.int_val": "three"`, this coercion from string to integer is not valid and would result in a coercion error. See the section on [Type Coercion](#type-coercion) for more details.

## Specifying / Overriding Runtime Attributes in JSON
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sub JSON for Inputs?

@cjllanwarne
Copy link
Contributor

cjllanwarne commented May 30, 2019

One thing worth making explicit one way or the other, do we want to consider these to be providable as call inputs too? ie something like...

call foo { input:
  runtime.docker: "ubuntu: latest"
}

My gut feeling is yes but I could see an argument for no

@pshapiro4broad
Copy link
Contributor

pshapiro4broad commented Jun 3, 2019

One thing worth making explicit one way or the other, do we want to consider these to be providable as call inputs too? ie something like...

call foo { input:
  runtime.docker: "ubuntu: latest"
}

My gut feeling is yes but I could see an argument for no

I think it should be supported, in the interest of keeping the WDL spec consistent.

@patmagee
Copy link
Member

patmagee commented Jun 3, 2019

In light of #315, I wonder if we should revist this?

@pshapiro4broad
Copy link
Contributor

pshapiro4broad commented Jun 4, 2019

In light of #315, I wonder if we should revist this?

Unless I'm misunderstanding your comment, I don't think it's required. There will be cases when PR overlap, but I don't think it's necessary to block or update one based on the other, since neither are yet part of the spec. In this case I think the PRs are orthogonal anyway; e.g. regardless of what changes are made to the contents of the runtime block, the topic of a how a user can provide key/value pairs for that block can be discussed.

@geoffjentry
Copy link
Member Author

Sorry this is what being out of town gets you. On the flip side, you should see the view I have while typing this :)

@patmagee I was going to suggest folding in the spirit of my PR into this, if that's not already done.

patmagee added a commit to patmagee/wdl that referenced this pull request Jul 19, 2019
patmagee added a commit to patmagee/wdl that referenced this pull request Jul 19, 2019
@patmagee
Copy link
Member

Closing this as per @geoffjentry suggestion. Changes have been folded into the #315

@patmagee patmagee closed this Jul 19, 2019
@jdidion jdidion deleted the jg_runtime_overrides branch June 29, 2020 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants