Download: Trim explicit registries

[MatthiasZepper]

Issue

Some pipelines respectively modules use full-length URIs such as quay.io/qiime2/core:2022.11 when referencing the Docker container image.

Such URIs were not properly accounted for when the new -l / --container-library feature for tools 2.9 was devised. This prevented the user from successfully pulling the images, because the registry was duplicated in the final URI as is exemplified with nf-core download ampliseq -r '2.6.1' -s 'singularity' -t:

INFO     Pulling of "quay.io/qiime2/core:2022.11" failed.
         Please troubleshoot the command
         "singularity pull --name
         /crex/proj/sllstore2017079/private/zepper/nextflow/download_pipelines/ampliseq/singularity-images/quay.io-qii
         me2-core-2022.11.img docker://quay.io/quay.io/qiime2/core:2022.11" manually. 

Approach

In general, three possible solutions to this problem exist:

1. Ignore the provided -l / --container-library arguments for full length URIs.
2. Exclusively considering the provided -l / --container-library arguments while ignoring any module-specified registry.
3. Trimming the registry from the module, but adding it to the list of -l / --container-library provided ones.

All in all, option 2 was chosen.

If changes were required after pipeline publication, for example due to the migration to another registry, option 1 would have prevented any. Option 3 would have posed the challenge of interfering with the items to loop over during the loop execution itself, with merely questionable benefits for the UX. Although it would have allowed for more instant successful downloads than any other approach, silently adding another container registry would also have been dangerous, as it would have enabled URI hijacking for other modules in a pipeline.

Therefore, option 2 was ultimately implemented. It eliminates a possible registry automatically to avoid duplications and fixes most download issues, since far the most explicit registries in nf-core modules are quay.io and docker.io. An INFO message with the module-specified registry is printed to the screen, so that a user can retry the download with said registry as explicit argument.

PR checklist

• This comment contains a description of changes (with reason)
• CHANGELOG.md is updated
• If you've fixed a bug or added code that should be tested, add tests!
• Documentation in docs is updated

[codecov]

Codecov Report

Merging #2403 (7573c90) into dev (ba395fe) will decrease coverage by 0.17%.
The diff coverage is 20.83%.

@@            Coverage Diff             @@
##              dev    #2403      +/-   ##
==========================================
- Coverage   71.23%   71.07%   -0.17%     
==========================================
  Files          87       87              
  Lines        9415     9431      +16     
==========================================
- Hits         6707     6703       -4     
- Misses       2708     2728      +20     

Files Changed	Coverage Δ
nf_core/download.py	59.35% <20.83%> (-0.86%)	⬇️

... and 1 file with indirect coverage changes

[adamrtalbot]

docker pull uses option 1. Do we want to be consistent with that or do our own thing? I agree 3 is rubbish and this might be the best option.

Also, can we mix and match registries? [edit] Oh I see, by running twice.

[MatthiasZepper]

What do you mean by mix and match registries?

An absolute URI is not automatically tried (unless that registry was anyway specified with e.g. -l docker.io), but a message is generated with the suggestion to add that registry for a future, second run. An alternative would be to try the original registry first and then use the truncated version later. If you anyway plan to refactor and modularize the code, then this might be an option.

[adamrtalbot]

I would personally prefer option 1 because this is consistent with the docker API, but then again I don't use Singularity. The code itself looks fine.

[MatthiasZepper]

@mirpedrol @mashehu - do you have a preference for either of the two?

[mirpedrol]

Option 1 looks more intuitive to me, modules with absolute URIs will usually be from a registry different from quai.io (at least in nf-core modules).
Do you think the case of a registry change will happen often?

[MatthiasZepper]

So, I have now implemented the requested changes to nf-core download. Now absolute URIs are just used without further changes or consideration of eventually specified --container-library. My rationale was that the ability to override paths specified in legacy software might be a good thing, but I see how consistency with other applications also matters for intuitive usability.

Additionally, I have also added the CHANGELOG.md to ignored paths in the Pytest Github Action, so they are not always run. While rebasing, I realized that the Actions have been refactored quite a lot, since I initially created my feature branch, so it might be that there are more actions which also would not need to run for simple Changelog changes. But since I do not know the rationale and purpose of the new workflows, I will leave them as they are.

[adamrtalbot]

Nice work (again!)

[mirpedrol]

LGTM! Thank you!