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

Fix and enhance job resume functionality #5247

Merged
merged 5 commits into from Dec 31, 2017

Conversation

Projects
None yet
2 participants
@mvdbeek
Member

mvdbeek commented Dec 27, 2017

This should fix #5222.

The problem was that HDA ids in nested parameters were not always being updated
properly. In the case of bowtie2 the input dataset is provided via a
conditional, but the conditional prefix is not being stored in the
JobToInputDatasetAssociation, and so the update_param function was not able
to update the dataset id in these instances. The approach now is to simply
replace all occurences of the old dataset id with the new dataset id.

The PR includes 2 testcases for this functionality, one for hda input and one for hdca input.

With this PR we also replace failed element of a collection with a job rerun (including a testcase), which fixes #2235.

@mvdbeek mvdbeek added this to the 18.01 milestone Dec 27, 2017

@mvdbeek mvdbeek added status/review and removed status/WIP labels Dec 29, 2017

@mvdbeek mvdbeek changed the title from [WIP] Fix job resume functionality for non-prefixed input data to Fix job resume functionality for non-prefixed input data Dec 29, 2017

@mvdbeek mvdbeek changed the title from Fix job resume functionality for non-prefixed input data to Fix and enhance job resume functionality Dec 30, 2017

mvdbeek added some commits Dec 27, 2017

Fix job resume functionality for non-prefixed input data
This should fix #5222.

The problem was that HDA ids in nested parameters were not always being updated
properly. In the case of bowtie2 the input dataset is provided via a
conditional, but the conditional prefix is not being stored in the
JobToInputDatasetAssociation, and so the `update_param` function was not able
to update the dataset id in these instances.  The approach now is to simply
replace all occurences of the old dataset id with the new dataset id.

TODO: tests, refactor the replacement functionality into a separate function
and make this work for JobToInputDatasetCollectionAssociation.
Add test for resume job functionality
This test runs a workflow whose first step fails, followed by a tool that uses
the first step's output as an input, which is behind a nested conditional. This
recapitulates the bug described in #5222.
Add job resume test with HDCA input to paused dataset
I am slightly surprised that this worked without change,
but it appears that the remapping occurs via the HDAs that
the HDCA is composed of.
Replace collection elements with re-run output
This specifically addresses the problem where some jobs of a mapped-over
collection have failed. Instead of filtering the failed collection and
restarting the workflow at this position (involving a lot of copy-paste ...)
the user can now limit the rerun to the problematic jobs and the workflow
should resume from there.
Should fix #2235.

This is one possible implementation, it would also be feasible to not
manipulate the original collection, but to copy the HDCA and then to replace
collection elements and replace all references for jobs that depend on the HDCA,
as we do for HDAs. This implementation seems simpler, but let me know if you
see problems with this approach.
@jmchilton

This comment has been minimized.

Member

jmchilton commented Dec 31, 2017

Wow - this is totally awesome. Great fix and great feature to finally have some test coverage of - this is very exciting! Thanks so much @mvdbeek.

@jmchilton jmchilton merged commit 8e93c2e into galaxyproject:dev Dec 31, 2017

6 checks passed

api test Build finished. 339 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 166 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 67 tests run, 0 skipped, 0 failed.
Details
selenium test Build finished. 118 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details

mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Jan 17, 2018

Add option to replace failed elements on job rerun
Replacing failed collection elements was already possible when dependent jobs
were found (galaxyproject#5247).
This commit restructures the remapping so that remapping is possible when no
dependent jobs are available. This also simplifies the replacement of HDAs
between old and new jobs.

mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Jan 17, 2018

Add option to replace failed elements on job rerun
Replacing failed collection elements was already possible when dependent jobs
were found (galaxyproject#5247).
This commit restructures the remapping so that remapping is possible when no
dependent jobs are available. This also simplifies the replacement of HDAs
between old and new jobs.

mvdbeek added a commit to mvdbeek/galaxy that referenced this pull request Jan 17, 2018

Add option to replace failed elements on job rerun
Replacing failed collection elements was already possible when dependent jobs
were found (galaxyproject#5247).
This commit restructures the remapping so that remapping is possible when no
dependent jobs are available. This also simplifies the replacement of HDAs
between old and new jobs.

@mvdbeek mvdbeek deleted the mvdbeek:job_rerun_fixes branch Jun 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment