We shouldn't install prod.txt

RESOLVED FIXED

Status

Release Engineering
Applications: Balrog (frontend)
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: peterbe, Assigned: peterbe)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

4 years ago
All the non-binary dependencies (e.g. Flask, flask-wtf, etc) are in `./vendor/`. That's great. However, we shouldn't install these in the virtualenv too. There are no clear instructions for installing `requirements/prod.txt` but you can't do development without doing `pip install -r requirements/dev.txt` and this file imports `prod.txt` in turn [0] so you can't avoid it. 

Also, when running locally (e.g. `python admin.py -d mysql://...`) we don't insert the `./vendor/` directory at all. And when we run the wsgi server we add the `./vendor/` directory at the BOTTOM of the PYTHONPATH meaning that the virtualenv comes first. 

Possibly two fixes to do:

1) Delete prod.txt because it's just a liability of becoming out-of-sync with the contents of `./vendor/lib/python/`

2) Make sure both local dev AND wsgi running inserts `./vendor/` FIRST in the PYTHONPATH. I.e. find the common denominator between the dev and prod way of starting the server and insert the necessary `site.adddir('vendor/lib/python')` there. 


[0] https://github.com/mozilla/balrog/blob/6a9bc560b42a28251f8eab8a814ac5e68fb811a5/requirements/dev.txt#L1
100% agreed.

> 1) Delete prod.txt because it's just a liability of becoming out-of-sync
> with the contents of `./vendor/lib/python/`

I think it would be bad to do this - it's how we keep track of what versions of things we're actually running.

> 2) Make sure both local dev AND wsgi running inserts `./vendor/` FIRST in
> the PYTHONPATH. I.e. find the common denominator between the dev and prod
> way of starting the server and insert the necessary
> `site.adddir('vendor/lib/python')` there. 

We're already doing this, so I'm surprised this is an issue. Eg:
https://github.com/mozilla/balrog/blob/master/admin.py#L7

For me, as long as I've installed the compiled.txt and dev.txt packages it works fine, eg:
(newbalrog)➜  balrog git:(master) ✗ pip freeze
Jinja2==2.5.5
MySQL-python==1.2.3
Paste==1.7.5.1
SQLAlchemy==0.7.1
argparse==1.2.1
certifi==14.05.14
coverage==3.7.1
mock==1.0.1
nose==1.3.4
pyflakes==0.8.1
requests==2.4.3
simplejson==2.0.9
wsgiref==0.1.2
(newbalrog)➜  balrog git:(master) ✗ python admin.py
2014-11-27 11:10:48,323 - INFO - PID: 11325 - Request: None - werkzeug._log#87:  * Running on http://127.0.0.1:9000/

Are you getting different behaviour?


One bad thing here is that dev.txt includes prod.txt, which makes it pretty easy to accidentally install all the prod packages. I think we should remove that line.
(Assignee)

Comment 2

4 years ago
The problem I see with keeping prod.txt is because it's dangerous to get that out of sync with what is actually installed. If the upkeep of the vendor directory was entirely automated with a good script I wouldn't be so worried. I've seen it many a times in playdoh projects that the intention is to keep prod.txt to reflect what's in vendor/.

Ok. Let's at least start with getting rid of the pointer to it from dev.txt.

Secondly, I didn't see that admin had the same site.addsitedir() stuff as the wsgi script. Either way, it would be best if it wasn't repeated code and it should put the ./vendor/lib/python directory at the TOP of the PYTHONPATH so a rogue virtualenv doesn't take precedence and potentially cause confusion.
(Assignee)

Comment 3

4 years ago
Let's keep prod.txt for now but I'm going to fix the addsitedir and the reference from dev.txt
Assignee: nobody → peterbe
Status: NEW → ASSIGNED

Comment 5

4 years ago
Commits pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/3f67f603960e0e91ae624a469baa847c2bd02bbf
bug 1105421 - don't pull in prod.txt from dev.txt

https://github.com/mozilla/balrog/commit/998802a8c09d98496ba9f2ecedda6718cbffd600
bug 1105421 - add vendor libs first to sys.path

https://github.com/mozilla/balrog/commit/89522e257d7fc54590baa5c6e4db76b41356030f
bug 1105421 - amend travis.yml file to use vendor/lib/python

https://github.com/mozilla/balrog/commit/96f589dc2632f6e5546eee62fa8245275e8cf7e5
Merge pull request #18 from peterbe/bug-1105421-we-shouldnt-install-prodtxt

Bug 1105421: we shouldn't install prod.txt. r=bhearsum
Depends on: 1108573

Comment 6

4 years ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/0fee4ddb90991d233d53ec44d743780d266a703b
Bustage fix for bug 1105421 (make sure to add auslib to sys.path before trying to import from it).
Pushed to prod in bug 1108573.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.