578703 - [dashboard] make django-site work in virtualenv/pip setups

Status: RESOLVED FIXED

(Mozilla Localizations :: Infrastructure, defect, P2)

Description

[Axel Hecht [:Pike]]

Using virtualenv and pip should make our infrastructure easier to set up, test, and update.

We should get this done before we try to migrate to django 1.2, too.

Comment 1

[Axel Hecht [:Pike]]

Hrm. The python-pip package isn't available for jaunty, aka ubuntu 9.04. It is for 10.04. Should we ask IT to update?

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

yeah, especially 10.04 is a LTS

Comment 3

[Axel Hecht [:Pike]]

Attachment: add a requirements.txt for the dashboard

First half of it, add a requirements.txt for django-site.

I tested this locally, seemed to work OK.

Stas, would you be up for a review?

Comment 4

[Staś Małolepszy :stas]

Comment on attachment 464071 [details] [diff] [review]
add a requirements.txt for the dashboard

r+ with a nit (below). Thanks Pike :)

>diff --git a/requirements.txt b/requirements.txt

>+Twisted

There's a "twisted" directory inside django-site, and pip gets confused. If you run `pip install -r requirements.txt` from django-site/, pip will think that the twisted directory in django-site is an installable package and will complain about it (and abort).

Maybe you could put requirements.txt inside l10n_site/ ? We keep the *-example.py files there already, seems like its only fitting to put requirements.txt there as well. If you run `pip install -r l10n_site/requirements.txt` from django-site pip will still complain, though.

(Or even better yet (might be a different bug though), put all those files in l10n_site/docs or something like that.)

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 464071 [details] [diff] [review]
add a requirements.txt for the dashboard

Checked in, http://hg.mozilla.org/l10n/django-site/rev/8b7108ebeed4.

Leaving open for l10n-master.

Comment 6

[Staś Małolepszy :stas]

Hijacking this bug to work on the wsgi part of the virtualenv setup. A trick that I've been using successfully is to add these lines somewhere near the top of the wsgi application script:

INTERP = "/path/to/env/bin/python"
if sys.executable != INTERP: os.execl(INTERP, INTERP, *sys.argv)

I'll create a patch with an example.

Comment 7

[Staś Małolepszy :stas]

Attachment: django.wsgi

This should work. I've used a similar file when setting up my own instance of the dashboard on dreamhost.

Comment 8

[Staś Małolepszy :stas]

Attachment: django.wsgi, the patch

Sorry, I accidentally attached the file itself, instead of the patch.

Comment 9

[Staś Małolepszy :stas]

I tested this file at http://elmo.dev.stasmade.com/ (I set it up from scratch using only pip). I was also able to upgrade to 1.1.3 with a single `pip install` command.

However, all this proves is that such a wsgi file will work on a dreamhost server. I'm not able to say if it will work on our Ubuntu VM.

Comment 10

[Staś Małolepszy :stas]

Attachment: Some changes to settings to make things work in my instance

I had to make the changes listed in this patch to make the instance at http://elmo.dev.stasmade.com/ work.

Comment 11

[Staś Małolepszy :stas]

Pike, can I install virtualenv on bm-l10n-dashboard01? :)

Comment 12

[Staś Małolepszy :stas]

Attachment: django.wsgi, patch 2

I took a different route with this patch, as the previous one wasn't working as expected. I followed the docs at https://code.google.com/p/modwsgi/wiki/VirtualEnvironments and the example of our AMO webdevs (https://github.com/jbalogh/zamboni/blob/master/wsgi/zamboni.wsgi).

The patch assumes that the virtual env was created in the same directory as django-site. 

Here are the exact steps of what I did on bm-l10n-dashboard01:/home/dashboard

1. cd test
2. virtualenv env
   (I didn't use the --no-site-packages options as that would require installing MySQL-python in the env, which in turn requires the MySQL Client.)
3. source env/bin/activate
4. pip install -r django-site/l10n_site/requirements.txt
   (This installed Django 1.1.2 and compare-locales 0.9.1. Other required packages are already installed system-wide and their versions match.)

The new django.wsgi runs here: https://l10n-stage-sj.mozilla.org/test

Comment 13

[Axel Hecht [:Pike]]

Comment on attachment 501055 [details] [diff] [review]
django.wsgi, patch 2

There are a few things in this patch that rub me backwards.

Firstly, can you detail on why the simple method proposed in one of your two links doesn't work for us?

More comments on the actual patch inline.

>diff --git a/django.wsgi b/django.wsgi
>new file mode 100644
>--- /dev/null
>+++ b/django.wsgi
>@@ -0,0 +1,27 @@
>+import os
>+import sys
>+import site
>+
>+ROOT = os.path.dirname(os.path.abspath(__file__))
>+path = lambda *a: os.path.join(ROOT, *a)
>+prev_sys_path = list(sys.path)
>+
>+LOCALDIRS = (
>+    ROOT,
>+    path(ROOT, '..', 'env', 'lib', 'python2.6', 'site-packages'),

Now you're having ROOT in there twice. Also, we shouldn't hard-code python2.6, really.

Comment 14

[Staś Małolepszy :stas]

(In reply to comment #13)
> Comment on attachment 501055 [details] [diff] [review]
> django.wsgi, patch 2
> 
> There are a few things in this patch that rub me backwards.
> 
> Firstly, can you detail on why the simple method proposed in one of your two
> links doesn't work for us?

The reason for all the juggling is that normally, site.addsitedir adds to the end of sys.path which means that any system-wide libs will mask them. In order to give precedence to the virtualenv's libs, we need to put the relevant paths at the beginning of sys.path.

The advantage the approach from the patch has over simply doing:
  sys.path.insert(0, path)
is that site.addsitedir also parses and pth files, while sys.path.insert doesn't.



> >+LOCALDIRS = (
> >+    ROOT,
> >+    path(ROOT, '..', 'env', 'lib', 'python2.6', 'site-packages'),
> 
> Now you're having ROOT in there twice. Also, we shouldn't hard-code python2.6,
> really.

Well, no, I have ROOT once (which is the contents of django-site directory), and I have the path to the environment's libs.

Hardcoding python2.6 could be only avoided by creating a symlink to it in ../env/lib/python.

Comment 15

[Axel Hecht [:Pike]]

For the path shuffling, I'd prefer what easy-install.pth does, 

import sys; new=sys.path[sys.__plen:]; del sys.path[sys.__plen:]; p=getattr(sys,'__egginsert',0); sys.path[p:p]=new; sys.__egginsert = p+len(new)


path(ROOT, '..', 'env', 'lib', 'python2.6', 'site-packages')
has ROOT twice, as the path lambda already prefixes ROOT.

For the site-packages path, I'd rather have you detect the python version like site.py,

        USER_SITE = os.path.join(USER_BASE, "lib",
                                 "python" + sys.version[:3],
                                 "site-packages")

Comment 16

[Staś Małolepszy :stas]

(In reply to comment #15)
> For the path shuffling, I'd prefer what easy-install.pth does, 
> 
> import sys; new=sys.path[sys.__plen:]; del sys.path[sys.__plen:];
> p=getattr(sys,'__egginsert',0); sys.path[p:p]=new; sys.__egginsert = p+len(new)

Hrm.   That's very much unreadable.  The other solution is what wsgi guys suggest and what our webdev uses.  I'd still vote for that one.

> path(ROOT, '..', 'env', 'lib', 'python2.6', 'site-packages')
> has ROOT twice, as the path lambda already prefixes ROOT.

Ah, now I see what you meant :)  I'll fix that, thanks.


> For the site-packages path, I'd rather have you detect the python version like
> site.py,
> 
>         USER_SITE = os.path.join(USER_BASE, "lib",
>                                  "python" + sys.version[:3],
>                                  "site-packages")

OK, I can do that.

Comment 17

[Axel Hecht [:Pike]]

Sounds reasonable.

Comment 18

[Axel Hecht [:Pike]]

stas, do you mind if I finish this up while you're drowning in web projects? Being able to update to 1.3 (yeah, it's in RC and I'd go for that) directly for pushes2 would be nice, as the CSRF stuff needs changes for all POST requests.

Comment 19

[Staś Małolepszy :stas]

If I prepare a new version of the patch in a few, would that be helpful?  Otherwise, I don't mind of course :)

Comment 20

[Staś Małolepszy :stas]

Attachment: django.wsgi, patch 3

Axel, how does this look like to you?

Comment 21

[Axel Hecht [:Pike]]

Comment on attachment 517437 [details] [diff] [review]
django.wsgi, patch 3

yep, let's go with this.

Comment 22

[Axel Hecht [:Pike]]

FWIW, I'm not too fond of the hard-coding of where the env is relative to django-site, but I haven't come up with anything better :-(

Comment 23

[Staś Małolepszy :stas]

Thanks Pike.

(In reply to comment #22)
> FWIW, I'm not too fond of the hard-coding of where the env is relative to
> django-site, but I haven't come up with anything better :-(

We could have a shell variable, but I'm not sure where to set it up to make sure it's defined in case of a server restart.

Also, there is virtualenvwrapper, but that also relies on exporting stuff in ~/.bashrc.

Axel, do you want to coordinate the landing of the patch?

Here are the steps to follow:

0. commit and push.

1. cd /home/dashboard/site
2. virtualenv env
   (I didn't use the --no-site-packages options as that would require
installing MySQL-python in the env, which in turn requires the MySQL Client.)
3. source env/bin/activate
4. pip install -r django-site/l10n_site/requirements.txt
   (This should install Django 1.1.2 and compare-locales 0.9.1. Other required
packages are already installed system-wide and their versions match.)

5. cd django-site
6. hg pull -u

Comment 24

[Axel Hecht [:Pike]]

I wouldn't be surprised if step 6 failed due to an untracked local file. Probably want to do 

hg pull
mv django.wsgi backup.wsgi&&hg update

instead. Should we bump the requirements for django to 1.1.4 right away?

Comment 25

[Axel Hecht [:Pike]]

... for steps up to 4 incl we don't need to coordinate, right?

Comment 26

[Staś Małolepszy :stas]

You're right, thanks for catching it.

Also, if you choose to use a different name in step 2 for the env (in lieu of 'env'), please remember to edit ENV_PATH in the new django.wsgi.

Can you file a new bug for 1.1.4?  Looks like there was one incompatible change compared to 1.1.3 re CSRF and AJAX requests [1] and I'd prefer to test how that works with shipping and signing off.

http://docs.djangoproject.com/en/1.2/releases/1.1.4/

Comment 27

[Staś Małolepszy :stas]

(In reply to comment #25)
> ... for steps up to 4 incl we don't need to coordinate, right?

No.  Want me to land that patch now?

Comment 28

[Axel Hecht [:Pike]]

Comment on attachment 517437 [details] [diff] [review]
django.wsgi, patch 3

landed as http://hg.mozilla.org/l10n/django-site/rev/52ef5eef7e0a.

I think there's still the migration of l10n-master open for this bug.

Comment 29

[Staś Małolepszy :stas]

I just did steps 1-4, too.

The env is installed in /home/dashboard/site/env.

"Successfully installed compare-locales Django".

Comment 30

[Staś Małolepszy :stas]

Hm. So we landed the patch from bug 639404 and felt good about trying it out on l10n-stage-sj.  However, we weren't able to as long as the process from this bug remained open.

I went ahead and made the call that it was OK to hg up.  We archived the old version of django.wsgi as django-old.wsgi.  It works :)

Leaving open for l10n-master, although would you mind filing a followup for that please?

Comment 31

[Staś Małolepszy :stas]

I filed bug 640138 for the l10n-master part.

Comment 32

[Staś Małolepszy :stas]

Marking fixed :)