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

support remote paths in NfCoreTemplate:dump_parameters #2465

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

fbdtemme
Copy link
Contributor

@fbdtemme fbdtemme commented Oct 9, 2023

Currently the new dump_parameters added in v2.10 (#2425) only works on local files.
When using a remote publish dir such as s3, the json file will be written to a weird directory s3 / path / to / outdir / params_{timestamp}.json in the launchDir.
This small tweak first creates the parameter dump on the nextflow head machine as a hidden file and then moves it to the possibly remote outdir using the nextflow FilesEx helper to deal with the remote filesystems.

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

@fbdtemme fbdtemme changed the base branch from master to dev October 9, 2023 12:00
@nf-core nf-core deleted a comment from github-actions bot Oct 9, 2023
@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #2465 (f1a99a8) into dev (a220bcd) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head f1a99a8 differs from pull request most recent head 5ec389e. Consider uploading reports for the commit 5ec389e to get more accurate results

@@           Coverage Diff           @@
##              dev    #2465   +/-   ##
=======================================
  Coverage   70.60%   70.60%           
=======================================
  Files          87       87           
  Lines        9437     9435    -2     
=======================================
- Hits         6663     6662    -1     
+ Misses       2774     2773    -1     

see 2 files with indirect coverage changes

@JoseEspinosa
Copy link
Member

Thanks for reporting, I see the issue, but I was wondering why the code does not work as I copied it from here and as far as I know, we are not encountering the same issue, maybe @mirpedrol can help here...

@fbdtemme
Copy link
Contributor Author

I have tested it again with a clean pipeline from nf-core create using the local executor but with an s3 outdir.

nextflow run . -profile test,docker --outdir s3://<bucket-name>/clean-pipeline-test --fasta genome.fa --email blah@example.com

image
It seems like the email code has the exact same issue. I have used the same approach to fix it there as well.

Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

I can't test this with S3, but code looks good, if you tested it LGTM :)

@fbdtemme fbdtemme merged commit e7f946d into nf-core:dev Oct 16, 2023
5 of 17 checks passed
fbdtemme added a commit to nf-core/pixelator that referenced this pull request Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants