launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/launchpad/bug-974617-5 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-974617-5/+merge/105893

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.

-- 
https://code.launchpad.net/~benji/launchpad/bug-974617-5/+merge/105893
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-974617-5 into lp:launchpad.

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