Merge lp:~benji/launchpad/bug-974617-5 into lp:launchpad

[Benji York]

This branch is the last fix (I hope) for bug 974617 which turned out to be two race conditions. One on starting pgbouncer (which has already been fixed) and one on LP reconnecting to that restarted pgbouncer.

This branch adds a loop that retries connecting until it succeeds or times out.

Lint: this branch fixes a few lint items and is now lint-free.

Tests: there are no tests for the test changed in this branch (which makes sense) but I interactively tested by wrapping the bouncer.start() call in a thread that waited a few seconds before starting. If those few seconds were less than the timeout, the test passed, if more, the test failed so I feel good about how this works.

[Gary Poster (gary)]

I asked benji about line 35--the technically superfluous urlopen--and he said it was about readability. That's fine by me, given the relative cost of that line in comparison to the test as a whole.

Thanks, Benji!

review: Approve

=== modified file 'lib/lp/services/webapp/tests/test_error.py'
--- lib/lp/services/webapp/tests/test_error.py	2012-04-24 18:09:54 +0000
+++ lib/lp/services/webapp/tests/test_error.py	2012-05-15 20:49:21 +0000
@@ -9,6 +9,7 @@
     DisconnectionError,
     OperationalError,
     )
+import time
 import transaction
 import urllib2
 
@@ -117,8 +118,24 @@
         # We keep seeing the correct exception on subsequent requests.
         self.assertEqual(httplib.SERVICE_UNAVAILABLE,
                          self.getHTTPError(url).code)
-        # When the database is available again, requests succeed.
+        # When the database is available again...
         bouncer.start()
+        # ...and Launchpad has succesfully connected to it...
+        retries = 10
+        for i in xrange(retries):
+            try:
+                urllib2.urlopen(url)
+            except urllib2.HTTPError as e:
+                if e.code != httplib.SERVICE_UNAVAILABLE:
+                    raise
+            else:
+                break
+            time.sleep(1)
+        else:
+            raise TimeoutException(
+                "Launchpad did not come up after {0} attempts."
+                    .format(retries))
+        # ...requests succeed again.
         urllib2.urlopen(url)
 
     def test_operationalerror_view(self):

=== modified file 'lib/lp/testing/fixture.py'
--- lib/lp/testing/fixture.py	2012-05-08 21:04:08 +0000
+++ lib/lp/testing/fixture.py	2012-05-15 20:49:21 +0000
@@ -25,7 +25,6 @@
     EnvironmentVariableFixture,
     Fixture,
     )
-import itertools
 from lazr.restful.utils import get_current_browser_request
 import oops
 import oops_amqp
@@ -132,7 +131,6 @@
         """Start PGBouncer, waiting for it to accept connections if neccesary.
         """
         super(PGBouncerFixture, self).start()
-        s = socket.socket()
         for i in xrange(retries):
             try:
                 socket.create_connection((self.host, self.port))
@@ -146,7 +144,6 @@
             raise PGNotReadyError("Not ready after %d attempts." % retries)
 
 
-
 class ZopeAdapterFixture(Fixture):
     """A fixture to register and unregister an adapter."""