-
Notifications
You must be signed in to change notification settings - Fork 16
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
A number of fixes and best-practice tweaks. #83
base: master
Are you sure you want to change the base?
Conversation
davebx
commented
Jun 23, 2016
- Clean up the local environment file.
- Install R and BiocInstaller.
- Install BioConductor R packages Rgraphviz and motifbreakR.
- Use get_url for file downloads instead of curl or wget.
- Use the known working method for enabling supervisord on boot.
- Some post-playbook cleanup.
- Rearrange some apt actions, consolidate others.
- Clean up the local environment file. - Install R and BiocInstaller. - Install BioConductor R packages Rgraphviz and motifbreakR. - Use get_url for file downloads instead of curl or wget. - Use the known working method for enabling supervisord on boot. - Some post-playbook cleanup. - Rearrange some apt actions, consolidate others.
export PYTHONPATH={{ dev_user_home }}/miniconda2/lib/python2.7 | ||
else | ||
export PYTHONPATH={{ dev_user_home }}/miniconda2/lib/python2.7:$PYTHONPATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is scary - why this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that it doesn't stomp previously defined PYTHONPATH additions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get why we are doing anything with the Python path at all I mean. Also you are putting conda ahead of system Python on the path - this breaks things. I'd just stick with adding conda to the path, but at the end and not touch Python path (unless there is a documented reason).
I am fine with changes (to my low understanding of packer and ansible) but I would rather not have them in the image that we will link out to conference attendees in one hour. We have no time to test. Can we just add the requested R packages as a separate PR and build the image or are there actual bugs with the machine that this is fixing @davebx ? |
👍 This looks great! |
@martenson I have created pull request #84 based on master + these R additions. |