Fix for re-installing an uninstalled TS repository with a dependency #1154
Conversation
where both the repository and the dependency were updated in the TS after the repository was uninstalled.
|
@galaxybot test this |
|
This change makes sense and my simple testing seems working. Thanks @gregvonkuster |
Fix for re-installing an uninstalled TS repository with a dependency
|
Awesome, thanks so much @gregvonkuster,this should be fixing issue #667 . |
|
@gregvonkuster this is great! Thanks so much. In exchange see https://github.com/bgruening/docker-galaxy-stable/tree/dev for automatic tested images :) |
| tool_shed_url = common_util.get_tool_shed_url_from_tool_shed_registry( app, tool_shed ) | ||
| repository_clone_url = os.path.join( tool_shed_url, 'repos', owner, name ) | ||
| repo_info_tuple = (None, repository_clone_url, changeset_revision, None, owner, None, None) | ||
| repository, pcr = repository_was_previously_installed( app, tool_shed_url, name, repo_info_tuple ) |
There was a problem hiding this comment.
@gregvonkuster It seems that the code at line 503-507 is a subset of what is done by repository_was_previously_installed(): can this be consolidated in this last function?
There was a problem hiding this comment.
@nsoranzo You make a good point, but the ideal solution to this is to rebase these functions and clean them up, which may be a bit non-trivial. My understanding is that all of this is going away some time soon?
I'm going to try to get some time to look at the 2nd problem discussed here (unless someone else gets to it first) #667 (comment)
If it makes sense to clean this function up a bit, I'll do it as part of that work.
By the way, what Tool Shed test harness should I be running to test PRs like this? I want to make sure I'm not breaking things. ;)
Thanks!
There was a problem hiding this comment.
@gregvonkuster we have dockerized TS tests running at https://jenkins.galaxyproject.org/job/docker-toolshed/ but currently 12 are failing because of lack of maintenance (that is why they are not plugged in the PRs like the others)
Jenkins runs: ./run_tests.sh --dockerize --db postgres -toolshed
I also think that things are not going away soon. Not if we want to stay reproducible.
There was a problem hiding this comment.
@martenson Thanks! I'll make sure to run those before changing any code and then submitting PRs that change Tool Shed code. It will at least help to know that the same 12 break before changes and after. ;)
There was a problem hiding this comment.
the list of failing tests is here: https://jenkins.galaxyproject.org/job/docker-toolshed/lastBuild/testReport/ (14 now)
where both the repository and the dependency were updated in the TS
after the repository was uninstalled.
This one is for @bgruening ;)
Here is the scenario:
In Galaxy:
In the Tool Shed:
Back in Galaxy:
Old behavior:
The new revision of B would get installed as a "white ghost" in addition to the original B
A would get installed with missing dependency B
Corrected behavior after this PR:
The updated revisions of A and B are installed, with single installed instanced of both.