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  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.  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==184.108.40.206 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.
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.
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
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
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.