-
Notifications
You must be signed in to change notification settings - Fork 600
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
Simplified Installation Instructions with Pipenv #110
Conversation
|
||
[packages] | ||
future = "*" | ||
futures = "==3.1.1" |
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.
Typically wouldn't pin this requirement, but it is a bug in Pipenv:
Not sure what the roadmap to support Py27 is so may not be a long term issue, but worth calling out
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.
Have you considered baselining with pyenv with the pyenv-virtualenv plugin? This would accommodate a simplified install story as well as be a sole route to document running on all supported python versions?
I've got a Dockerfile working that builds a running container that is likely useful to the project. I'd contribute that; I'd just need to tweak some minor modifications and likely switch to an alpine base image instead of the custom internal one I use now. I have this with Supervisor managing the processes. I was considering have it fronted with an Nginx proxy; that could be added to the example container or a single container of it's own to complete that story. For simplicity and 'access' to new users, the former would be more simple I'd imagine. Finally i'm fixing some "problems" with tabby.py startup that I found needed to be simplified that should be generally useful. I'll look further on Tableau's site or in the project for contributing and so on.
How actively is the main branch being maintained? 1 of your merges took a month and that looks only to mainly be pep8 cleanups......
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 have not but at the same time I've never used that plug in. My reasoning for choosing pipenv was that it gives you both the virtual environment and package management all in one, and given the talk on it at PyCon this year I think its going to be well engrained in the future of the language. If you think this is better done with pyenv-virtualenv however you could always submit a PR to see what the team prefers :-)
I am not associated with Tableau in any way and therefore am not qualified to speak on the maintenance aspect, but I have gotten feedback on issues and PRs submitted so far
"809dc83" = {path = "./tabpy-client", edtiable = true} | ||
numpy = "*" | ||
|
||
[dev-packages] |
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.
One potential weakness here is that Pipenv doesn't support specifying multiple Python versions:
If someone is trying to develop on a non-supported Python version this wouldn't stop them from doing so, but as far as distributions are concerned you'd be guarding against that in the setup.py
requirements
@@ -0,0 +1,196 @@ | |||
{ |
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.
In a pretty active development environment this file may be volatile, so I'll admit I'm not entirely sure of the consequences of including/excuding from VCS.
That said, it is suggested by the Pipenv team to include so that's what I've done here:
|
||
```Batchfile | ||
|
||
pip install tabpy-server |
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 thought this was pretty confusing before as it muddles the process for using the source code vs a pre-built distribution. As you'll see in the changes I've created a separate Contributing guide for dealing with the source and simply updated the server.md
file to note how to deal with pre-buil distributions
Side note - that itself is admittedly kind of strange in the current iteration - would probably be better served with a CLI component than having to launch directly from site-packages
See #108 - this should greatly simplify the installation of packages by using Pipenv exclusively, instead of a mangled combination of conda / pip.
The user is of course still free to use whatever virtual environment and package configuration they'd like, but this should give simpler instructions to the 99% of users that just want something to work.