launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/code-import-remote-broken into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jelmer/launchpad/code-import-remote-broken/+merge/72865

Add a new code import event result: REMOTE_BROKEN.

This is used when the remote branch is present but in some way corrupted. It can either have invalid data or a server that is misbehaving.

This covers most of the remaining failing Subversion imports. Some SVN servers will simply stop streaming log files if they hit a revision with invalid data. 
-- 
https://code.launchpad.net/~jelmer/launchpad/code-import-remote-broken/+merge/72865
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/code-import-remote-broken into lp:launchpad.

=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2011-06-28 21:13:33 +0000
+++ lib/lp/code/enums.py	2011-08-25 12:03:14 +0000
@@ -811,6 +811,12 @@
         Bazaar or the Bazaar foreign branch support.
         """)
 
+    FAILURE_REMOTE_BROKEN = DBItem(250, """
+        Broken remote branch
+
+        The remote branch exists but is corrupted in some way
+        """)
+
     RECLAIMED = DBItem(310, """
         Job reclaimed
 

=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2011-08-22 11:56:26 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2011-08-25 12:03:14 +0000
@@ -451,7 +451,25 @@
             makeFailure(
                 error.ProcessTerminated,
                 exitCode=CodeImportWorkerExitCode.FAILURE_UNSUPPORTED_FEATURE))
-        self.assertEqual(calls, [CodeImportResultStatus.FAILURE_UNSUPPORTED_FEATURE])
+        self.assertEqual(
+            calls, [CodeImportResultStatus.FAILURE_UNSUPPORTED_FEATURE])
+        self.assertOopsesLogged([])
+        # We return the deferred that callFinishJob returns -- if
+        # callFinishJob did not swallow the error, this will fail the test.
+        return ret
+
+    def test_callFinishJobCallsFinishJobRemoteBroken(self):
+        # If the argument to callFinishJob indicates that the subprocess
+        # exited with a code of FAILURE_REMOTE_BROKEN, it
+        # calls finishJob with a status of FAILURE_REMOTE_BROKEN.
+        worker_monitor = self.makeWorkerMonitorWithJob()
+        calls = self.patchOutFinishJob(worker_monitor)
+        ret = worker_monitor.callFinishJob(
+            makeFailure(
+                error.ProcessTerminated,
+                exitCode=CodeImportWorkerExitCode.FAILURE_REMOTE_BROKEN))
+        self.assertEqual(
+            calls, [CodeImportResultStatus.FAILURE_REMOTE_BROKEN])
         self.assertOopsesLogged([])
         # We return the deferred that callFinishJob returns -- if
         # callFinishJob did not swallow the error, this will fail the test.

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2011-08-02 11:28:46 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2011-08-25 12:03:14 +0000
@@ -71,6 +71,7 @@
     SUCCESS_PARTIAL = 3
     FAILURE_INVALID = 4
     FAILURE_UNSUPPORTED_FEATURE = 5
+    FAILURE_REMOTE_BROKEN = 7
 
 
 class BazaarBranchStore:
@@ -589,6 +590,11 @@
         raise NotImplementedError
 
     @property
+    def broken_remote_exceptions(self):
+        """The exceptions to consider for broken remote branches."""
+        raise NotImplementedError
+
+    @property
     def probers(self):
         """The probers that should be tried for this import."""
         raise NotImplementedError
@@ -640,8 +646,11 @@
                     return (
                         CodeImportWorkerExitCode.FAILURE_UNSUPPORTED_FEATURE)
                 elif e.__class__ in self.invalid_branch_exceptions:
-                    self._logger.info("Branch invalid: %s", e(str))
+                    self._logger.info("Branch invalid: %s", str(e))
                     return CodeImportWorkerExitCode.FAILURE_INVALID
+                elif e.__class__ in self.broken_remote_exceptions:
+                    self._logger.info("Remote branch broken: %s", str(e))
+                    return CodeImportWorkerExitCode.FAILURE_REMOTE_BROKEN
                 else:
                     raise
             self._logger.info("Pushing local import branch to central store.")
@@ -675,6 +684,10 @@
         ]
 
     @property
+    def broken_remote_exceptions(self):
+        return []
+
+    @property
     def probers(self):
         """See `PullingImportWorker.probers`."""
         from bzrlib.plugins.git import (
@@ -744,6 +757,10 @@
         ]
 
     @property
+    def broken_remote_exceptions(self):
+        return []
+
+    @property
     def probers(self):
         """See `PullingImportWorker.probers`."""
         from bzrlib.plugins.hg import HgProber
@@ -811,6 +828,11 @@
             InvalidFileName,
         ]
 
+    @property
+    def broken_remote_exceptions(self):
+        from bzrlib.plugins.svn.errors import IncompleteRepositoryHistory
+        return [IncompleteRepositoryHistory]
+
     def getRevisionLimit(self):
         """See `PullingImportWorker.getRevisionLimit`."""
         return config.codeimport.svn_revisions_import_limit

=== modified file 'lib/lp/codehosting/codeimport/workermonitor.py'
--- lib/lp/codehosting/codeimport/workermonitor.py	2011-06-16 23:43:04 +0000
+++ lib/lp/codehosting/codeimport/workermonitor.py	2011-08-25 12:03:14 +0000
@@ -257,6 +257,8 @@
                 CodeImportResultStatus.FAILURE_UNSUPPORTED_FEATURE,
             CodeImportWorkerExitCode.FAILURE_INVALID:
                 CodeImportResultStatus.FAILURE_INVALID,
+            CodeImportWorkerExitCode.FAILURE_REMOTE_BROKEN:
+                CodeImportResultStatus.FAILURE_REMOTE_BROKEN,
                 }
         if isinstance(reason, failure.Failure):
             if reason.check(error.ProcessTerminated):