CWL document resolver via GA4GH tool API #201

denis-yuen · 2016-09-30T20:21:29Z

This allows cwltool to launch tools from https://dockstore.org/ in one of two ways

cwltool --non-strict quay.io/collaboratory/dockstore-tool-bamstats:master test.json

and

# defaults to latest when a version is not specified
cwltool --non-strict quay.io/collaboratory/dockstore-tool-bamstats test.json

Grab the test.json (and input file) from https://github.com/CancerCollaboratory/dockstore-tool-bamstats

denis-yuen · 2016-09-30T21:36:18Z

@tetron FYI, looks like something has gone wrong stylistically with the merge from master

mr-c · 2016-10-03T15:46:30Z

@denis-yuen The conformance tests are failing: https://ci.commonwl.org/job/cwltool-pr-conformance/275/console

denis-yuen · 2016-10-03T15:50:17Z

@mr-c Yeah, just got travis satisfied. Looks like Jenkins is next if I can figure this out.

mr-c · 2016-10-05T16:08:35Z

Looks useful to me, shall I update the branch and merge?

mr-c · 2016-10-05T16:09:39Z

@denis-yuen FYI, I think you have permissions to create branches on this repo directly

denis-yuen · 2016-10-05T16:16:00Z

@mr-c Ah ok, thanks.
Feel free to update and merge/squash

tetron · 2016-10-14T01:43:31Z

Hi @denis-yuen can you update this PR?

denis-yuen · 2016-10-14T15:08:37Z

@tetron FYI, done

denis-yuen · 2016-10-18T16:52:29Z

@tetron @mr-c Hi, not sure what is the next step with this. Just give me a heads-up if more is needed on my side.

mr-c · 2016-10-18T17:27:29Z

Hey @denis-yuen thank you for this contribution!

The documentation at the top should be added to the readme as a general section about resolving the location of referenced documents, with a link to http://www.commonwl.org/v1.0/CommandLineTool.html#Discovering_CWL_documents_on_a_local_filesystem

Specifically it must be clearly explained that the reference implementation will try to contact dockstore.org if the document can't be found locally; likewise there should be a switch to skip dockstore.org resolution for privacy preservation.

Also, cwltool/resolver.py is missing PEP 484 type signatures for the methods.

docs
opt-out switch
PEP 484 type signatures

denis-yuen · 2016-10-27T15:23:17Z

FYI @tetron , I may need you (or someone more pythony) to finish off this since my Python knowledge is lacking.
It looks like the immediate issue is related to this python/mypy#1141 . In addition, there's probably a more elegant way of passing the opt-out parameter.

mr-c · 2016-10-27T15:48:25Z

@denis-yuen I'll fix the types for you

mr-c · 2016-11-10T09:35:35Z

@denis-yuen A few questions:

Can you help me understand the value in a dockstore.org specific resolver when we already support any HTTP[S] IRI?

What's the benefit of typing:

cwltool --non-strict https://raw.githubusercontent.com/CancerCollaboratory/dockstore-tool-bamstats/develop/Dockstore.cwl test.json

(which works today, and won't need the --non-strict once my PR is merged)
over

cwltool --non-strict quay.io/collaboratory/dockstore-tool-bamstats:master test.json

?

As an end user I'm not likely to want to type quay.io/collaboratory/dockstore-tool-bamstats:master from memory, so if we stay in the land of copy and paste then a regular URL/URI/IRI seems fine.

As a shortcut, the user's input object (which really should be YAML not JSON for readability) can include a cwl:tool entry pointing to the remote location of the tool or workflow. So maybe the docstore.org website or tool can generate a skelton input object YAML with that prefilled?

denis-yuen · 2016-11-10T14:54:36Z

@mr-c
Well, the alternative to quay.io/collaboratory/dockstore-tool-bamstats:master would be something more like https://dockstore.org:8443/api/ga4gh/v1/tools/quay.io/collaboratory/dockstore-tool-bamstats/versions/master/plain-CWL/descriptor. https://raw.githubusercontent.com/CancerCollaboratory/dockstore-tool-bamstats/develop/Dockstore.cwlis different in the sense that its pointing to the GitHub repo where Dockstore sources its content and is pointing at GitHub's develop rather than dockstore/quay.io's docker tag.

As for whether copy-and-paste is sufficient, I think @tetron 's intent from the original commit was that this is more meant for ease-of-use, so that someone can go to the dockstore site and copy just quay.io/collaboratory/dockstore-tool-bamstats:master rather than the long URL above.

Oh, I was not aware of that last short-cut. Could you point me to an example in the user guide or similar?

mr-c · 2016-11-10T15:49:56Z

cwl:tool is described at http://www.commonwl.org/v1.0/CommandLineTool.html#Executing_CWL_documents_as_scripts

mr-c · 2016-11-11T11:47:44Z

~~@denis-yuen To keep the code cleaner and more maintainable, and to be fair to other registries, I suggest the following:~~
~~- [ ] @mr-c adds support for resolver plugins using the cwltool.resolver entry point~~
~~- [ ] @denis-yuen and/or others implement their additional resolvers and advertise them using the cwltool.resolver entry point~~
~~- [ ] Once the above is working and tested, dependencies will be added to cwltool as named extras so that users can install them easier. Example: pip install cwltool[docstore,bioshadock]~~

This has been moved to #314

mrc · 2016-11-16T04:01:46Z

I reckon I've been CC'd here by mistake, which means you might be missing a
review from someone.

On Tue, Nov 15, 2016 at 3:20 AM, Common Workflow Language project bot <
[email protected]> wrote:

Can one of the admins verify this patch?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#201 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAB7aQFNO6p5AcYUjc6Rn_Si9Wg_ESqyks5q-ZVjgaJpZM4KLedJ
.

mr-c · 2016-11-16T08:40:39Z

@mrc Sorry, typo'd my own username :-)

denis-yuen · 2016-11-16T15:22:04Z

@mr-c FYI, sounds reasonable, but it may take some time before I can get to it.

mrc · 2016-11-16T15:45:07Z

Hehe no worries.

On Wed, Nov 16, 2016 at 12:40 AM, Michael R. Crusoe <
[email protected]> wrote:

@mrc https://github.com/mrc Sorry, typo'd my own username :-)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#201 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAB7aWbNhFY9PNhEXJqUw2rfNpnT85U0ks5q-sGIgaJpZM4KLedJ
.

mr-c

@tetron
Since the GA4GH tool registry API is getting more stable, is your thinking that we should just merge this w/o a resolver plugin as discussed in #201 (comment) ?

tetron · 2017-03-01T21:35:54Z

@mr-c I don't object to implementing a proper plugin framework. That would provide a much better solution to configuring the various extension points than the ad-hoc current approach. However I have limited bandwidth so my personal preference would be to go ahead and merge the feature as-is and refactor later.

Do you have any experience / opinions on plugin frameworks for Python that we can use?

mr-c · 2017-03-02T08:55:54Z

README.rst

@@ -86,3 +86,16 @@ documents using absolute or relative local filesytem paths. If a relative path
 is referenced and that document isn't found in the current directory then the
 following locations will be searched:
 http://www.commonwl.org/v1.0/CommandLineTool.html#Discovering_CWL_documents_on_a_local_filesystem
+
+Use with Dockstore


All references to Dockstore need to be replaced with "the GA4GH Tool Registry API"

At some point there should be a link to https://github.com/ga4gh/tool-registry-schemas#what-is-the-tool-registry-api-schema

Dockstore.org should still be mentioned as it is the default backend

mr-c · 2017-03-02T09:01:53Z

cwltool/main.py

+                        dest="enable_tool_registry", default=True)
+
+    parser.add_argument("--add-tool-registry", action="append", help="Add a tool registry to use for resolution, default %s" % tool_registries,
+                        dest="tool_registries", default=[])


The help text for all three of theses needs to mention what kind of tool registry; in this case it is "a GA4GH compliant tool registry"

See ga4gh/tool-registry-service-schemas#17 for a discussion on this

mr-c · 2017-03-02T09:04:43Z

@tetron Sure, we can merge as soon as the docs and help for the new command line flags in the PR are brought up to date with the latest changes (which center the GA4GH API and use Dockstore as the exemplar default)

As for plugins, I've expanded my comment above into #314

…ol-resolver

…ries.

Conflicts: cwltool/main.py

Conflicts: README.rst

tetron · 2017-03-15T20:40:07Z

I think this branch is well baked at this point, so I'm going to merge it at the next opportunity.

mr-c changed the title ~~Tool resolver~~ CWL document resolver via local filesystem, dockstore.org Oct 24, 2016

mr-c mentioned this pull request Nov 10, 2016

Local resolver #224

Merged

Peter Amstutz and others added 8 commits March 1, 2017 10:11

Support Dockstore lookups for tools.

999ba7e

Point at production site and allow for specification of tool versions

7ceb4e5

Formatting changes

be08705

Remove print statement breaking tests

7d6cad7

Update README.rst to add reference to discovery mechanism

050078a

Add --enable/disable-tool-registry and --add-tool-registry

522943b

Fix

5edd681

Fix mypy

05b6efd

mr-c requested changes Mar 1, 2017

View reviewed changes

mr-c mentioned this pull request Mar 2, 2017

support plugins for third party CWL registry APIs, etc #314

Open

8 tasks

mr-c reviewed Mar 2, 2017

View reviewed changes

tetron and others added 2 commits March 2, 2017 16:56

Merge branch 'master' into tool-resolver

38c609d

Update "Use with Dockstore" to "Use with GA4GH Tool Registry API"

5d70973

tetron changed the title ~~CWL document resolver via local filesystem, dockstore.org~~ CWL tool resolver via dockstore.org Mar 2, 2017

tetron changed the title ~~CWL tool resolver via dockstore.org~~ CWL document resolver via GA4GH tool API Mar 2, 2017

Peter Amstutz added 2 commits March 2, 2017 17:06

Merge branch 'tool-resolver' of github.com:denis-yuen/cwltool into to…

4f3e7d5

…ol-resolver

Change tool_registries symbols to be explicit about ga4gh_tool_regist…

621946e

…ries.

mr-c approved these changes Mar 3, 2017

View reviewed changes

mr-c and others added 5 commits March 3, 2017 13:24

format fix

7a6dd2e

Merge branch 'master' into tool-resolver

45144e6

Merge branch 'master' into denis-yuen-tool-resolver

64100b9

Conflicts: cwltool/main.py

Fix pylint.

a9a4a51

Merge branch 'master' into denis-yuen-tool-resolver

4728d49

Conflicts: README.rst

jmchilton mentioned this pull request Mar 8, 2017

Tool Resolvers Schema galaxyproject/planemo#654

Closed

Merge branch 'master' into tool-resolver

ac6d9d5

tetron merged commit d0f9d40 into common-workflow-language:master Mar 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CWL document resolver via GA4GH tool API #201

CWL document resolver via GA4GH tool API #201

denis-yuen commented Sep 30, 2016

denis-yuen commented Sep 30, 2016

mr-c commented Oct 3, 2016

denis-yuen commented Oct 3, 2016

mr-c commented Oct 5, 2016

mr-c commented Oct 5, 2016

denis-yuen commented Oct 5, 2016

tetron commented Oct 14, 2016

denis-yuen commented Oct 14, 2016

denis-yuen commented Oct 18, 2016

mr-c commented Oct 18, 2016 •

edited

Loading

denis-yuen commented Oct 27, 2016 •

edited

Loading

mr-c commented Oct 27, 2016

mr-c commented Nov 10, 2016

denis-yuen commented Nov 10, 2016

mr-c commented Nov 10, 2016

mr-c commented Nov 11, 2016 •

edited

Loading

mrc commented Nov 16, 2016

mr-c commented Nov 16, 2016

denis-yuen commented Nov 16, 2016

mrc commented Nov 16, 2016

mr-c left a comment

tetron commented Mar 1, 2017

mr-c Mar 2, 2017

mr-c Mar 2, 2017

mr-c commented Mar 2, 2017

tetron commented Mar 15, 2017

CWL document resolver via GA4GH tool API #201

CWL document resolver via GA4GH tool API #201

Conversation

denis-yuen commented Sep 30, 2016

denis-yuen commented Sep 30, 2016

mr-c commented Oct 3, 2016

denis-yuen commented Oct 3, 2016

mr-c commented Oct 5, 2016

mr-c commented Oct 5, 2016

denis-yuen commented Oct 5, 2016

tetron commented Oct 14, 2016

denis-yuen commented Oct 14, 2016

denis-yuen commented Oct 18, 2016

mr-c commented Oct 18, 2016 • edited Loading

denis-yuen commented Oct 27, 2016 • edited Loading

mr-c commented Oct 27, 2016

mr-c commented Nov 10, 2016

denis-yuen commented Nov 10, 2016

mr-c commented Nov 10, 2016

mr-c commented Nov 11, 2016 • edited Loading

mrc commented Nov 16, 2016

mr-c commented Nov 16, 2016

denis-yuen commented Nov 16, 2016

mrc commented Nov 16, 2016

mr-c left a comment

Choose a reason for hiding this comment

tetron commented Mar 1, 2017

mr-c Mar 2, 2017

Choose a reason for hiding this comment

mr-c Mar 2, 2017

Choose a reason for hiding this comment

mr-c commented Mar 2, 2017

tetron commented Mar 15, 2017

mr-c commented Oct 18, 2016 •

edited

Loading

denis-yuen commented Oct 27, 2016 •

edited

Loading

mr-c commented Nov 11, 2016 •

edited

Loading