galaxyproject / galaxy

Data intensive science for everyone.
https://galaxyproject.org
Other
1.38k stars 992 forks source link

Wrong default tool version when clicking on toolbox #15945

Closed abretaud closed 1 year ago

abretaud commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/galaxyproject/galaxy/issues/15580.

abretaud commented 1 year ago

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

bernt-matthias commented 1 year ago

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

bernt-matthias commented 1 year ago

Actually we have one https://github.com/galaxyproject/galaxy/blob/6f50816fa5d91fa744b1f44df169dad651b4e10a/lib/galaxy/tool_util/linters/general.py#L41 .. 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 commented 1 year ago

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

abretaud commented 1 year ago

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

nsoranzo commented 1 year ago

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

bernt-matthias commented 1 year ago

Ahh. I just suggested to fail on warning over here: https://github.com/galaxyproject/tools-iuc/pull/4830

We can also make it an error in general ..