8000 default results should be null (fails on s3) by maxulysse · Pull Request #884 · nf-core/sarek · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

default results should be null (fails on s3) #884

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 11, 2022
Merged

default results should be null (fails on s3) #884

merged 3 commits into from
Dec 11, 2022

Conversation

maxulysse
Copy link
Member

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs- [ ] If necessary, also make a PR on the nf-core/sarek branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@github-actions
Copy link
github-actions bot commented Dec 7, 2022 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 465673e

+| ✅ 150 tests passed       |+
#| ❔   8 tests were ignored |#
!| ❗   2 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline
  • schema_description - No description provided in schema for parameter: cnvkit_reference

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.7.1
  • Run at 2022-12-11 19:35:07

@FriederikeHanssen
Copy link
Contributor

makes sense. I think we had this before

@asp8200
Copy link
Contributor
asp8200 commented Dec 7, 2022 •
edited
Loading

Why are we setting outdir to null? Does it mean that if the user doesn't specify outdir to something else, then all the output-files will end up in ${projectRootDir}/null/?

@FriederikeHanssen
Copy link
Contributor

on aws, you need to specify the absolut path for the output. Having the default path there has caused failures for many people. So to be safe, across pipeline we went without default path but always have everyone set it manually

@asp8200
Copy link
Contributor
asp8200 commented Dec 7, 2022

on aws, you need to specify the absolut path for the output. Having the default path there has caused failures for many people. So to be safe, across pipeline we went without default path but always have everyone set it manually

okay, so the pipeline will not run at all unless one sets outdir to something other than the default null?

@FriederikeHanssen
Copy link
Contributor

yep exactly.

@asp8200 asp8200 self-requested a review December 7, 2022 12:29
asp8200
asp8200 approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor
@asp8200 asp8200 left a comment

Choose a reason for hiding this comment

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

A bit annoying we have to specify the outdir to ./results in each test-yml, but I guess there was no other way 🤔

@asp8200
Copy link
Contributor
asp8200 commented Dec 7, 2022

Some CI-tests failing, but I guess you are on to that ;-)

@maxulysse maxulysse merged commit e3fa426 into nf-core:dev Dec 11, 2022
@maxulysse maxulysse deleted the dev_results branch December 11, 2022 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asp8200 asp8200 asp8200 approved these changes

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
@maxulysse @FriederikeHanssen @asp8200
0