ODM2 / WOFpy

A server-side implementation of CUAHSI's Water One Flow service stack in Python.
http://odm2.github.io/WOFpy/
9 stars 9 forks source link

Use conda-forge only for extra packages #105

Closed ocefpaf closed 7 years ago

ocefpaf commented 7 years ago

@lsetiawan and @emiliom I notice that .travis.yml may be used as "instructions" to get an environment for development. Because of that this PR uses the conda-forge channel with more care by not adding it to the .condarc. This ensure that people won't compromise their envs with package versions that may not be 100% compatible with defaults and odm2. (See http://conda-forge.github.io/docs/conda-forge_gotchas.html#using-multiple-channels for more info on the possible problems from mixing channels.)

ocefpaf commented 7 years ago

This PR will require me to add a few packages to the ODM3 channel before we can merge it...

lsetiawan commented 7 years ago

@ocefpaf seems like the uwsgi package is missing causing travis to fail.. should I remove this from requirements.txt?

ocefpaf commented 7 years ago

@ocefpaf seems like the uwsgi package is missing causing travis to fail.. should I remove this from requirements.txt?

uwsgi is not a direct dependency listed in requirements.txt. (I am not sure what is demanding it btw.)

But the right thing to do is to add uwsgi, and others that will show up, in the odm2 channel. I am working on that now.

lsetiawan commented 7 years ago

I just added uwsgi to WOFpy requirements.txt yesterday.. not sure if you got that notification. Thanks.

ocefpaf commented 7 years ago

I just added uwsgi to WOFpy requirements.txt yesterday.. not sure if you got that notification. Thanks.

Ah. Now I see. Somehow I missed that PR... Either way the right thing to do is to add that package to the odm2 channel if we really need it.

I do not know how uwsgi is used by WOFpy. If that is a "dev" dependency maybe we should move it to requirements-dev.txt.

lsetiawan commented 7 years ago

@ocefpaf uwsgi will be used for production deployment of wofpy, so I just added that as a dependency for wofpy do you think that's bad practice, and user should just have it as an extra package to install themselves?

ocefpaf commented 7 years ago

@ocefpaf uwsgi will be used for production deployment of wofpy

Not sure what you mean by production here. uwsgi is not imported directly, right? If the user do need it to get a particular setup working then it should be optional. If not we don't have a choice but to leave it in the main requirement.txt file.

If we do go that route we make WOFpy incompatible with Windows b/c uwsgi does not work on Windows.

lsetiawan commented 7 years ago

Not sure what you mean by production here.

I meant when a user is trying to deploy wofpy within their own server. In order to do this, uwsgi is needed. I didn't know uwsgi doesn't work on windows. In that case, let's make uwsgi optional, and I'll note that within my documentation. Thanks!

ocefpaf commented 7 years ago

I meant when a user is trying to deploy wofpy within their own server

We can add a server extra and people installing with pip install wofpy[server] will get the right set of packages.

lsetiawan commented 7 years ago

Everything looks good on this pull request, so should I merge this first or should I change the requirements.txt first?

ocefpaf commented 7 years ago

Everything looks good on this pull request, so should I merge this first or should I change the requirements.txt first?

Let's wait for the uwsgi to make into the odm2 channel. I just merged https://github.com/ODM2/conda-recipes-ODM2/issues/55 and it should take a few minutes. I will restart the CI here and ping you once that is done.

ocefpaf commented 7 years ago

@lsetiawan it is passing now. Can you review/merge it?

lsetiawan commented 7 years ago

@ocefpaf It has been reviewed and merged.. should I remove uwsgi from requirements.txt?

ocefpaf commented 7 years ago

@ocefpaf It has been reviewed and merged.. should I remove uwsgi from requirements.txt?

I am working on adding that as an optional dependency right now.

lsetiawan commented 7 years ago

Okay. I'll let you work on that. Thanks! 😃