Merge lp:~mwhudson/launchpad/dont-acquire-remote-branch-bug-408656 into lp:launchpad

[Michael Hudson-Doyle]

[Michael Hudson-Doyle (mwhudson)]

Hi,

From the bug report (bug 408656):

In general, remote branches never have next_mirror_time set. However, we have a bunch in the database that do, from when we mass-converted Jelmer's gnome-svn mirror branches from MIRRORED to REMOTE. It's probably better for acquireBranchToPull to not return these than fix the database and never have this happen again.

The fix is very simple.

[Tim Penhey (thumper)]

Looks good.

review: Approve

=== modified file 'lib/lp/code/model/branchpuller.py'
--- lib/lp/code/model/branchpuller.py	2009-06-30 16:56:07 +0000
+++ lib/lp/code/model/branchpuller.py	2009-08-04 05:14:32 +0000
@@ -55,7 +55,8 @@
         store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
         branch = store.find(
             Branch,
-            Branch.next_mirror_time <= UTC_NOW).order_by(
+            Branch.next_mirror_time <= UTC_NOW,
+            Branch.branch_type != BranchType.REMOTE).order_by(
                 Branch.next_mirror_time).first()
         if branch is not None:
             branch.startMirroring()

=== modified file 'lib/lp/code/model/tests/test_branchpuller.py'
--- lib/lp/code/model/tests/test_branchpuller.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/model/tests/test_branchpuller.py	2009-08-04 05:14:32 +0000
@@ -73,7 +73,6 @@
         self.assertEqual(
             [branch],
             list(self.branch_puller.getPullQueue(branch.branch_type)))
-        next_mirror_time = branch.next_mirror_time
         branch.mirrorComplete('rev1')
         self.assertEqual(
             [branch],
@@ -256,6 +255,16 @@
         branch.requestMirror()
         self.assertBranchIsAquired(branch)
 
+    def test_remote_branch_not_acquired(self):
+        # On a few occasions a branch type that is mirrored has been
+        # converted, with non-NULL next_mirror_time, to a remote branch, which
+        # is not mirrored.  These branches should not be returned.
+        branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
+        branch.requestMirror()
+        removeSecurityProxy(branch).branch_type = BranchType.REMOTE
+        self.assertNoBranchIsAquired()
+
+
     def test_private(self):
         # If there is a private branch that needs mirroring,
         # acquireBranchToPull returns that.

=== modified file 'lib/lp/codehosting/inmemory.py'
--- lib/lp/codehosting/inmemory.py	2009-07-17 00:26:05 +0000
+++ lib/lp/codehosting/inmemory.py	2009-08-04 05:14:32 +0000
@@ -468,7 +468,8 @@
     def acquireBranchToPull(self):
         branches = sorted(
             [branch for branch in self._branch_set
-            if branch.next_mirror_time is not None],
+             if branch.next_mirror_time is not None
+             and branch.branch_type != BranchType.REMOTE],
             key=operator.attrgetter('next_mirror_time'))
         if branches:
             branch = branches[-1]