Skip to content

Conversation

@manu-chroma
Copy link
Member

@manu-chroma manu-chroma commented Jul 22, 2017

Fixes #341, #140

@tetron
Copy link
Member

tetron commented Jul 22, 2017

If no version number is found it should be a fatal error.

else:
_logger.warning("No cwlVersion found, treating this file as draft-2.")
workflowobj["cwlVersion"] = "draft-2"
_logger.warning("No cwlVersion found, treating this file as v1.0")
Copy link
Member Author

Choose a reason for hiding this comment

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

@tetron No such fallback should be provided?
#482 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, instead of falling back it should be a fatal error. We don't want to encourage people to start writing files cwl where we can't identify the version, it makes spec evolution much harder.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Done 👍

@manu-chroma
Copy link
Member Author

testing on the example @FarahZKhan mentioned #482 (comment)

(venv3) [venv3] $ cwltool --pack hello-param.cwl params.yaml
/home/manu/github/cwltool/venv3/bin/cwltool 1.0.20170717092913
Resolved 'hello-param.cwl' to 'file:///home/manu/github/test_cwl_runner/workflows/workflows/hello/hello-param.cwl'
Tool definition failed validation:
No cwlVersion found. Use the following syntax in your CWL workflow to declare version: cwlVersion: <version>

@manu-chroma manu-chroma changed the title load_tool: when no version is found, fallback to cwlVersion v1.0 load_tool: when no version is found, create fatal error Jul 22, 2017
@manu-chroma manu-chroma changed the title load_tool: when no version is found, create fatal error load_tool: when no version or incorrect cwlVersion is found, create fatal error Jul 23, 2017
@manu-chroma
Copy link
Member Author

When using cwlVersion: v0.1 in the workflow.

(venv3) [venv3] ~/github/cwltool/tests · (version_fix±) $ cwltool wf/wrong_cwlVersion.cwl                     23:10:22
/home/manu/github/cwltool/venv3/bin/cwltool 1.0.20170723153038
Resolved 'wf/wrong_cwlVersion.cwl' to 'file:///home/manu/github/cwltool/tests/wf/wrong_cwlVersion.cwl'
Tool definition failed validation:
'cwlVersion' not valid. Supported CWL versions are: 
draft-2
draft-3
draft-3.dev1
draft-3.dev2
draft-3.dev3
draft-3.dev4
draft-3.dev5
draft-4.dev1
draft-4.dev2
draft-4.dev3
v1.0
v1.0.dev4
v1.1.0-dev1

@manu-chroma
Copy link
Member Author

Also, added tests :)

@manu-chroma manu-chroma requested review from mr-c and tetron July 23, 2017 17:45
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

I approve, though this does not fix #482 (that can happen in another PR) -- thanks!

@mr-c mr-c merged commit 63db4dd into master Jul 24, 2017
@mr-c mr-c deleted the version_fix branch July 24, 2017 12:06
@manu-chroma
Copy link
Member Author

With this PR merged:
cwlVersion: cwl:draft-3 throws validation exception.
cwlVersion: draft-3 works as expected.

Is this behavior acceptable?

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