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

Allow relative imports from URL-submitted CWL #4308

Merged
merged 13 commits into from Nov 16, 2018

Conversation

cjllanwarne
Copy link
Contributor

@cjllanwarne cjllanwarne commented Oct 24, 2018


def parse(): Parse[Cwl] = source.workflowUrl match {
case Some(url) => for {
reference <- fromEither[IO](CwlReference.fromString(url).map(Right(_)).getOrElse(Left(NonEmptyList.one(s"Invalid workflow reference: $url")))): Parse[CwlReference]
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like somewhere in all this abstraction we must be making an HTTP requests to actually get the url. How is it doing that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually all this goes into a call to cwltool, which accepts commands in the form of cwltool --print-pre URL and gives us back an appropriate CWL string with any relative paths inside converted into fully-qualified references for us.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah OK, I was hunting around in our Scala code for this but it makes total sense that we're handing it off

commandResult.exitCode match {
case 0 => commandResult.out.string
case error =>
throw new RuntimeException(
s"running CwlTool on file $file resulted in exit code $error and stderr ${commandResult.err.string}")
s"running CwlTool on file $reference resulted in exit code $error and stderr ${commandResult.err.string}")
Copy link
Contributor

Choose a reason for hiding this comment

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

ToL: Line 65 below uses ${reference.pathAsString} instead of $reference. Not sure if it matters anymore, but just noticed there was a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, and I've updated this.

reference.toString includes the "which tool/workflow in the file to look at" information, which is not relevant for a message about an unreadable file.

The process of parsing CWL runs approximately like this:

1. Produce a canonical one-file representation of the CWL as a JSON object in memory
1. Use Circe to turn that representation into scala case classes

This comment was marked as resolved.


## Convert the case classes into WOM representations

As of today, CWL doesn't produce WOM bundles. Instead, it goes straight to executable.

This comment was marked as resolved.

@cjllanwarne cjllanwarne merged commit 448067f into develop Nov 16, 2018
@cjllanwarne cjllanwarne mentioned this pull request Nov 16, 2018
1 task
@cjllanwarne cjllanwarne deleted the cjl_cwl_relative_imports branch November 16, 2018 17:45
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.

None yet

4 participants