-
Notifications
You must be signed in to change notification settings - Fork 826
[bug 1035319] Switching from /vendor to peep. #2083
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
Conversation
|
You say "most of" vendor. What do you mean by that? Since the diff is so huge this time, it's hard to check for myself. NB: Since we would be installing from peep/pip every deploy now, compiled packages should be easier. It might make deploys take longer though. Or we could keep doing what we do now and not actually use peep to install the compiled stuff anyways. |
|
There's still a /vendor/tarballs package. It keeps ES/Redis and a couple other packages for installation. Additionally, I just realized I'll need to fix the deploy script before this hits dev I think. |
|
One one nice thing about having vendor/ is that when you switch branches or go look at someone else's stuff, the dependencies follow you. when With the new system, there's nothing to tell you when your virtual environment is out of sync with regards to the requirements files. With fjord, we added some code to tell you you're out of sync and it kicks off in Anyhow, the fjord code is here:
I'm definitely interested in better ways to do it. Regarding compiling dependencies, that requires the requirements and a compiler be installed on the admin node that's used for deployemnt. If I was a betting man, I would bet that the admin node does not have these things. Definitely worth checking, plus it's worth making sure it's ok with IT that we switch to compiling our own dependencies on deployment. |
|
Thanks for dropping those in here will, that's a nice addition! It might be worth throwing something like that into a library, as this is an issue everyone who uses a requirements file can run into. If I come up with a better way of doing it, I'll let you know too. As for deployment, we should probably let IT know our intentions so everyone is on the same page. |
|
travis fails. expected? |
|
Ha. That's definitely a problem. @dean: It looks like it's using the upstream django-mozilla-product-details. Did your patch land on that? |
|
I assume this is on stage now? All cron jobs are erroring out with: |
|
Yes, this is on stage. That is the kind of thing we wanted to check. It's probably a problem with virtualenv. We'll have to tweak the crontab to account for that. |
|
requirements/requirements_src.txt temporarily refers to a repo of mine, since the PR to fix jingo-minify is not currently merged. This needs to be fixed when the following PR is merged: |
|
Oh, all tests pass now too. |
82494e1 to
172bd05
Compare
785ef4c to
ed15d1e
Compare
This removes (most of) our /vendor directory and replaces the files with requirements files installable via peep[1]. Existing requirement files were also updated to be peep compatible, so we can use it for all of our package installations. Everything seems to work fine in my dev environment and all tests pass, other than some DeprecationWarnings.
Finally, PIL was replaced with Pillow since we were already planning on switching and all these changes will need to be heavily tested on dev anyways. #1984
r?