search

yola / yodeploy

Python library for deployment
4 stars 0 forks source link

Ensure specified requirements are installed #181

Closed stefanor closed 7 years ago

stefanor commented 7 years ago

We previously just passed easy_install each requirement, in order, and didn't pay any attention to what it did. This lead to a lot of breakage. If a library was installed, as a dependency of another one, and was then listed again in requirements.txt, we'd happily overwrite the existing install, possibly violating a version constraint.

To avoid this in future:

  1. Ensure we aren't upgrading/downgrading a library, before we attempt to install it.
  2. Ensure all the specified requirements are still met, after the install completes.

This breaks many of our apps. Arguably, they were already broken, we just hadn't noticed. I'm trying to fix the majority of them, before we land this.

beck commented 7 years ago

Is it possible to install requirements in such a way where a package is not removed and end up with a venv with unmeet requirements?

Or to phrase that another way, doesn't the check for "removed" cover all cases the following "check_requirements" would catch?

Amenable to adding some coverage? ... something like: https://github.com/yola/yodeploy/compare/doug/capture-venv-fails?expand=1

It was writing ☝️ that made me think of the first question. I'm unable to cobble together a testcase where a venv passes the first check but not the second.

stefanor commented 7 years ago

Is it possible to install requirements in such a way where a package is not removed and end up with a venv with unmeet requirements?

No, but I don't really trust scraping output from installation. So I like having the double-check at the end.

And yes, those tests look great, I'm going to steal them.

snitch commented 7 years ago

:sparkles: No lint errors found. :sparkles:

beck commented 7 years ago

Code review is so much more fun when changes are committed on-top of eachother's work vs leaving comments on the pull.

I really enjoyed reading the changes @stefanor made to my tests.

stefanor commented 7 years ago

Not going to merge this until everything at Yola is ready for it.