Skip to content

Implement targeted "overrides" of requirements on specific tools #440

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

Merged
merged 18 commits into from
Nov 21, 2017

Conversation

tetron
Copy link
Member

@tetron tetron commented Jun 24, 2017

You can provide an "overrides" section in your document:

cwltool revsort.cwl revsort-ovr-job.json

{
  "input": {
    "class": "File",
    "location": "whale.txt"
  },
    "cwltool:overrides": {
        "revtool.cwl": [{
            "class": "DockerRequirement",
            "dockerPull": "ubuntu:14.04"
        }]
    }
}

You can also use --overrides to pull in just the overrides section of a document:

cwltool --overrides revsort-ovr-job.json revsort.cwl revsort-job.json

    "cwltool:overrides": {
        "revtool.cwl": [{
            "class": "DockerRequirement",
            "dockerPull": "ubuntu:14.04"
        }]
    }

Currently only works on leaf nodes (CommandLineTool) but intended to generalize to workflows / workflow steps.

@tetron
Copy link
Member Author

tetron commented Jun 27, 2017

So the current implementation works by overriding the behavior of @get_requirement@, but this only works for leaf nodes and not for overrides applied to workflows or workflow steps.

So either:

  1. Apply overrides at load time (this is where the requirements list gets built, so all the changes related to @get_requirement@ can be reverted)
  2. Compute requirements at runtime, then apply overrides (requires refactoring)

The benefit of the 2nd approach is that you could, in principal, run multiple instances of the same workflow in the same process, but with different overrides, without loading multiple copies of the workflow/tool files. But it isn't clear we need that right now, and the refactoring is kind of annoying and will make this feature take longer.

@mr-c
Copy link
Member

mr-c commented Jun 27, 2017

@tetron Option two will be useful later -- we've had request for the equivalent of scattering over requirements, for example.

Does the current syntax & option one allow for overriding a specific (sub)step (not to be confused with a specific CommandLineTool) ?

@tetron
Copy link
Member Author

tetron commented Jun 27, 2017

Yea, I was afraid you'd say that. I agree that it's better to apply it at runtime, just more work. But probably better to do it right than do it quick.

I hope to incorporate overrides into a future spec revision. That's an interesting idea, it could allow an expression to support the scatter case.

@mr-c
Copy link
Member

mr-c commented Jun 27, 2017

Well, I'm not saying no to option 1 -- as long as it is forwards compatible with option 2 :-)

@tetron
Copy link
Member Author

tetron commented Jul 11, 2017

Implementation of proposal common-workflow-language/common-workflow-language#210

@tetron tetron force-pushed the requirement-overrides branch from 34bef35 to 36032fa Compare November 17, 2017 03:29
underlying workflow, cwltool supports requirement "overrides".

The format of the "overrides" object is a mapping of item identifier (workflow,
workflow step, or command line tool) followed by a list of ProcessRequirements
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an example of an identifier for a workflow step?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added some discussion. Also made this more complex (but hopefully more useful) by resolving workflow ids relative to the workflow, and everything else relative to the job/--overrides document.

@tetron tetron force-pushed the requirement-overrides branch from 47d6fdd to f48d3c7 Compare November 21, 2017 18:03
@tetron tetron force-pushed the requirement-overrides branch from f48d3c7 to 0815a40 Compare November 21, 2017 18:09
@tetron tetron force-pushed the requirement-overrides branch from af8178e to 53dd06f Compare November 21, 2017 21:16
@tetron tetron merged commit c63e4a5 into master Nov 21, 2017
@tetron tetron deleted the requirement-overrides branch November 21, 2017 21:35
anton-khodak pushed a commit that referenced this pull request Nov 22, 2017
* Apply requirement overrides at load time

* Override identifiers are relative to workflow, other identifiers are relative
to job document.

* Update documentation
uri = urllib.parse.urljoin(uri, workflowobj["https://w3id.org/cwl/cwl#tool"])
del cast(dict, jobobj)["https://w3id.org/cwl/cwl#tool"]

if "http://commonwl.org/cwltool#overrides" in jobobj:
Copy link
Member

Choose a reason for hiding this comment

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

I think this is over-indented

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.

2 participants