Feature/dock 2353/hello world workflow fails #236

fhembroff · Apr 28, 2023

Description
This PR allows you to launch CWL workflows/tools that do not have a test parameter file.

Currently, if you try and launch a CWL tool or workflow without a test parameter file, you get this error:

Created temporary working directory at '/var/folders/nr/dkx1p38s24nb7fx_qgcxggcm0000gv/T/1678817449367-0'
Using local file 'foo.cwl' as primary descriptor
Calling out to cwltool to run your workflow
Executing: cwltool --enable-dev --non-strict --outdir null/outputs/ --tmpdir-prefix null/tmp/ --tmp-outdir-prefix null/working/ /Users/charlesoverbeck/temp/foo.cwl
INFO /Users/charlesoverbeck/.local/share/virtualenvs/python3.9.4/bin/cwltool 3.0.20200807132242
INFO Resolved '/Users/charlesoverbeck/temp/foo.cwl' to 'file:///Users/charlesoverbeck/temp/foo.cwl'
INFO [job foo.cwl] /Users/charlesoverbeck/temp/null/working/1nli99x9$ echo \
    'Hello World'
Hello World
INFO [job foo.cwl] completed success
{}
INFO Final process status is success
cwltool exit code: 0
cwltool stdout:
{}

cwltool stderr:
INFO /Users/charlesoverbeck/.local/share/virtualenvs/python3.9.4/bin/cwltool 3.0.20200807132242
INFO Resolved '/Users/charlesoverbeck/temp/foo.cwl' to 'file:///Users/charlesoverbeck/temp/foo.cwl'
INFO [job foo.cwl] /Users/charlesoverbeck/temp/null/working/1nli99x9$ echo \
    'Hello World'
Hello World
INFO [job foo.cwl] completed success
INFO Final process status is success

Saving copy of cwltool stdout to: /Users/charlesoverbeck/temp/null/outputs/cwltool.stdout.txt
Saving copy of cwltool stderr to: /Users/charlesoverbeck/temp/null/outputs/cwltool.stderr.txt
11:10:50.642 [main] ERROR io.dockstore.client.cli.ArgumentUtility - Cannot invoke "java.util.Map.size()" because "this.outputMap" is null
11:10:50.644 [main] ERROR io.dockstore.client.cli.ArgumentUtility - java.lang.NullPointerException: Cannot invoke "java.util.Map.size()" because "this.outputMap" is null
	at io.dockstore.client.cli.nested.CwltoolLauncher.provisionOutputFiles(CwltoolLauncher.java:68)
	at io.github.collaboratory.cwl.CWLClient.provisionOutputFiles(CWLClient.java:265)
	at io.dockstore.client.cli.nested.BaseLanguageClient.launchPipeline(BaseLanguageClient.java:207)
	at io.github.collaboratory.cwl.CWLClient.launch(CWLClient.java:292)
	at io.dockstore.client.cli.nested.AbstractEntryClient.launchCwl(AbstractEntryClient.java:1429)
	at io.dockstore.client.cli.nested.AbstractEntryClient.checkEntryFile(AbstractEntryClient.java:886)
	at io.dockstore.client.cli.nested.AbstractEntryClient.launch(AbstractEntryClient.java:1378)
	at io.dockstore.client.cli.nested.AbstractEntryClient.processEntryCommands(AbstractEntryClient.java:312)
	at io.dockstore.client.cli.Client.run(Client.java:737)
	at io.dockstore.client.cli.Client.main(Client.java:633)

With this PR, no error occurs.

What was causing this error?

This error was occurring because the variable outputMap is only initialized when this line is called, however that line is only called when a test parameter file is provided (due to this if condition). However, we were always calling this line, which is what was leading to the error.

Now I have made it such that if we do not provision the input files (from the test parameter file), we do not provision the output files. Which means that we now do not call this line, if outputMap is not initialized.

Review Instructions
Ensure that you can launch a CWL workflow and tool that without a test parameter file.

Ensure that you can successfully launch the tool described in the ticket.

Issue
DOCK-2353
dockstore/dockstore#5427

I believe this PR should also fix: dockstore/dockstore#4922

Security
If there are any concerns that require extra attention from the security team, highlight them here.

Please make sure that you've checked the following before submitting your pull request. Thanks!

Check that you pass the basic style checks and unit tests by running ./mvnw clean install
If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.

…sioned

… input files are provisioned.

Fix sonarcloud issues

coverbeck · Apr 28, 2023

dockstore-client/src/main/java/io/dockstore/client/cli/nested/BaseLanguageClient.java

                }
            } else {
                LOG.debug("No test parameter file provided, skipping provisioning");
+                provisionFiles = false;


I don't know CWL very well, but it seems like you are saying, if there are no inputs, then there are no outputs.

Aren't those separate, or are they tied together?

IOW, can't you have a workflow with inputs but no outputs, or even one with outputs but no inputs?

My understanding is that you can still have output files, but in your test parameter file you can specify where you want to upload your result files to. Because

dockstore-cli/dockstore-client/src/main/java/io/dockstore/client/cli/nested/BaseLanguageClient.java

Line 215 in 52c762d

provisionOutputFiles();

calls this method

dockstore-cli/dockstore-client/src/main/java/io/github/collaboratory/cwl/CWLClient.java

Line 1015 in 20aa167

public static List<ImmutablePair<String, FileProvisioning.FileInfo>> registerOutputFiles(Map<String, List<FileProvisioning.FileInfo>> fileMap,

and my understanding is that fileMap is obtained from the test parameter file. I could be wrong on this though.

This change would also effect WDL workflows, but I don't think this would cause an issue, as WDL workflows call

dockstore-cli/dockstore-client/src/main/java/io/github/collaboratory/cwl/CWLClient.java

Line 1015 in 20aa167

public static List<ImmutablePair<String, FileProvisioning.FileInfo>> registerOutputFiles(Map<String, List<FileProvisioning.FileInfo>> fileMap,

too and I believe for a WDL workflow fileMap would still be empty. Meaning that the above method doesn't do anything.

Yeah, I think the wrinkle here may be that WDL does something sane if there is no output location declared, but CWL doesn't

I'd misread the code in my earlier comments, so it's good. I thought it was reading the parameter definitions in the descriptor and acting off of that, which was the source of my questions. What it's doing instead is:

Is there an input parameter file?

If there isn't, don't bother doing the output provisioning.

And that is correct and works.

sonarqubecloud · Apr 28, 2023

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

denis-yuen · Apr 28, 2023

Currently, if you try and launch a CWL tool or workflow without a test parameter file, you get this error:

This is pretty surprising to me, may need to do some experimentation to confirm

coverbeck · May 9, 2023

Retarget to release/1.14 per issue milestone

coverbeck · May 9, 2023

dockstore-client/src/main/java/io/dockstore/client/cli/nested/BaseLanguageClient.java

                }
            } else {
                LOG.debug("No test parameter file provided, skipping provisioning");
+                provisionFiles = false;


I'd misread the code in my earlier comments, so it's good. I thought it was reading the parameter definitions in the descriptor and acting off of that, which was the source of my questions. What it's doing instead is:

Is there an input parameter file?

If there isn't, don't bother doing the output provisioning.

And that is correct and works.

fhembroff added 6 commits April 28, 2023 10:37

Made it so output files are only provisioned if input files are provi…

663c0ac

…sioned

Added test and made it such that output files are only provisioned if…

84613f4

… input files are provisioned.

Fixed checkstyle

48473ec

Removed test from GeneralIT and moved it to LaunchTestIT

429cc60

Undoing changes to GeneralIT

a820a2b

Removed unused import

790ba46

fhembroff mentioned this pull request Apr 28, 2023

dockstore-cli unable to handle empty outputs in cwl dockstore/dockstore#4922

Open

Removed unneeded line

52c762d

Fix sonarcloud issues

fhembroff requested review from coverbeck, denis-yuen and kathy-t April 28, 2023 20:17

fhembroff marked this pull request as ready for review April 28, 2023 20:19

coverbeck reviewed Apr 28, 2023

View reviewed changes

denis-yuen assigned fhembroff Apr 28, 2023

denis-yuen approved these changes Apr 28, 2023

View reviewed changes

kathy-t approved these changes May 2, 2023

View reviewed changes

coverbeck changed the base branch from develop to release/1.14 May 9, 2023 16:40

coverbeck assigned coverbeck and unassigned fhembroff May 9, 2023

coverbeck approved these changes May 9, 2023

View reviewed changes

coverbeck mentioned this pull request May 9, 2023

Unused code in CLI dockstore/dockstore#5503

Closed

coverbeck merged commit 8e18345 into release/1.14 May 10, 2023

coverbeck deleted the feature/DOCK-2353/hello-world-workflow-fails branch May 10, 2023 00:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/dock 2353/hello world workflow fails #236

Feature/dock 2353/hello world workflow fails #236

Uh oh!

fhembroff commented Apr 28, 2023 •

edited

Loading

Uh oh!

coverbeck Apr 28, 2023 •

edited

Loading

Uh oh!

fhembroff Apr 28, 2023

Uh oh!

fhembroff Apr 28, 2023

Uh oh!

denis-yuen Apr 28, 2023

Uh oh!

coverbeck May 9, 2023

Uh oh!

sonarqubecloud bot commented Apr 28, 2023

Uh oh!

denis-yuen commented Apr 28, 2023

Uh oh!

coverbeck commented May 9, 2023

Uh oh!

coverbeck May 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Search code, repositories, users, issues, pull requests...

Feature/dock 2353/hello world workflow fails #236

Feature/dock 2353/hello world workflow fails #236

Uh oh!

Conversation

fhembroff commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coverbeck Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhembroff Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

fhembroff Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

denis-yuen Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

coverbeck May 9, 2023

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 28, 2023

Uh oh!

denis-yuen commented Apr 28, 2023

Uh oh!

coverbeck commented May 9, 2023

Uh oh!

coverbeck May 9, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fhembroff commented Apr 28, 2023 •

edited

Loading

coverbeck Apr 28, 2023 •

edited

Loading