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

Wrong default tool version when clicking on toolbox #15945

Closed
abretaud opened this issue Apr 17, 2023 · 9 comments
Closed

Wrong default tool version when clicking on toolbox #15945

abretaud opened this issue Apr 17, 2023 · 9 comments

Comments

@abretaud
Copy link
Contributor

Describe the bug
On .eu and .org (23.0), when I click on repeatmasker in the tool box, it opens version 4.1.1 instead of the latest 4.1.2-p1+galaxy1
Maybe because of "p1" in the version?

Galaxy Version and/or server at which you observed the bug
Galaxy Version: 23.0
Commit: the one on .eu :)

To Reproduce
Steps to reproduce the behavior:

  1. In the toolbox, search for repeatmasker
  2. Pass your mouse on the link: it contains "4.1.2-p1+galaxy1"
  3. Click on the link -> it's version "4.1.1" that is displayed

Expected behavior
It should be the latest version, I guess by alphanumerical order, so "4.1.2-p1"

@nsoranzo
Copy link
Member

Unfortunately 4.1.2-p1 is not recognised as a valid version number, so it gets sorted as lower of all valid versions.
This needs to fixed in the tool wrapper.

@mvdbeek
Copy link
Member

mvdbeek commented Apr 17, 2023

4.1.2-p1 is not a valid version according to pep440, so gets parsed as LegacyVersion which always sorts below valid versions.

In [1]: from packaging.version import parse

In [2]: parse('4.1.2-p1+galaxy1')
Out[2]: <LegacyVersion('4.1.2-p1+galaxy1')>

In [3]: parse('4.1.2+galaxy1')
Out[3]: <Version('4.1.2+galaxy1')>

Closing as duplicate of #15580.

@abretaud
Copy link
Contributor Author

Ok sorry for the noise, will wrap a newer version of the tool that doesn't have this bad suffix

@bernt-matthias
Copy link
Contributor

I guess we should have a linter for this .. or do we have this already?

@bernt-matthias
Copy link
Contributor

Actually we have one

lint_ctx.warn(WARN_VERSION_MSG % version, node=tool_node)
.. but its only a warning. IUC should fail on warnings nowadays .. maybe the bgruening tool repo does not do this yet?

Could ypu tests this @abretaud ?

@abretaud
Copy link
Contributor Author

ah, will test it sure, though repeatmasker is in iuc now

@abretaud
Copy link
Contributor Author

@bernt-matthias so it's a warning, but it passes

@nsoranzo
Copy link
Member

Maybe we should make this an error instead of a warning? (independently of tools-iuc failing on warning or not)

@bernt-matthias
Copy link
Contributor

Ahh. I just suggested to fail on warning over here: galaxyproject/tools-iuc#4830

We can also make it an error in general ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants