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

[23.1] Fix duplicated tools in tool panel view section copying #17036

Merged
merged 6 commits into from
Nov 17, 2023

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Nov 15, 2023

Alternative to #17034. This has the advantage over that general approach of loading the newest version of the tool referenced in the panel instead of the newest tool overall.

How to test the changes?

(Select all options that apply)

  • Instructions for manual testing are as follows:
    1. Run Galaxy with GALAXY_RUN_WITH_TEST_TOOLS=1 on
    2. Install two versions of a tool shed tool into the "Test Section with Multiple Versions" section of the tool panel.
    3. Swap to tool panel view "Filtered Test Section w/multiple versions" , verify all tools grouped by version.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek
Copy link
Member

mvdbeek commented Nov 16, 2023

This has the advantage over that general approach of loading the newest version of the tool referenced in the panel instead of the newest tool overall.

It is an advantage for correctness, but it is also very likely not what admins or users want. I can't think of a reason for ever wanting users to get a not-latest version by default. reproducibility and workflows are reasons to load old tools, and you can always switch to old versions. Having users land at old versions which you don't want in most cases means you have to invest a large amount of time in maintaining the panels up to date. See also #16145

@jmchilton
Copy link
Member Author

I greatly disagree with your statement. I think tool panels with suites of tools at different versions in clearly labeled sections is an important use case that Galaxy and only Galaxy could really excel at. This is important for validation and testing for publication, for large organizations that resist change, for use cases around CLIA and other cases where validation is the limiting step and takes considerable resources. It is wild to me to call these use cases invalid when they could be our strength. But this is such a pedantic point when the rest of the UI doesn’t support it fully and an admin would have to go out of their way to ensure the panels are setup like that. Certainly the use case of just porting sections defined by tool shed installs from the main tool panel to others is fixed by this approach. I hope you don’t sink this PR on this point.

@mvdbeek
Copy link
Member

mvdbeek commented Nov 16, 2023

ok, i can accept that argument but I think this needs to be clearly communicated (see https://github.com/galaxyproject/usegalaxy-playbook/blob/main/env/main/files/galaxy/config/panel_views/vgp.yml#L77 ... those should all be versionless then for that use case ?). Can you cherry-pick 19a4c9d and c6435f7 as well ?

@jmchilton
Copy link
Member Author

I agree that admins should't be putting versioned IDs in their configs ever unless they mean to specify a specific version. That is exactly why I implemented #16664. We should make working without IDs as easy and simple as possible - I understand that is the primary use case and the only one usegalaxy.* will care about for the near future.

@jmchilton jmchilton marked this pull request as ready for review November 16, 2023 13:25
@github-actions github-actions bot added this to the 23.2 milestone Nov 16, 2023
@mvdbeek
Copy link
Member

mvdbeek commented Nov 16, 2023

@jmchilton jmchilton merged commit e5d37d4 into galaxyproject:release_23.1 Nov 17, 2023
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants