launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:repack-log-on-404 into launchpad:master.

Commit message:
Repack - log 404 instead of raising exception

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/402880

For the scenario where we have repositories that exist in the LP DB but not really hosted in Turnip we want to avoid generating OOPses and just log the error message.

-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:repack-log-on-404 into launchpad:master.

diff --git a/lib/lp/code/model/githosting.py b/lib/lp/code/model/githosting.py
index 787e63e..671c6b7 100644
--- a/lib/lp/code/model/githosting.py
+++ b/lib/lp/code/model/githosting.py
@@ -326,9 +326,16 @@ class GitHostingClient:
                         path))
             return self._post(url)
         except requests.RequestException as e:
-            raise CannotRepackRepository(
-                "Failed to repack Git repository %s: %s" %
-                (path, six.text_type(e)))
+            if (e.response is not None and
+                    e.response.status_code == requests.codes.NOT_FOUND):
+                if logger:
+                    logger.error(
+                        "Git repository %s not found." % ensure_text(path))
+                return "Git repository %s not found." % ensure_text(path)
+            else:
+                raise CannotRepackRepository(
+                    "Failed to repack Git repository %s: %s" %
+                    (path, six.text_type(e)))
 
     def collectGarbage(self, path, logger=None):
         """See `IGitHostingClient`."""
diff --git a/lib/lp/code/model/tests/test_githosting.py b/lib/lp/code/model/tests/test_githosting.py
index 9ffe7c7..18e61dd 100644
--- a/lib/lp/code/model/tests/test_githosting.py
+++ b/lib/lp/code/model/tests/test_githosting.py
@@ -504,6 +504,12 @@ class TestGitHostingClient(TestCase):
                 "400 Client Error: Bad Request",
                 self.client.repackRepository, "/repo/123")
 
+    def test_repack_failure_404(self):
+        with self.mockRequests("POST", status=404):
+            repack = self.client.repackRepository("/repo/123")
+            self.assertEqual(u'Git repository /repo/123 not found.',
+                             repack)
+
     def test_git_gc(self):
         with self.mockRequests("POST", status=200):
             gc = self.client.collectGarbage("/repo/123")