tools icon indicating copy to clipboard operation
tools copied to clipboard

Moved all singularity-related download functions into a new file

Open muffato opened this issue 8 months ago • 1 comments

Closes #3019 Supersedes #3163

Ongoing work part of the "Workflows to go" hackathon project. I've started moving all singularity functions into a new file.

Overview of the moves from nf_core/pipelines/download.py

Object or function New file New name
DownloadError downloads/utils (same name)
DownloadProgress downloads/utils (same name)
ContainerError downloads/singularity (same name)
symlink_singularity_images downloads/singularity symlink_registries
singularity_image_filenames downloads/singularity get_container_filename
singularity_copy_cache_image downloads/singularity SingularityFetcher.copy_image
singularity_download_image downloads/utils FileDownloader.download_file
singularity_pull_image downloads/singularity SingularityFetcher.pull_image
get_singularity_images (part of) downloads/singularity SingularityFetcher.fetch_containers, SingularityFetcher.pull_images, SingularityFetcher.download_images
get_singularity_images (part of) downloads/utils FileDownloader.download_files_in_parallel

In this move, I wanted to have clean copy/download/pull_image functions that know nothing about a "cache" or a "library". They're just given a path they need to put their file into. I was also able to decouple things and move code from methods to functions when access to class variables etc wasn't really needed. Or in the case of the file downloads, I've introduced a dedicated class where only the necessary state is tracked.

So far, we've got this functionality in downloads.utils:

  • intermediate_file is a context manager for getting download/copy operations done in a temporary file that is deleted if there is a problem. This is to avoid polluting the cache/output directories with partial files (the question was raised in https://github.com/nf-core/tools/pull/3163#discussion_r1768694798) and I've rolled it out across all three source methods.
  • DownloadProgress is the extension of rich.progress. I've added helper methods to deal with the "main task" and "sub tasks"
  • FileDownloader can download files in parallel. I've reviewed the handling of errors and exceptions to make sure that no state if left behind.

and in downloads.singularity:

  • get_container_filename returns the name a container will have on disk, taking into consideration a set of registry prefixes to ignore.
  • symlink_registries creates symlinks for a container so that the same image file can be used across registries.

That's all completely covered in unit tests.

At this minute, I'm not 100% settled on this SingularityFetcher class. I want to look into decoupling it a bit more.

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

muffato avatar Mar 25 '25 11:03 muffato

Codecov Report

:x: Patch coverage is 63.17204% with 137 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 77.45%. Comparing base (33830f9) to head (ea4bc89). :warning: Report is 840 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/pipelines/downloads/singularity.py 44.11% 133 Missing :warning:
nf_core/pipelines/download.py 90.90% 2 Missing :warning:
nf_core/pipelines/downloads/utils.py 98.21% 2 Missing :warning:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 26 '25 12:03 codecov[bot]

Thank you very much for your contribution! Your commits have been merged into #3634 and will be added through this branch. If you have time to test and review, this would be amazing!

MatthiasZepper avatar Jul 30 '25 11:07 MatthiasZepper