common-workflow-language / cwltool

Common Workflow Language reference implementation
https://cwltool.readthedocs.io/
Apache License 2.0
336 stars 231 forks source link

"paths starting with '/' only permitted in CWL 1.2 and later" error message needs to be more helpful #1356

Closed alexiswl closed 3 years ago

alexiswl commented 4 years ago

Intro

I am getting this error:

ERROR Workflow error, try again with --debug for more information:
createfile.cwl:18:5: Name '/scripts/myscript.sh' at index 0 of listing is invalid, paths starting
                     with '/' only permitted in CWL 1.2 and later

yet mounting an absolute path also works in previous versions of CWL.
By upgrading cwltool, I no longer have backwards compatibility for this tool.

The following cwltool completes successfully using cwltool version: 3.0.20200709181526
but fails using version: 3.0.20200807132242.

Commandline Tool

class: CommandLineTool
cwlVersion: v1.1

# Based on
# https://github.com/common-workflow-language/cwltool/blob/639229b1159cf484e70e52da10194561b3fad719/cwltool/schemas/v1.0/examples/createfile.cwl

inputs:
  message:
    type: string

baseCommand: ["sh", "/scripts/myscript.sh"]

requirements:
  DockerRequirement:
    dockerPull: 'ubuntu:latest'
  InlineJavascriptRequirement: {}
  InitialWorkDirRequirement:
    listing:
      - entryname: /scripts/myscript.sh
        entry: |
          echo "Start of message" && \
          echo "My message: $(inputs.message)" && \
          echo "End of message"

stdout: "message.txt"

outputs:
  message:
    type: stdout

Input

message: a_string

Expected behaviour:

As available in 3.0.20200709181526

$ cwltool createfile.cwl createfile.input.yaml
INFO /home/alexiswl/anaconda3/envs/toil/bin/cwltool 3.0.20200709181526
INFO Resolved 'createfile.cwl' to 'file:///c/Users/awluc/OneDrive/GitHub/UMCCR-ILLUMINA/cwl-iap/test/createfile/createfile.cwl'
createfile.cwl:8:3: object id `createfile.cwl#message` previously defined
INFO [job createfile.cwl] /tmp/dstghge_$ docker \
    run \
    -i \
    --mount=type=bind,source=/tmp/dstghge_,target=/UagLqk \
    --mount=type=bind,source=/tmp/r6x1dqso,target=/tmp \
    --mount=type=bind,source=/tmp/11rcio5o/myscript.sh,target=/scripts/myscript.sh,readonly \
    --workdir=/UagLqk \
    --read-only=true \
    --net=none \
    --log-driver=none \
    --user=1000:1000 \
    --rm \
    --env=TMPDIR=/tmp \
    --env=HOME=/UagLqk \
    --cidfile=/tmp/rct04itp/20201022172135-699623.cid \
    ubuntu:latest \
    sh \
    /scripts/myscript.sh > /tmp/dstghge_/message.txt
INFO [job createfile.cwl] Max memory used: 0MiB
INFO [job createfile.cwl] completed success
{
    "message": {
        "location": "file:///c/Users/awluc/OneDrive/GitHub/UMCCR-ILLUMINA/cwl-iap/test/createfile/message.txt",
        "basename": "message.txt",
        "class": "File",
        "checksum": "sha1$20643771ea01e2ac9082cc8e7da093a8d1d3ec6b",
        "size": 53,
        "path": "/c/Users/awluc/OneDrive/GitHub/UMCCR-ILLUMINA/cwl-iap/test/createfile/message.txt"
    }
}
INFO Final process status is success

Actual Behavior

Tell us what happens instead

$ cwltool createfile.cwl createfile.input.yaml
INFO /home/alexiswl/anaconda3/envs/cwl/bin/cwltool 3.0.20200807132242
INFO Resolved 'createfile.cwl' to 'file:///c/Users/awluc/OneDrive/GitHub/UMCCR-ILLUMINA/cwl-iap/test/createfile/createfile.cwl'
createfile.cwl:8:3: object id `createfile.cwl#message` previously defined
ERROR Workflow error, try again with --debug for more information:
createfile.cwl:18:5: Name '/scripts/myscript.sh' at index 0 of listing is invalid, paths starting
                     with '/' only permitted in CWL 1.2 and later

Full Traceback

Traceback (most recent call last):
  File "/home/alexiswl/anaconda3/envs/cwl/lib/python3.8/site-packages/cwltool/main.py", line 1131, in main
    (out, status) = real_executor(
  File "/home/alexiswl/anaconda3/envs/cwl/lib/python3.8/site-packages/cwltool/executors.py", line 59, in __call__
    return self.execute(process, job_order_object, runtime_context, logger)
  File "/home/alexiswl/anaconda3/envs/cwl/lib/python3.8/site-packages/cwltool/executors.py", line 150, in execute
    self.run_jobs(process, job_order_object, logger, runtime_context)
  File "/home/alexiswl/anaconda3/envs/cwl/lib/python3.8/site-packages/cwltool/executors.py", line 234, in run_jobs
    for job in jobiter:
  File "/home/alexiswl/anaconda3/envs/cwl/lib/python3.8/site-packages/cwltool/command_line_tool.py", line 889, in job
    self._initialworkdir(j, builder)
  File "/home/alexiswl/anaconda3/envs/cwl/lib/python3.8/site-packages/cwltool/command_line_tool.py", line 637, in _initialworkdir
    raise SourceLine(
cwltool.errors.WorkflowException: createfile.cwl:18:5: Name '/scripts/myscript.sh' at index 0 of listing is invalid, paths starting with '/' only permitte
d in CWL 1.2 and later

Your Environment

mr-c commented 3 years ago

Thank you @alexiswl for your issue. This is due to us clarifying a behaviour that is now explicitly allowed in CWL version 1.2

If you change your cwlVersion to v1.2 then this will work.

alexiswl commented 3 years ago

If you change your cwlVersion to v1.2 then this will work.

If the endpoint to supports v1.2? I'm sure my local machine does where I've tested, but that doesn't mean I can upload them to a service running an older cwltool version. I've worked around most cases for now by just removing the slash and having the dirent in the working directory.

alexiswl commented 3 years ago

This will also break users' workflows on an endpoint if the endpoint updates its cwltool version.

Could this error not be a warning instead?

mr-c commented 3 years ago

If you change your cwlVersion to v1.2 then this will work.

If the endpoint to supports v1.2?

Yes, there is a conformance test for CWL v1.2 that confirms that paths starting with / are allowed as long as there is a DockerReqirement under requirements as per the requirements in CWL v1.2 Dirent.entryname. As far as we know, older versions of cwltool were the only CWL runners that supported paths starting with /, so your CommandLineTool would likely not have worked with any other CWL aware workflow runner.

So this was never allowed in CWL v1.0 or v1.1 and only accidentally worked sometimes in cwltool due to a bug,

As cwltool is the reference CWL runner, and thus sets the expectation that if a workflow works with cwltool then that workfow should work on any other CWL-aware workflow system. So it is important that we fixed this bug in cwltool. Unfortunately this bug was a feature you were relying on.

As cwltool is a reference runner, and not meant to be a production runner, we strongly suggest that any users of cwltool pin a particular version.

I've worked around most cases for now by just removing the slash and having the dirent in the working directory.

Glad you've been able to find a better solution. That is more flexible anyhow! Makes it easier to switch to another software container.

alexiswl commented 3 years ago

As far as we know, older versions of cwltool were the only CWL runners that supported paths starting with /, so your CommandLineTool would likely not have worked with any other CWL aware workflow runner.

But this includes Toil since it calls cwltool right? And my experience with an Airflow/Kubernetes platform also supported it.

Which is why I thought a warning would be more appropriate than an error.

mr-c commented 3 years ago

As far as we know, older versions of cwltool were the only CWL runners that supported paths starting with /, so your CommandLineTool would likely not have worked with any other CWL aware workflow runner.

But this includes Toil since it calls cwltool right?

Depends, we'd have to check the exact versions of cwltool that toil-cwl-runner pinned in different releases.

Regardless, there are many CWL implementations that don't use cwltool or don't use the part of cwltool that had this accidental feature.

Which is why I thought a warning would be more appropriate than an error.

I understand this could hurt a bit, and we are sorry for that.

twigleingrid commented 3 years ago

I could fix that. What might be a better error message?

mr-c commented 3 years ago

Thanks for volunteering @twigleingrid

A better error message might be

Name '/scripts/myscript.sh' at index 0 of listing is invalid, paths starting with '/' are only permitted in CWL 1.2 and later. Consider changing the absolute path to a relative path, or upgrade the CWL description to CWL v1.2 using https://pypi.org/project/cwl-upgrader/