Skip to content
This repository has been archived by the owner on Aug 8, 2018. It is now read-only.

Don't use bioconda environments in docker image #29

Closed
ewels opened this issue May 28, 2018 · 14 comments
Closed

Don't use bioconda environments in docker image #29

ewels opened this issue May 28, 2018 · 14 comments

Comments

@ewels
Copy link
Member

ewels commented May 28, 2018

See nf-core/chipseq#10 (comment) by @tiagochst:


  • Changed RUN conda env create -f /environment.yml to RUN conda env update -n root --file environment.yml because of the source below:
    • If you are using the Conda package manager, avoid creating a Conda environment. Instead, update the root environment with whatever dependencies you want to install.

      As an example to the above warning, it would be best practice to run this: RUN conda env update -n root --file environment.yml and avoid this: RUN conda env create -f environment.yml

https://docs.datascience.com/en/master/appendix-1/dockerfile-basics-and-best-practices-1.html


We should update all pipelines to use this method.

@apeltzer
Copy link
Member

Didn’t we have that in the beginning and then switched to define a conda environment ? I just can’t remember anymore why and for what reason right now ..... will check this once I got my notebook back ....

@sven1103
Copy link
Member

The documentation just states, that one should not create Conda environments in Docker builds, but unfortunately not why. Can someone enlighten me?

@ewels
Copy link
Member Author

ewels commented May 29, 2018

@tiagochst - any ideas why this is the case? Also pinging @tsnowlan - you're not involved in this project at all sorry, but you know your stuff when it comes to Docker so perhaps you have some insight!

@tsnowlan
Copy link

tsnowlan commented May 29, 2018

Looks like those are restrictions for the specific DataScience.com platform. They also say not to use ARG, custom ENTRYPOINT, etc., and those are definitely normally used dockerfile commands. Are you using their platform, or just using this as a best practices reference?

As to why they don't like conda environments, I have no idea. I see a number of other pages detailing how to set it up in both docker and singularity. They're from different times in the past few years as well so I don't think it's a matter of new or outdated practice. Might just be something for simplifying their ops requirements.

@ewels
Copy link
Member Author

ewels commented May 29, 2018

Are you using their platform, or just using this as a best practices reference?

Just thinking about best practices. @tiagochst - how did you come across these docs in the first place? Was there a reason that you were referring to them?

We currently have a line to avoid needing to activate the conda environment in the container:

ENV PATH /opt/conda/envs/nfcore-chipseq-1.4dev/bin:$PATH

I've always felt that this is a little hacky, so using the root environment feels cleaner somehow. But maybe we should keep things simple and stay with the current Dockerfile setup then. After all, it seems to work..

@tiagochst
Copy link

You can keep the old way if it was working and it was just a warning. I don't know the reason why use one instead of the another unless I ask who wrote the documentation.

I'm not sure how I came to that documentation. But, I would say I found that by chance (I was looking to use conda with github repo, If I'm not wrong) and decided to change if it was working since it was a warning. There was no problem with the last code.

@apeltzer
Copy link
Member

I have je same feeling: it looks cleaner but thought it’s bad practice to have everything in the root environment. As we are only using this environment in the container and are not creating more than that, I assume it would be fine too and feels a bit cleaner . Also we could get rid of the current special ENV hack ...

@ewels
Copy link
Member Author

ewels commented May 29, 2018

@apeltzer - I'm a bit confused. Are you saying that we should make the change or keep it as it is?

@sven1103
Copy link
Member

@apeltzer I dont get it now either :D

@sven1103
Copy link
Member

But my feeling is, that you go for root environment @apeltzer. I was just curious, if there is a strong reason, not to use conda environments in Dockerbuilds. If this is not the case, I vote for leaving the Dockerfile template as it is now.

@ewels
Copy link
Member Author

ewels commented May 29, 2018

+1 vote for leaving it as it is now if no practical differences.

@sven1103
Copy link
Member

+1

@apeltzer
Copy link
Member

apeltzer commented May 30, 2018

Hi everyone,
sorry for my ipad comment - trying to save typing there since I'm terrible on that "keyboard" they provide on ipads....

No there is no strong reason to NOT use them - only that they require a "hackish" ENV to add the binaries to the current PATH inside the container seems a bit "unclean" to me and I'd prefer the root solution for that purpose. However, changing everyone's containers just because of such a purely cosmetic thing is overkill for me too - so let's keep things as they are!

If we at some point find a technical reason why we shouldn't have the conda environments inside the container we can anyways migrate ;-)

Sorry for not being more clear with my comments on this

@ewels
Copy link
Member Author

ewels commented May 30, 2018

Ok cool, I think we have consensus 👍 Will close this issue now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants