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

nf-core download: Create container image symlinks so images are still found, even if registry config diverges. #2768

Merged
merged 12 commits into from
Feb 29, 2024

Conversation

MatthiasZepper
Copy link
Member

@MatthiasZepper MatthiasZepper commented Feb 16, 2024

This addition to nf-core download addresses #2751, #2644, demultiplex#164 and multiple Slack issues: 1,2,3.

Dropping the hardcoded registry from the modules in favour of the flexible config parameters apptainer.registry, docker.registry, podman.registry and singularity.registry was initiated by Seqera, but so far Nextflow has not received concordant updates to also resolve to images downloaded from alternative registries as a fallback.

The code in this PR creates a bunch of symlinks to the downloaded images to ensure that they are found nonetheless. To ensure that it is possible to run the pipelines without symlinks by setting docker.registry or apptainer.registry to empty strings at runtime, the original registry is not preserved any more when downloading. This harbours a small risk of naming collisions, however, due to the symlinks it is anyway not possible to use the same $NXF_CACHE_DIR for pipelines using different containers with identical image names pulled from different registries.

Additionally, this PR modifies the pytest.yml Github Action such that the new tests are run at all, because they require Singularity.

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

@MatthiasZepper MatthiasZepper added the WIP Work in progress label Feb 16, 2024
@MatthiasZepper MatthiasZepper self-assigned this Feb 16, 2024
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

Attention: Patch coverage is 90.47619% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 75.00%. Comparing base (f64d486) to head (e64f1a7).
Report is 6 commits behind head on dev.

Files Patch % Lines
nf_core/download.py 90.47% 6 Missing ⚠️
Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MatthiasZepper MatthiasZepper force-pushed the download_image_naming_issue branch 4 times, most recently from aacde33 to 453afca Compare February 21, 2024 21:13
@MatthiasZepper MatthiasZepper removed the WIP Work in progress label Feb 21, 2024
@MatthiasZepper MatthiasZepper marked this pull request as ready for review February 21, 2024 21:34
Copy link
Contributor

@Aratz Aratz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Just had a couple minor questions for you

nf_core/download.py Outdated Show resolved Hide resolved
nf_core/download.py Outdated Show resolved Hide resolved
nf_core/download.py Outdated Show resolved Hide resolved
nf_core/download.py Outdated Show resolved Hide resolved
@MatthiasZepper MatthiasZepper merged commit c24b390 into nf-core:dev Feb 29, 2024
35 checks passed
@MatthiasZepper MatthiasZepper deleted the download_image_naming_issue branch February 29, 2024 16:57
@edmundmiller edmundmiller added the download nf-core download label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants