lp-scanner-team team mailing list archive

["Timothy R. Chavez" <timothy.chavez@xxxxxxxxxxxxx>]

Timothy R. Chavez has proposed merging lp:~timrchavez/lp-scanner/lp-scanner-fix-1187481 into lp:lp-scanner.

Requested reviews:
  The Launchpad Security Scanner Dev Team (lp-scanner-team)
Related bugs:
  Bug #1187481 in lp-scanner: "503 Service Unavailable breaking security scanner"
  https://bugs.launchpad.net/lp-scanner/+bug/1187481

For more details, see:
https://code.launchpad.net/~timrchavez/lp-scanner/lp-scanner-fix-1187481/+merge/168839

Handle a 503 ServerError exception more gracefully when calling getBranches() (LP #1187481)
-- 
https://code.launchpad.net/~timrchavez/lp-scanner/lp-scanner-fix-1187481/+merge/168839
Your team The Launchpad Security Scanner Dev Team is requested to review the proposed merge of lp:~timrchavez/lp-scanner/lp-scanner-fix-1187481 into lp:lp-scanner.

=== modified file 'security-scanner.py'
--- security-scanner.py	2013-05-24 17:45:49 +0000
+++ security-scanner.py	2013-06-12 03:28:25 +0000
@@ -48,6 +48,9 @@
 from launchpadlib.errors import HTTPError
 
 
+# The number of times we'll retry getBranches() on a given project
+MAX_GET_BRANCHES_RETRY = 3
+
 # this implements ConfigParser in a way that isn't compatible with
 # ConfigParser. Replacing this would mean changing our config files
 # so, leaving for the moment.
@@ -267,11 +270,21 @@
                 # Project owner that's not allowed to own the project
                 wrong_owners[project] = None
 
-            for branch in project.getBranches():
-                if (not branch.private and
-                        branch.name not in branch_exceptions):
-                    # Public branch that we haven't made an exception for
-                    public_branches[branch] = project
+            for retry in range(1, MAX_GET_BRANCHES_RETRY+1):
+                try:
+                    for branch in project.getBranches():
+                        if (not branch.private and
+                            branch.name not in branch_exceptions):
+                            # Public branch we haven't made an exception for
+                            public_branches[branch] = project
+                except lazr.restfulclient.errors.ServerError:
+                    printf("W: getBranches() on project '%s' timed out.  "
+                           "Retrying..." % project.name)
+                else:
+                    break
+            if retry == MAX_GET_BRANCHES_RETRY:
+                printf("E: getBranches() on project '%s' failed after %d "
+                       "attempts!" % (project.name, retry))
 
             if project.name not in bug_exceptions:
                 bug_tasks = project.searchTasks(order_by='id')
@@ -498,7 +511,13 @@
             self.mocker.result(owner)
             self.mocker.count(0, None)
             mock_project.getBranches()
-            self.mocker.result(branches)
+            if branches is None:
+                self.mocker.throw(
+                    lazr.restfulclient.errors.ServerError(
+                        "Service Unavailable", 503))
+                self.mocker.count(0, None)
+            else:
+                self.mocker.result(branches)
             mock_project.name
             self.mocker.result(name)
             self.mocker.count(0, None)
@@ -757,6 +776,32 @@
                 "with the project team"
                 in security_report)
 
+        def test_project_get_branches_fails(self):
+            """
+            Test that getBranches() is handled when it fails with an exception.
+            """
+            mock_project = self._setup_project(
+                name="ok-project",
+                driver=self._setup_driver(name="valid-driver"),
+                owner=self._setup_owner(
+                    name="joey", ppas=[self._setup_ppa(private=True), ]),
+                branches=None,
+                tasks=[self._setup_task(bug=self._setup_bug(private=True)), ]
+            )
+            self._setup_project_group("test-project", [mock_project, ])
+            self._setup_project_grantee(
+                mock_project, name="ok-project-team",
+                permissions={"PRIVATESECURITY": "ALL"})
+            self.mocker.replay()
+            outfile = cStringIO.StringIO()
+            security_scanner(self.lp, self.sharing, self.config, outfile)
+            outfile.seek(0)
+            security_report = outfile.read()
+            self.assertTrue(
+                "E: getBranches() on project '%s' failed after %d "
+                "attempts!" % (mock_project.name, MAX_GET_BRANCHES_RETRY)
+                in security_report)
+
 
 except ImportError:
     if __name__ == "security-scanner":