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

Pipenv #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Pipenv #1

wants to merge 4 commits into from

Conversation

kennethreitz
Copy link

@kennethreitz kennethreitz commented Apr 17, 2019

Sending this PR on behalf of @digitalocean.

tl;dr: We should use modern tools, if available :)

This solves many problems, which should be evident, but I'll note some of them:

  • seperating the "deps i want" vs. "deps i need" concerns
  • declares the greater Python version in the Pipfile (Pipenv's default, but optional, behavior)
  • uses Postgres instead of MySQL, which is far-and-away considered best practice within the Python community
  • easier maintinence

pip-tools does solve many of these problems itself, and supporting requirements.txt should still be a target. Using https://buildpacks.io should make this relatively simple though, as Heroku supports both Pipenv and requirements.txt (and setup.py, secretly) at the same time. Pipenv is the documented and recommended tool, however.

@kennethreitz
Copy link
Author

I also added a production web server (gunicorn) and pointed out how to run it with the [scripts] section. You could harness/parse this in your platform (Pipfile is a TOML file), or just use it as an example to update how it's executed elsewhere in your system.

@kennethreitz
Copy link
Author

One question: do you want Windows users to be able to run this locally?

@kennethreitz
Copy link
Author

If so, Gunicorn will need to be replaced with Waitress.

@kennethreitz
Copy link
Author

Also, does your router prevent SlowLoRiS attacks (e.g. do HTTP buffering?). If not, waitress would be a better choice anyway.

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

Successfully merging this pull request may close these issues.

1 participant