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

[17.09] Do not visit child inputs of invalid conditionals #5010

Merged
merged 5 commits into from Nov 28, 2017

Conversation

Projects
None yet
4 participants
@guerler
Contributor

guerler commented Nov 14, 2017

Fixes #4889. Invalid conditionals states are usually the result of tool version changes. Child inputs of invalid conditionals should not be visited if the underlying visitor is unable to fix them. The only input visitor which is able to resolve them and warn the user is check_and_update_param_values in tools.py.

@jmchilton

This comment has been minimized.

Member

jmchilton commented Nov 15, 2017

Thanks for working on this - I take you were able to reproduce this consistently?

@guerler guerler modified the milestones: 18.01, 17.09 Nov 15, 2017

@guerler

This comment has been minimized.

Contributor

guerler commented Nov 15, 2017

Yeah, I created a local test tool with a conditional parameter which has a data parameter as child input (see https://gist.github.com/guerler/1979ad03916b42aaf3fc4e2a0d608e81). Remove the conditional parameter and add the tool to a workflow. Save it, and add it to a page. Then re-add the conditional parameter to the tool xml, reload the instance and load the page. This reproduces the issue. I added a couple test cases to this PR to capture this behavior.

@guerler guerler added status/review and removed status/WIP labels Nov 15, 2017

@nsoranzo nsoranzo modified the milestones: 17.09, 18.01 Nov 15, 2017

@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Nov 15, 2017

Set milestone to 18.01, it should be the next release so it gets added to the release notes (unless it's a backport PR).

@mvdbeek mvdbeek merged commit e903cb7 into galaxyproject:release_17.09 Nov 28, 2017

7 checks passed

api test Build finished. 292 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 161 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 46 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
selenium test Build finished. No test results found.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@mvdbeek

This comment has been minimized.

Member

mvdbeek commented Nov 28, 2017

Thanks a lot @guerler ! We should keep an eye on the published pages list on usegalaxy.org and make sure the tool version changes are legitimate and not a result of using the StockToolLineage or another bug in the tool version selection.

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