Recurse handle_invalid_pipeline #1165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Upgrades are now always done on the latest upgrade in the chain TODOs: Should fall back to previous upgrade action when upgrade fails Add test
core/vistrail/controller.py:
- validate_version:
Try older upgrades when latest upgrade fails.
If all fail, the upgrade attempt for the latest upgrade
will be selected.
core/vistrail/vistrail.py:
- get_upgrade_chain: *new
Returns current upgrade chain for specified version.
TODO: Add test
Tests that upgrade is created from the latest node. Tests that a failed upgrade falls back to the previous upgrade.
Hides version nodes that have action.description == 'Upgrade', when hideUpgrades is set. This reduces dangling node clutter when we have broken upgrades.
It never made sense to call handle_invalid_pipeline multiple times. Only handle_invalid_pipeline knows if new exceptions should be handle again. Now we recurse if more exceptions need to be processed. Reasons for recursing: * Not handled and new exceptions * New exceptions from validating current pipeline vistrails/core/vistrail/controller.py: Added recursion to handle_invalid_pipeline Multiple calls to handle_invalid_pipelines are no longer necessary
| child not in tm): | ||
| elif (not self.show_upgrades and | ||
| (child in upgrades or | ||
| am[child].description == 'Upgrade') and |
There was a problem hiding this comment.
This is probably safe since only our upgrade code will ever perform upgrades.
test_infinite_looping_upgrade: *new Tests that we give up after 50 upgrade iterations test_looping_pipeline_fix: Assert that we iterate handle_invalid_pipeline 5 times.
Before we always tried to build a new valid ModuleDescriptor for new module upgrades. Now we use the given descriptor so that it does not need to refer to an up-to-date module type. Also, now ModuleDescriptor.package_version is respected so that we do not replace the descriptor package_verasion with the current package_version.
|
The test now correctly loops 5 times. There was a bug in the upgrade code (0124df5) that replaced the descriptor for B version 0.1 with the latest version, causing the upgrade to C to never happen. Added test for infinite loops. Added debug messages and conf option from chain-upgrades-loop. Fixed invalid pipelines causing version=Null (eff5d13) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces https://github.com/VisTrails/VisTrails/tree/invalid-pipeline-hides-package-errors_/loops and #901.
Before, handle_invalid_pipelines was called up to 2 times to resolve pipeline errors. But some cases will require 3 calls, as shown by 21708fa.
This pr makes handle_invalid_pipeline call iteself when needed so that it only needs to be called once.
This is based on, and includes, #1164.