We need the following Puppet fixups on reviewboard-hg to make Review Board ready for production: 1) Install RBTools system package 2) Add [format] generaldelta=true to /etc/mercurial/hgrc RBTools is flat out missing. You get a Python module import failure when pushing reviews. format.generaldelta is a Mercurial optimization. It will more efficiently encode repository data for many-headed repositories. It has performance downsides for clone. But nobody should clone the review repos, so we don't care. *It is important this is only set on the review servers, not hg.mozilla.org.*

3) The self-referential URL in the Review Board API is http, not https. (I'm not sure how to fix this - it might be a Django settings thing.)

The http:// vs https:// URL likely to do with the fact that traffic arriving to Review Board has already been decrypted. Here is the code Django uses to construct self-referential URIs: https://github.com/django/django/blob/75d03c775553b9778db513213376d335d23819a8/django/http/request.py#L105 There is some magic at https://github.com/django/django/blob/75d03c775553b9778db513213376d335d23819a8/django/http/request.py#L122 that looks for the SECURE_PROXY_SSL_HEADER setting. The docs for this setting are at https://docs.djangoproject.com/en/dev/ref/settings/#secure-proxy-ssl-header. Essentially, we need to set this setting to a 2-tuple of (<header whose presence signals originally ssl>, 'https'). I'm not sure what this would be for us. Alternatively, we can set the HTTPS environment variable to force generation of https:// URLs.

I have a suspicion that the mis-configured self-referential URLs are what's causing the exceptions on push. We see the following cryptic error during push: remote: File "/usr/lib64/python2.6/site-packages/mercurial/wireproto.py", line 501, in dispatch remote: return func(repo, proto, *args) remote: File "/repo/hg/version-control-tools/hgext/reviewboard/hgrb/proto.py", line 204, in reviewboard remote: cookie=bzcookie) remote: File "/repo/hg/version-control-tools/hgext/reviewboard/hgrb/proto.py", line 37, in post_reviews remote: return pr(*args, **kwargs) remote: File "/repo/hg/version-control-tools/pylib/reviewboardmods/reviewboardmods/pushhooks.py", line 155, in post_reviews remote: squashed_rr.get_diffs().upload_diff(commits["squashed"]["diff"]) remote: AttributeError: 'ListResource' object has no attribute 'get_diffs' abort: unexpected response: empty string I temporarily modified pushhooks.py on the server and confirmed that the URL of objects returned is http://, not https://. I suspect RBTools isn't handling the HTTP 301 from http://reviewboard.mozilla.org/ gracefully.

(In reply to Gregory Szorc [:gps] from comment #0) > We need the following Puppet fixups on reviewboard-hg to make Review Board > ready for production: > > 1) Install RBTools system package r96150, assuming the epel version is sufficient (0.6 vs 0.62). > 2) Add [format] generaldelta=true to /etc/mercurial/hgrc r96149 (In reply to Gregory Szorc [:gps] from comment #2) > There is some magic at > https://github.com/django/django/blob/ > 75d03c775553b9778db513213376d335d23819a8/django/http/request.py#L122 that > looks for the SECURE_PROXY_SSL_HEADER setting. The docs for this setting are > at > https://docs.djangoproject.com/en/dev/ref/settings/#secure-proxy-ssl-header. > > Essentially, we need to set this setting to a 2-tuple of (<header whose > presence signals originally ssl>, 'https'). I'm not sure what this would be > for us. Easily done by zeus: http.setHeader("X-Forwarded-Proto", "https"); Added to settings_local.py in r96152.

OK. Issues reported last night are fixed. That enabled me to find some more: 4) /data/www/reviewboard.mozilla.org/reviewboard/logs is not writable by the HTTP server 5) Aforementioned logs directory appears to get blown away during rsync --delete? We should move lots to somewhere persistent. 6) /data/www/reviewboard.mozilla.org/venv/bin/hg has a wrong python binary in the shebang 7) Review Board is unable to find the `hg` binary. We need to ensure os.environ['PATH'] contains /data/www/reviewboard.mozilla.org/venv/bin. We can probably muck this variable in reviewboard.wsgi.

For logging, we probably want to include something in settings_local.py. We should probably eventually set the LOGGING variable )https://docs.djangoproject.com/en/1.6/topics/logging/). However, the quick and appropriate fix is to set LOGGING_DIRECTORY = '/path/to/directory'. I made this change via the admin UI at https://reviewboard.mozilla.org/admin/settings/logging/ to unblock me. We should make the change permanently in Puppet.

For #7, I manually added the following line to reviewboard.wsgi: os.environ['PATH'] = '/data/www/reviewboard.mozilla.org/venv/bin:' + os.environ.get('PATH', '') After applying this fix, I was successfully able to push a review to production \o/

8) rbmozui extension/package is out of date

9) We noticed that primary keys for review requests are incrementing by 2. Not sure if that is by design or accidental. 1 would be preferred. But it shouldn't be a huge deal.

(In reply to Gregory Szorc [:gps] from comment #6) > For logging, we probably want to include something in settings_local.py. We > should probably eventually set the LOGGING variable > )https://docs.djangoproject.com/en/1.6/topics/logging/). However, the quick > and appropriate fix is to set LOGGING_DIRECTORY = '/path/to/directory'. > > I made this change via the admin UI at > https://reviewboard.mozilla.org/admin/settings/logging/ to unblock me. We > should make the change permanently in Puppet. Is LOGGING_DIRECTORY actually honored by reviewboard? I'm fairly certain the log directory setting in the admin gui is stored in the database.. In any case, I've changed it to log to /var/log/reviewboard/. It's a vastly more obvious location. That dir and the associated logrotate conf is in puppet.

(In reply to Gregory Szorc [:gps] from comment #9) > 9) We noticed that primary keys for review requests are incrementing by 2. > Not sure if that is by design or accidental. 1 would be preferred. But it > shouldn't be a huge deal. This is almost certainly for db replication. We do the same for BMO and other sites that (will eventually) have a failover site in PHX1.

(In reply to Gregory Szorc [:gps] from comment #8) > 8) rbmozui extension/package is out of date Yes, we had a request yesterday to update rbmozui and RB on dev and prod, but ran into rbmozui packaging issues. Updated rbmozui to 0.2.3beta0.

(In reply to Gregory Szorc [:gps] from comment #5) > > 6) /data/www/reviewboard.mozilla.org/venv/bin/hg has a wrong python binary > in the shebang this *seems* to survive making the venv relocatable. the alternative is a system install of mercurial. preference? > 7) Review Board is unable to find the `hg` binary. We need to ensure > os.environ['PATH'] contains /data/www/reviewboard.mozilla.org/venv/bin. We > can probably muck this variable in reviewboard.wsgi. I don't see an obvious way of making this permanent; reviewboard bug? I expect that rb-site will replace it on every subsequent update.

(In reply to Kendall Libby [:fubar] from comment #13) > (In reply to Gregory Szorc [:gps] from comment #5) > > > > 6) /data/www/reviewboard.mozilla.org/venv/bin/hg has a wrong python binary > > in the shebang > > this *seems* to survive making the venv relocatable. the alternative is a > system install of mercurial. preference? When it comes to Python, I will almost always favor running things from virtualenvs than from the system package (I have a severe distrust of OS packages for Python packages). > > 7) Review Board is unable to find the `hg` binary. We need to ensure > > os.environ['PATH'] contains /data/www/reviewboard.mozilla.org/venv/bin. We > > can probably muck this variable in reviewboard.wsgi. > > I don't see an obvious way of making this permanent; reviewboard bug? I > expect that rb-site will replace it on every subsequent update. Oh, does the .wsgi not come from Puppet. If not, then I guess changing it would be a bit haphazard. Idea A: Create a wrapper .wsgi script that sets os.environ['PATH'] and does an execfile() on reviewboard.wsgi Idea B: SetEnv in Apache. You can do this at the vhost level, so it should be safe.

(In reply to Gregory Szorc [:gps] from comment #14) > > this *seems* to survive making the venv relocatable. the alternative is a > > system install of mercurial. preference? > > When it comes to Python, I will almost always favor running things from > virtualenvs than from the system package (I have a severe distrust of OS > packages for Python packages). yeah, that's what I figured. I'll add the relocatable bit to the deploy docs that I still need to put on the wiki. > Idea A: > > Create a wrapper .wsgi script that sets os.environ['PATH'] and does an > execfile() on reviewboard.wsgi > > Idea B: > > SetEnv in Apache. You can do this at the vhost level, so it should be safe. http://httpd.apache.org/docs/2.2/mod/mod_env.html#setenv "On 2.2, the PATH environment variable cannot be set using Setenv." created a wrapper and puppet bits in r96205.

Given that we are already using production, I don't think this qualifies as a blocker for "initial deployment" (meaning wider announcements), as important as it is for on-going maintenance, but correct me if I'm wrong.

I think we addressed everything in this bug. fubar can reopen if not.