Skip to content

support remote paths in NfCoreTemplate:dump_parameters#2465

Merged
fbdtemme merged 3 commits into
nf-core:devfrom
PixelgenTechnologies:fix-remote-params-json
Oct 16, 2023
Merged

support remote paths in NfCoreTemplate:dump_parameters#2465
fbdtemme merged 3 commits into
nf-core:devfrom
PixelgenTechnologies:fix-remote-params-json

Conversation

@fbdtemme

@fbdtemme fbdtemme commented Oct 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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
@fbdtemme fbdtemme force-pushed the fix-remote-params-json branch from ce457a2 to 86bfec9 Compare October 9, 2023 12:02
@nf-core nf-core deleted a comment from github-actions Bot Oct 9, 2023
@codecov

codecov Bot commented Oct 9, 2023 •
edited
Loading

Copy link
Copy Markdown

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

@fbdtemme fbdtemme requested a review from JoseEspinosa October 9, 2023 12:27
@JoseEspinosa

JoseEspinosa commented Oct 9, 2023

Copy link
Copy Markdown
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

fbdtemme commented Oct 11, 2023

Copy link
Copy Markdown
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.

@fbdtemme fbdtemme requested a review from mirpedrol October 12, 2023 11:33
mirpedrol
mirpedrol approved these changes Oct 16, 2023
View reviewed changes

@mirpedrol mirpedrol left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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 force-pushed the fix-remote-params-json branch from f1a99a8 to 5ec389e Compare October 16, 2023 13:14
@fbdtemme fbdtemme merged commit e7f946d into nf-core:dev Oct 16, 2023
fbdtemme added a commit to nf-core/pixelator that referenced this pull request Oct 16, 2023
@fbdtemme
fix remote outdir in NfCoreTemplate.groovy
8a2320d 
See: nf-core/tools#2465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mirpedrol mirpedrol mirpedrol approved these changes

@JoseEspinosa JoseEspinosa Awaiting requested review from JoseEspinosa

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fbdtemme @JoseEspinosa @mirpedrol