Skip to content

Experiences with Rietveld on Django

November 14, 2008

It’s been a while now that we’re running a Rietveld instance on Django at our company for internal reviews. Besides the basic setup as described in my article on Google’s App Engine site we’ve made some more modifications to integrate Rietveld into our infrastructure. Here’s a short description of what we’ve done and what problems we’ve had (and still have):

First off, we’re using PostgreSQL as the database backend. Thanks to Django the database setup was quite simple: We’ve just changed the database settings in settings.py, ran “manage.py syncdb” and that’s it. The first time we’ve ran this setup a few minor bugs regarding type conversion in gae2django came up, but they were fixed soon.

For authentication we’ve added a simple backend that talks to our company’s ActiveDirectory (using python-ldap). Implementing that was again quite easy, just follow Django’s documentation on how to write a custom authentication backend and let Django do the rest.

The most time consuming change was the integration with an existing web server. We’ve already had a lighttpd server running and Rietveld should live inside a subfolder on this server. Because Rietveld assumes to sit in a top-level directory we need to change all absolute URLs in Rietveld’s sources, templates and in the JavaScript. We still have some broken links in our setup, e.g.  using the keyboard shortcuts to navigate between deltas still doesn’t work and a few weeks ago there was a change in Rietveld (collapsed patch sets on the issue page are loaded with an AJAX call when expanded) that is still not ported to our setup yet. It’s just a matter of time to have this fixed.
Django runs in fcgi mode (threaded) and lighttpd acts as a proxy as described in the “Deployment” section of Django’s documentation.

In the first few days we’ve had some strange problems with sending emails. Some emails were lost. We had some troubles to find the actual bug, but after I’ve done some improvements to gae2django the problem was almost gone. Currently only the sender of an email doesn’t receive his own mail but this might be an issue related to the setup of our mail servers.

We’re using upload.py to create and update issues. To avoid confusions we’ve renamed it and changed the default settings for the login URL and the address of the review server. The master template has some slight modifications too: We removed the link to the issue creation form and to all repository related pages, added a little question mark pointing to a help page and we’ve added a nice header image to match our CI.

Updating this customized installation of Rietveld is done using a Makefile-driven patch system. First we fetch the latest Rietveld sources from it’s Subversion repository, then we update gae2django and apply the patches from this project required to get Rietveld running on Django. On top of that we apply the changes I’ve described above. Of course, this procedure still needs some work, but at least it works and keeping those patches in sync isn’t that complicated as you might think ,-)

But besides the email issue and some broken links due to our customized setup Rietveld runs pretty stable on Django.  Currently the Django server runs 3 weeks without any troubles even without a traceback ;-)

Finally a few notes on how code reviews were integrated in our workflow. When we’ve decided to use a code review tool, there were some concerns about the time needed to review other peoples code. But after a short period of getting comfortable with this tool – and now we feel very comfortable with it ;-) –  it turned out that it’s less effort to do a review than expected. Mostly one or two iterations are sufficient enough to eliminate the most obvious things and to give a LGTM. Only changes that have to go immediately into the code base are reviewed afterwards. Besides that, the knowledge about special parts of the code has spread and as another side-effect there’s now much more discussion about best-practices for some application specific tasks and the process of finding and defining them has accelerated noticeably.

Edit 2009-01-12: The email issue seems to be solved. Our Postfix had some troubles with multiple addresses given as in a single, comma separated string. Splitting this string into a list of string (addresses) solved the problem.

2 Comments
  1. June 16, 2009 9:49 pm

    I just use git for such patching — I have my local modifications in the branch, then I just update the master branch with upstream sources and then merge it with my local branch (or rebase my local branch to always have a set of clean patches). Git helps a lot with the merge, it’s much easier than to do it manually, if there are some conflicts.

    • June 17, 2009 9:05 am

      That’s right, Ondřej! I’m currently migrating my local patches to Mercurial to achieve the almost the same. It’s MqExtension seems to provide a nice way to rebase my local changes on a new version and in addition to version my local changes. The later is a nice feature of Mercurial queues as it allows my co-workers to maintain our Rietveld instance on their own :)

      Andi

Comments are closed.

%d bloggers like this: