Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[16.04] backport of model tool dependencies fix #2959

Merged
merged 4 commits into from
Sep 23, 2016
Merged

[16.04] backport of model tool dependencies fix #2959

merged 4 commits into from
Sep 23, 2016

Conversation

martenson
Copy link
Member

xref: #2950
ping @nsoranzo @davebx

davebx and others added 2 commits September 22, 2016 14:54
@davebx @martenson
[16.07] Fix tool dependencies handling in Tool Shed model (galaxyproj…
9b3c061 
…ect#2950)

* Fix the cause of toolshed test 1400 failing.

* Fix handling of repositories without tool dependencies.

* Fix tool dependency retrieval.

In some cases, a process might attempt to fetch tool dependencies for revision
1:7334f6d0ac17, but the repository has been updated, and tool dependency
metadata has been reassigned to e.g. 7:ce22ceec214a. This commit resolves
that issue.

* Correct the logic that returns an empty string for an empty dict.

* Fix getting latest revision with tool dependencies.

* Use self.app instead of trans.app

Conflicts:
	lib/galaxy/webapps/tool_shed/controllers/repository.py
@martenson
Merge branch 'release_16.04' of https://github.com/galaxyproject/galaxy…
a78b8bc 
… into release_16.04
@martenson martenson added this to the 16.04 milestone Sep 22, 2016
martenson
martenson commented Sep 22, 2016
View reviewed changes
lib/galaxy/webapps/tool_shed/controllers/repository.py
@@ -1713,8 +1713,8 @@ def get_tool_dependencies( self, trans, **kwd ):
owner = kwd.get( 'owner', None )
changeset_revision = kwd.get( 'changeset_revision', None )
repository = suc.get_repository_by_name_and_owner( trans.app, name, owner )
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to change this to suc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was expected: #2950 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nsoranzo that's why I had no problems doing it :)

Copy link
Member Author

@martenson martenson Sep 22, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, seems I had a problem doing it :D

nsoranzo
nsoranzo requested changes Sep 22, 2016
View reviewed changes
lib/galaxy/webapps/tool_shed/model/__init__.py
def get_tool_dependencies( self, changeset_revision ):
def get_tool_dependencies( self, app, changeset_revision ):
repo = hg.repository( ui.ui(), self.repo_path( app ) )
changeset_revision = metadata_util.get_next_downloadable_changeset_revision( self, repo, changeset_revision )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that's the one that should be changed: s/metadata_util/suc/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, that one was not in conflict...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because these are new lines.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@nsoranzo
Copy link
Member

nsoranzo commented Sep 22, 2016 •
edited

The TS test is still failing https://jenkins.galaxyproject.org/job/docker-toolshed/4140/testReport/shed_functional.functional.test_1400_review_migration_stages/TestToolMigrationStages/test_0005_load_migration_stages_page/ , but I think this should be fixed by updating the freebayes version in test/shed_functional/functional/test_1400_review_migration_stages.py . Testing it now.

@nsoranzo
Copy link
Member

Confirmed. @martenson, can you add the following to this PR ? I have to go home.

diff --git a/test/shed_functional/functional/test_1400_review_migration_stages.py b/test/shed_functional/functional/test_1400_review_migration_stages.py
index 8499b68..926f30a 100644
--- a/test/shed_functional/functional/test_1400_review_migration_stages.py
+++ b/test/shed_functional/functional/test_1400_review_migration_stages.py
@@ -29,7 +29,7 @@ class TestToolMigrationStages( ShedTwillTestCase ):
                                       'blast_datatypes', 'bwa_wrappers', 'picard', 'lastz',
                                       'lastz_paired_reads', 'bowtie_color_wrappers', 'bowtie_wrappers',
                                       'xy_plot', 'bam_to_sam' ]
-        migrated_tool_dependencies = [ 'emboss', '5.0.0', 'freebayes', '0.9.4_9696d0ce8a962f7bb61c4791be5ce44312b81cf8',
+        migrated_tool_dependencies = [ 'emboss', '5.0.0', 'freebayes', '0.9.4_a46483351fd0196637614121868fb5c386612b55',
                                        'samtools', '0.1.18', 'blast+', '2.2.26+', 'bwa', '0.5.9', 'picard', '1.56.0',
                                        'lastz', '1.02.00', 'bowtie', '0.12.7', 'FreeBayes requires g++', 'ncurses', 'zlib',
                                        'blast.ncbi.nlm.nih.gov', 'fastx_toolkit', '0.0.13', 'samtools', '0.1.16', 'cufflinks',

@martenson
Copy link
Member Author

thanks @nsoranzo - we discussed the need to do this with @davebx previously but then it vanished from our minds

@jmchilton jmchilton merged commit 45e6949 into galaxyproject:release_16.04 Sep 23, 2016
@nsoranzo
Copy link
Member

Forward merged.

@RoshanRajapakse RoshanRajapakse mentioned this pull request Feb 16, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nsoranzo nsoranzo nsoranzo approved these changes

Assignees
No one assigned
Labels
merge
Projects
None yet
Milestone
16.04
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@martenson @nsoranzo @jmchilton @davebx