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

Django: Dependency management #186

Closed
lemonsaurus opened this issue Feb 3, 2019 · 26 comments
Closed

Django: Dependency management #186

lemonsaurus opened this issue Feb 3, 2019 · 26 comments
Labels
status: planning Discussing details

Comments

@lemonsaurus
Copy link
Member

lemonsaurus commented Feb 3, 2019

The Django branch has done away with pipenv in favor of a setup.py and just setting up environments manually.

I understand that this is far faster for builds, but in my opinion that advantage comes at the cost of convenience. I also don't love that it's inconsistent with our dependency management for bot.

I'd like to hear more opinions about this though. It is a shame that pipenv is slowing down our builds by several orders of magnitude, but I do think that there is a solution for this to be found in some sort of clever caching, and I also don't know if 10 minute build times is a real problem.

@lemonsaurus lemonsaurus added the status: planning Discussing details label Feb 3, 2019
@lemonsaurus lemonsaurus changed the title Django: Requirements management Django: Dependency management Feb 3, 2019
@sco1
Copy link
Contributor

sco1 commented Feb 3, 2019

Would it be worth investigating Poetry as an alternative to Pipenv?

@lemonsaurus
Copy link
Member Author

Certainly worth investigating. If it provides the same utility as pipenv and has faster build times in a docker container, I'd be all for it.

@jb3
Copy link
Member

jb3 commented Feb 4, 2019

I copied across all the dependencies from the setup.py file into an example pyproject.toml which looks like this:

[tool.poetry]
name = "pydis-site"
version = "0.1.0"
description = "Django application for the Python Discord server."
authors = ["Your Name <you@example.com>"]
license = "MIT"

[tool.poetry.dependencies]
python = "^3.7"
django = "^2.1"
djangorestframework = "^3.9"
djangorestframework-bulk = "^0.2.1"
django-crispy-forms = "^1.7"
django-hosts = "^3.0"
django-environ = "^0.4.5"
django-filter = "^2.1"
psycopg2-binary = "^2.7"
uwsgi = "^2.0"

[tool.poetry.dev-dependencies]
flake8 = "^3.7"
flake8-bandit = "^2.1"
flake8-bugbear = "^18.8"
flake8-import-order = "^0.18.0"
flake8-string-format = "^0.2.3"
flake8-tidy-imports = "^2.0"
pep8-naming = "^0.8.0"
mccabe = "^0.6.1"
coverage = "^4.5"

[build-system]
requires = ["poetry>=0.12"]
build-backend = "poetry.masonry.api"

When running poetry install from no virtualenv it takes 1 minute and 4 seconds on my MacBook to install all the production dependencies and 1 minute 14 seconds to install the dev dependencies as well (I may have some caching going on but I tried to eliminate it).

@lemonsaurus
Copy link
Member Author

should be tested inside a docker, ideally.

@jb3
Copy link
Member

jb3 commented Feb 4, 2019

will give it a try now 👌

@jb3
Copy link
Member

jb3 commented Feb 4, 2019

@heavysaturn docker test complete. Assuming that the python:latest image was cached it took 2 minutes and 56 seconds to install poetry as well as deps & dev deps.

Dockerfile here:

FROM python:latest

COPY . /app
WORKDIR /app

RUN pip install poetry

RUN poetry install

CMD ["python", "-m", "application"]

@lemonsaurus
Copy link
Member Author

yeah that seems pretty fast. how does it compare to pipenv in usability? is the [tool.poetry] section meant to be able to replace the setup.py file for stuff like pypi deployments?

@jerbob
Copy link
Contributor

jerbob commented Feb 4, 2019

Yeah, according to their README, it looks like they intend to replace setup.py, requirements.txt, setup.cfg, MANIFEST.in and Pipfile. I've used poetry for a couple of projects, and can confirm that it's super easy to publish stuff to pypi (poetry publish). All we really need is the pyproject.toml and optionally a poetry.lock.

@lemonsaurus
Copy link
Member Author

that's cool, and perhaps we should consider using it for our PyPI django packages if it works well.

Anyway, I'd be okay with testing out poetry for this project, I guess. if it works well for us, we might consider replacing pipenv with poetry in our other projects.

Does anyone happen to remember why we were so opposed to poetry back in the day?

@jerbob
Copy link
Contributor

jerbob commented Feb 4, 2019

As for usability, I haven't really used poetry for testing or running projects locally (like pipenv's virtualenv functionality), so maybe someone who's more experienced with it can chip in.

@lemonsaurus
Copy link
Member Author

From what I can tell, poetry does not provide virtual environment integration at all, and expects you to handle this separately. Although it does "detect if you're in a virtual environment and install the packages accordingly". So in other words, it does not provide the functionality that pipenv does, and which I consider absolutely essential.

@jerbob
Copy link
Contributor

jerbob commented Feb 4, 2019

I've done a bit of digging in poetry issues, and it looks like it has commands for poetry run and poetry shell that expose its virtualenv, but they aren't really documented at all. I'll be able to do some experimenting with this tonight.

@jerbob
Copy link
Contributor

jerbob commented Feb 4, 2019

Ah, looks like they're documented here: https://poetry.eustace.io/docs/cli/

@jerbob
Copy link
Contributor

jerbob commented Feb 4, 2019

It looks like poetry run even supports scripts in the pyproject.toml, like pipenv does:

[tool.poetry.scripts]
my-script = "my_module:main"
poetry run my-script

https://poetry.eustace.io/docs/cli/#run

@lemonsaurus
Copy link
Member Author

I think run and shell are just interacting with an already created virtual environment, though. Which means people will still have to create their own virtual environments. but yeah, test it tonight and I guess we'll see.

@sco1
Copy link
Contributor

sco1 commented Feb 4, 2019

It should create an environment if none exists:

... Poetry will check if it's currently inside a virtualenv and, if not, will use an existing one or create a brand new one for you ...

@lemonsaurus
Copy link
Member Author

mmmm.. okay that sounds good then. my mistake, that seems fairly clear cut.

@jb3
Copy link
Member

jb3 commented Feb 4, 2019 via email

@scragly
Copy link
Contributor

scragly commented Feb 4, 2019

I'm extremely green when it comes to poetry, but when I did some testing a couple of months ago, pycharm and vscode did not detect venvs that poetry created. This is different from pipenv, which has better IDE/editor venv detection support.

This can be resolved with either:

  • Setting settings.virtualenvs.in-project to true. This places the .venv directly in project root which then is detected without issue. This seems to be a global setting though, not per-project.
  • Copying the venv path poetry that's output with poetry show -v and manually setting the venv path project setting in the IDE/editor.

This is mainly noteworthy due to editors using venvs for 3rd party lib autocompletion and typically having a set of dev requirements for our projects such as specific linting tools and addons.

Poetry integrations are still in the discussion stage it seems for Pycharm and VSCode.

@sco1
Copy link
Contributor

sco1 commented Feb 4, 2019

RE: lemon's earlier question:

Does anyone happen to remember why we were so opposed to poetry back in the day?

Looking through the chat logs, overall it seems like it was mostly a case of not being unhappy enough with pipenv to make the switch. There were a couple instances of issues with dependency resolution (at least with d.py) when gdude was trying it out, but that was also last July and whatever issues he encountered don't seem to still be present (not that pipenv is without issues). Anecdotally, I switched my personal bot to poetry from requirements.txt/setup.py without any issue and we share many of the same dependencies.

@scragly
Copy link
Contributor

scragly commented Feb 4, 2019

I seen poetry shell was mentioned earlier also, so I figured I'd mention there's currently an ongoing issue on environmental variables being overwritten on shell initialisation when using poetry shell.

While the venv is activated, there's strange behaviour such as which python showing the original system python instead of the venv's one. This is thanks to any environmental settings loaded from the venv being overwritten by shell profile settings after the subshell is spawned, due to incorrect load order.

This link will take you to an amazingly comprehensive comment on the relevant issue ticket which breaks down the problem and a proposed method to solve it, but there's no PR yet in the works as far as I can see and the ticket has not received any activity for two months.

@lemonsaurus
Copy link
Member Author

@scragly I think settings.virtualenvs.in-project should be true anyway. I've always been using the pipenv equivalent setting, and I've instructed most of our contribs to do the same.

as for poetry shell, does anyone really use this? We've always relied on run scripts or IDE integration.

I do wish it had pycharm integration, but to be honest the pipenv integration in pycharm is far from perfect either. I frequently have to manually select the interpreter from my .venv folder because the integration fails to work properly.

I will say though that I'm not really that bothered by pipenvs performance either, but if others are then poetry sounds like a roughly equivalent alternative, with some advantages and some disadvantages.

@lemonsaurus
Copy link
Member Author

lemonsaurus commented Feb 4, 2019

I also hate using the setup.py for the django packages we publish to PyPI, and would be happy if we could have a single tool across all our repos that solved both publishing, dependency locking, and virtual environment creation. That, plus faster build times, seems like a worthwhile trade overall for what seems like minor inconveniences in return. And I know that some of the members of our dev team (including @jchristgit who is spearheading the Django rewrite) are really in hate with pipenv, so I'd be happy to entertain a competent alternative if only so they'll be happy, too.

For these reasons, I think we should give this a try and see how it works out. we can always change our minds later, this is hardly a major change.

@jchristgit
Copy link
Member

sure, i'll add it this week

@jchristgit
Copy link
Member

poetry is live on django branch

@jchristgit
Copy link
Member

I believe this can be closed

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

No branches or pull requests

6 participants