Merge lp:~milo/linaro-patchmetrics/bug1012538 into lp:linaro-patchmetrics

[Milo Casagrande]

Hi,

I'm proposing the following merge in order to address, if not completely, the 503 HTTP error we sometimes get from Launchpad. Since we cannot make anything else about it, I added a retry loop with a sleep timeout in order to have a little bit of time between one retry and the other.

At the moment, the retries are fixed at a maximum of 3, and the timeout is set to 30 seconds.

[Deepti B. Kalakeri (deeptik)]

I guess the bug https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290 mentioned in the following
code is not fixed yet . Have you tested this, that email parsing code does not insert any invalid email address in the first place ?

69 + else:
70 + # XXX We should not encounter bad email addresses.
71 + #https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290
72 + person = None # Launchpad lookup failed.
73 + logger.warning("Launchpad rejected email lookup and "
74 + "returned HTTPError. Offending email "
75 + "address: %s" % email)

If the above code has been tested for bug https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290 to be fixed, then everything looks good +1.
Also, use FIXME: instead of XXX when you merge this code.

[Milo Casagrande (milo)]

Hi Deepti,

now that you point it out, no, this merge request has not been tested WRT bug 967290, and the logic of that part needs to be rethought.

Thanks.

[lp:~milo/linaro-patchmetrics/bug1012538]

366. By Milo Casagrande on 2012-06-29

Fixed retries logic.

[Milo Casagrande (milo)]

Hi,

the latest push should fix the logic of that part, basically reimplementing it as it was before, with two different try statements.

[Deepti B. Kalakeri (deeptik)]

+1 Looks good.

review: Approve

=== modified file 'apps/patchmetrics/utils.py'
--- apps/patchmetrics/utils.py	2012-04-03 13:16:22 +0000
+++ apps/patchmetrics/utils.py	2012-06-29 15:50:27 +0000
@@ -6,6 +6,7 @@
 from patchwork.models import Person
 from patchmetrics.models import Team
 import urllib2
+import time
 from xml.dom.minidom import parseString
 from lazr.restfulclient.errors import HTTPError
 
@@ -13,6 +14,11 @@
 logging.basicConfig()
 logger = logging.getLogger()
 
+# Time in seconds to wait before performing another HTTP request.
+REQUEST_WAIT = 30
+# Max retries if something goes wrong, but it can be recoverd just retrying.
+MAX_RETRIES = 3
+
 
 def sync_memberships(memberships):
     """Make sure the given memberships are represented in our database.
@@ -151,23 +157,38 @@
         try:
             person = get_lp_person_by_email(lp, email)
         except HTTPError:
-            # XXX We should not encounter bad email addresses.
+            # FIXME We should not encounter bad email addresses.
             # https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290
             person = None  # Launchpad lookup failed.
             logger.warning("Launchpad rejected email lookup and returned "
-                           "HTTPError. Offending email address: %s" % email)
+                            "HTTPError. Offending email address: %s" % email)
 
         if not person:
-            continue  # No Launchpad entry for this person.
-        member_of = is_member(person, teams)
-        for team in member_of:
-            if not team in memberships:
-                memberships[team] = {}
-
-            memberships[team][person] = [email]
+            # No Launchpad entry for this person.
+            continue
+
+        # Counter for the retries to perform in case of an HTTP error 503.
+        retries_count = 0
+        while True:
+            try:
+                member_of = is_member(person, teams)
+                for team in member_of:
+                    if not team in memberships:
+                        memberships[team] = {}
+
+                    memberships[team][person] = [email]
+            except HTTPError, ex:
+                # We should not get an error 503 from Launchpad.
+                # https://bugs.launchpad.net/linaro-patchmetrics/+bug/1012538
+                if ex.code == 503 and retries_count < MAX_RETRIES:
+                    retries_count += 1
+                    time.sleep(REQUEST_WAIT)
+                    continue
+                else:
+                    raise(ex)
+            break
 
     sync_memberships(memberships)
-
     sync_openid_urls()