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

Add docker.runOptions to avoid memory swap error #351

Merged
merged 1 commit into from
Jun 28, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ profiles {
test { includeConfig 'conf/test.config' }
}

// Avoid this error:
// WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.
// Thanks to: https://github.com/alesssia/YAMP/wiki/How-to-use-Docker
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Thanks to: https://github.com/alesssia/YAMP/wiki/How-to-use-Docker
// Thanks to: https://github.com/alesssia/YAMP/wiki/How-to-use-Docker
// Testing this in nf-core after discussion here https://github.com/nf-core/tools/pull/351, once this is established and works well, nextflow might implement this behavior as new default.

Copy link
Member

Choose a reason for hiding this comment

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

Could you add this in @olgabot? That should be enough to be able to trace things back once they're in the template.

docker.runOptions = '-u $(id -u):$(id -g)'
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Not sure that is required - the '...' should be enough if it's in nextflow.config right?

Copy link
Member

Choose a reason for hiding this comment

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

sorry, I didn't understood what you meant ;-)

Copy link
Member

Choose a reason for hiding this comment

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

I mean I'm not sure whether we need to escape the $ with a \$ here as they are inside a '...' single quotes block :)

Copy link
Member

Choose a reason for hiding this comment

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

Aaaah, now I understand.
I'm pretty sure I copied that from someone else (probably Paolo) at some point.
I'm guessing the double quotes render the escaping the $ a necessity

Copy link
Member

Choose a reason for hiding this comment

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

Then we need to address this as well - I was looking for another place where this is used but couldn't find it. Wouldn't fixOwnership as listed here do the same? https://www.nextflow.io/docs/latest/config.html#config-docker


// Load igenomes.config if required
if(!params.igenomesIgnore){
includeConfig 'conf/igenomes.config'
Expand Down