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.

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 wrong version being returned if requested version doesn't exist #6702

Merged
merged 4 commits into from Sep 13, 2018

Conversation

Projects
None yet
3 participants
@mvdbeek
Copy link
Member

commented Sep 12, 2018

Includes a unit test that fails without the fix and an API test that tests showing a tool by version (but that wouldn't actually fail without the fix for a local tool).

# No tool matches by version, simply return the first available tool found
return rval[0]
# No tool matches by version, simply return the newest matching tool
return sorted(rval, key=lambda x: x.version, reverse=True)[0]

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Sep 12, 2018

Member

I think rval[-1] should be a faster equivalent, since tool_lineage.get_versions() returns ToolLineageVersion objects from oldest to newest.

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Sep 12, 2018

Author Member

I had forgotten that!

@martenson martenson added this to the 18.09 milestone Sep 12, 2018

@martenson

This comment has been minimized.

Copy link
Member

commented Sep 12, 2018

this could aim at 18.05 I think

@mvdbeek

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2018

probably earlier than that, need to figure out when we moved away from the toolshed versioning scheme

@mvdbeek mvdbeek force-pushed the mvdbeek:fix_latest_version branch from 08199d0 to bae1a0b Sep 12, 2018

@mvdbeek mvdbeek changed the title [WIP] Fix latest version Fix wrong version being returned if requested version doesn't exist Sep 12, 2018

@mvdbeek mvdbeek added status/review and removed status/WIP labels Sep 12, 2018

url = "tools/%s" % tool_id
if version:
url += "?version=%s" % version
tool_show_response = self._get(url, data=dict(io_details=True))

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Sep 13, 2018

Member

I think this should be:

        data = dict(io_details=True)
        if version:
            data['version'] = version
        tool_show_response = self._get("tools/%s" % tool_id, data=data)

@mvdbeek mvdbeek force-pushed the mvdbeek:fix_latest_version branch from bae1a0b to f8430b1 Sep 13, 2018

@mvdbeek

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

I've rebased this with the changes, should be ready to go!

@nsoranzo nsoranzo merged commit d1352c4 into galaxyproject:dev Sep 13, 2018

6 checks passed

api test Build finished. 422 tests run, 1 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 187 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 118 tests run, 5 skipped, 0 failed.
Details
selenium test Build finished. 146 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details

@nsoranzo nsoranzo deleted the mvdbeek:fix_latest_version branch Sep 13, 2018

@martenson martenson modified the milestones: 18.09, 19.01 Sep 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.