DOCK-2170: Fix tools and DAG generation problem#5232
Conversation
Codecov ReportBase: 73.04% // Head: 73.00% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #5232 +/- ##
=============================================
- Coverage 73.04% 73.00% -0.05%
+ Complexity 4358 4350 -8
=============================================
Files 287 287
Lines 16707 16704 -3
Branches 1835 1836 +1
=============================================
- Hits 12204 12194 -10
- Misses 3638 3642 +4
- Partials 865 868 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Can you link to this, I think I'd like to read up on this and wonder if its a reference to a previous ticket/discussion |
I would add a link, but I don't have one. :) The "not an obviously-better alternative" part is my software-engineering opinion, "multi-level DAG" is an idea for a DAG that renders the steps in the subworkflows (rather than represent each subworkflow as a single node, as we have historically). |
was mainly curious what this meant, thanks! |
coverbeck
left a comment
There was a problem hiding this comment.
It would be nice to have a test for this so it doesn't break in the future.
|
Kudos, SonarCloud Quality Gate passed! |








Description
This PR fixes some bugs in the CWL "tool list" and DAG generation code.
While testing the previous PR (#5063), Charles found a workflow for which the list of tools and DAG were generated incorrectly:
https://qa.dockstore.org/workflows/github.com/sevenbridges-openworkflows/Broad-Best-Practice-Data-pre-processing-CWL1.0-workflow-GATK-4.1.0.0/GATK_4_1_0_0_data_pre_processing_workflow:master?tab=info
The problem was that the workflow ID included slashes, which messed up the workflow step ID and dependency parsing algorithms. This PR changes the algorithms to be more robust.
Yes, that one function takes a ridiculous number of arguments, and now, it's even ridiculous-er, because I added an argument.
Alas, there's not an obviously-better alternative, unless we rewrite large chunks of the code (which we might if we ever implement multi-level DAG generation).
Review Instructions
Clone the repo of the above-mentioned workflow, register the cloned workflow, and confirm that the resulting list of tools and DAG are equivalent to those referenced by the original link.
Issue
https://ucsc-cgl.atlassian.net/browse/DOCK-2170
#4906
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean install@RolesAllowedannotation