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

Adds support for custom conda channels in the config file #2507

Conversation

chaosphere2112
Copy link

A project I'm involved with (UV-CDAT/vcs) uses automodule, and our build is kind of gnarly (lots of misc. science tools), so we distribute conda packages to install everything. We're not on the main conda channel, though; we live at http://anaconda.org/uvcdat. In order to successfully build our API docs, we need to be able to specify the channels to use for conda to find our packages. The environment.yml file sometimes-sort-of-supports doing this itself, but the most reliable way is to manually specify what channels to search (using the -c option). I added support to the config file for a conda > channels list, and reworked the install process to handle it appropriately.

I also made the conda install use path-based environments, so it works more or less the same as virtualenv. I'm about 75% sure that the install_user_requirements function wasn't doing anything useful for the conda build (since creating the environment from a conda environment file installs all of the packages listed in it), so I removed the body of that function.

I was a little confused as to how to run tests; I wound up with:

DJANGO_SETTINGS_MODULE=readthedocs.settings.test py.test --ignore rtd

which got me a few failures (which all look path related). I'll happily run again given more input as to how to do so. I'm also happy to add tests as necessary.

@ericholscher
Copy link
Member

This needs a corresponding PR to the readthedocs-build package which does the validation of the YAML files. This one is similar: readthedocs/readthedocs-build#18

Otherwise this looks good.

@chaosphere2112
Copy link
Author

@ericholscher I'll try and get that done this week; sorry about the delay, I was at conferences for a couple of weeks (plus the holidays).

@chaosphere2112
Copy link
Author

@ericholscher I submitted readthedocs/readthedocs-build#20; it adds appropriate validation and testing for said validation. Let me know if I need to change things in either place.

@ericholscher ericholscher self-requested a review March 2, 2017 22:30
@ericholscher
Copy link
Member

Sorry this has been sitting for so long. I don't have a lot of familiarity with conda, and I'm on the fence as to whether this is a good change, mainly because I don't know a lot about it. There's a lot of chatter about this in #2378 regarding getting more resources with familiarity with conda -- but hopefully we can get this reviewed and merged in the next couple weeks.

@chaosphere2112
Copy link
Author

@ericholscher No worries! If you guys do wind up adding conda-forge, that'll very nearly work for us (we're working on pushing all of our stuff to conda forge right now, there's just a lot of nested dependencies). The security implications of this PR are not to be taken lightly; I've tried to logic my way through the threat vectors as much as possible, but it's a distinct possibility that something bad snuck through. I'd be happy to walk you through each of the individual parts, concerns, and mitigations that I went through (assuming I can dredge those up from my memory).

@humitos
Copy link
Member

humitos commented Dec 5, 2017

Hi @chaosphere2112, do we still need this?

The environment.yml file sometimes-sort-of-supports doing this itself, but the most reliable way is to manually specify what channels to search

Why the environment.yml file is not enough? I'm not following you there.

@humitos humitos added the Needed: more information A reply from issue author is required label Mar 23, 2018
@humitos
Copy link
Member

humitos commented Mar 23, 2018

@chaosphere2112 friendly ping :)

@chaosphere2112
Copy link
Author

chaosphere2112 commented Mar 23, 2018 via email

@humitos
Copy link
Member

humitos commented Mar 23, 2018

Thanks for your reply.

I'll will close this PR here since I think that using the environment.yml to create the env with conda should be enough since you can define the custom channels there.

Otherwise, in any case that this is not enough, feel free to let us know and reopen this branch.

@humitos humitos closed this Mar 23, 2018
@doutriaux1
Copy link

@humitos actually this is stil something we would want. At the moment we cannot host our doc because our docstring will need the vcs package which is hosted on our channel cdat for more info see: http://cdat.llnl.gov

@humitos
Copy link
Member

humitos commented Apr 18, 2018

@doutriaux1
Copy link

@humitos the channel is an official conda channel (cdat). But this mkaes me realize something, are you saying we can pass a conda environment yaml file to readthedocs? If so we're good we have such thing (see: https://github.com/CDAT/cdat/blob/master/conda/cdat-v80-nox_py2.Linux.yaml ) . My understanding was that we couldn't use ANY channel except the default anaconda channel, hence @chaosphere2112 PR here.

@stsewd
Copy link
Member

stsewd commented Apr 18, 2018

@doutriaux1 you need to use a rtd configuration file for conda support https://docs.readthedocs.io/en/latest/conda.html. Here is the full documentation for the configuration file https://docs.readthedocs.io/en/latest/yaml-config.html

@doutriaux1
Copy link

@stsewd sounds like exactly like what we need. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: more information A reply from issue author is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants