Skip to content

Fix Issues/3729: Cleanup nextflow files / directories after pipeline download#3750

Merged
JulianFlesch merged 8 commits into
nf-core:devfrom
JulianFlesch:issues/3729-download-cleanup
Oct 8, 2025
Merged

Fix Issues/3729: Cleanup nextflow files / directories after pipeline download#3750
JulianFlesch merged 8 commits into
nf-core:devfrom
JulianFlesch:issues/3729-download-cleanup

Conversation

@JulianFlesch

@JulianFlesch JulianFlesch commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Closes #3729

Adds a new context manager for creating and stepping into a temporary directory handles cleanup and stepping back upon errors. Wraps this around the call to nextflow inspect.

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

@JulianFlesch JulianFlesch requested a review from a team September 9, 2025 09:08
@JulianFlesch JulianFlesch self-assigned this Sep 9, 2025
@JulianFlesch JulianFlesch added enhancement download nf-core download labels Sep 9, 2025
@codecov

codecov Bot commented Sep 9, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.36%. Comparing base (c5af93a) to head (e397d50).
⚠️ Report is 9 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/pipelines/download/download.py 91.66% 1 Missing ⚠️
nf_core/pipelines/download/utils.py 90.00% 1 Missing ⚠️

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

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

Comment thread nf_core/pipelines/download/utils.py Outdated
Comment thread nf_core/pipelines/download/utils.py Outdated
tmp.cleanup()
except: # noqa: E722
os.chdir(original_dir)
tmp.cleanup()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think the cleanup is needed, because tempfile should take care of that already

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes :) The use of Tempdir is what was added in this PR. This solves the cleaning up in a nice way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but then you don't need tmp.cleanup(), it will be removed anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah right, sorry 🤦 I also was looking in the wrong function for a second, thinking we reference tmp outside the with context ...
Changed!

@JulianFlesch JulianFlesch added this to the 3.4.0 milestone Oct 7, 2025
@github-project-automation github-project-automation Bot moved this from In Review to In Progress in nf-core infrastructure projects Oct 7, 2025
@JulianFlesch JulianFlesch merged commit 7a02ae6 into nf-core:dev Oct 8, 2025
199 of 202 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in nf-core infrastructure projects Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

download nf-core download enhancement

Projects

Development

Successfully merging this pull request may close these issues.

3 participants