Merge lp:~mwhudson/launchpad/remove-old-puller-xmlrpc-methods into lp:launchpad

[Michael Hudson-Doyle]

Remove some code to do with the 'old' way of scheduling branch pulling.

[Michael Hudson-Doyle (mwhudson)]

Just deleting some code we don't use any more.

[Tim Penhey (thumper)]

  merge approved

Yay for obsolete code removal.

review: Approve

=== modified file 'lib/lp/code/doc/xmlrpc-branch-puller.txt'
--- lib/lp/code/doc/xmlrpc-branch-puller.txt	2009-10-22 11:55:51 +0000
+++ lib/lp/code/doc/xmlrpc-branch-puller.txt	2010-02-24 02:03:16 +0000
@@ -28,19 +28,4 @@
     True
 
 The IBranchPuller interface defines some methods, for which see the unit
-tests.  To allow a minimal test here, we call getBranchPullQueue,
-which will return an empty list.
-
-    >>> from lp.code.enums import BranchType
-    >>> branch_puller.getBranchPullQueue(BranchType.HOSTED.name)
-    []
-
-This remains true when it is accessed over XMLRPC.
-
-    >>> import xmlrpclib
-    >>> from canonical.functional import XMLRPCTestTransport
-    >>> puller = xmlrpclib.ServerProxy(
-    ...     'http://xmlrpc-private.launchpad.dev:8087/branch_puller',
-    ...     transport=XMLRPCTestTransport())
-    >>> puller.getBranchPullQueue(BranchType.HOSTED.name)
-    []
+tests.
 
=== modified file 'lib/lp/code/interfaces/branchpuller.py'
--- lib/lp/code/interfaces/branchpuller.py	2009-06-30 16:56:07 +0000
+++ lib/lp/code/interfaces/branchpuller.py	2010-02-24 02:03:16 +0000
@@ -23,12 +23,6 @@
     MIRROR_TIME_INCREMENT = Attribute(
         "How frequently we mirror branches.")
 
-    def getPullQueue(branch_type):
-        """Return a queue of branches to mirror using the puller.
-
-        :param branch_type: A value from the `BranchType` enum.
-        """
-
     def acquireBranchToPull():
         """Return a Branch to pull and mark it as mirror-started.
 
 
=== modified file 'lib/lp/code/interfaces/codehosting.py'
--- lib/lp/code/interfaces/codehosting.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/codehosting.py	2010-02-24 02:03:16 +0000
@@ -58,20 +58,6 @@
     Published at 'branch_puller' on the private XML-RPC server.
     """
 
-    def getBranchPullQueue(branch_type):
-        """Get the list of branches to be mirrored.
-
-        :param branch_type: One of 'HOSTED', 'MIRRORED', or 'IMPORTED'.
-
-        :raise UnknownBranchTypeError: if the branch type is unrecognized.
-
-        :returns: a list of (branch_id, pull_url, unique_name, default_branch)
-            4-tuples.  branch_id is the database id of the branch, pull_url is
-            where to pull from, unique_name is the unique_name of the branch
-            and default_branch is the default stacked on branch for the
-            branch's target.
-        """
-
     def acquireBranchToPull():
         """Return a Branch to pull and mark it as mirror-started.
 
 
=== modified file 'lib/lp/code/model/branchpuller.py'
--- lib/lp/code/model/branchpuller.py	2009-08-04 05:14:32 +0000
+++ lib/lp/code/model/branchpuller.py	2010-02-24 02:03:16 +0000
@@ -9,17 +9,13 @@
 
 from datetime import timedelta
 
-from storm.expr import LeftJoin, Join
 from zope.component import getUtility
 from zope.interface import implements
 
 from canonical.database.constants import UTC_NOW
 from lp.code.enums import BranchType
 from lp.code.model.branch import Branch
-from lp.code.interfaces.branch import BranchTypeError
 from lp.code.interfaces.branchpuller import IBranchPuller
-from lp.registry.model.person import Owner
-from lp.registry.model.product import Product
 from canonical.launchpad.webapp.interfaces import (
     IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
 
@@ -32,24 +28,6 @@
     MAXIMUM_MIRROR_FAILURES = 5
     MIRROR_TIME_INCREMENT = timedelta(hours=6)
 
-    def getPullQueue(self, branch_type):
-        """See `IBranchPuller`."""
-        if branch_type == BranchType.REMOTE:
-            raise BranchTypeError("No pull queue for REMOTE branches.")
-        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-        # Prejoin on owner and product to preserve existing behaviour.
-        # XXX: JonathanLange 2009-03-22 spec=package-branches: This prejoin is
-        # inappropriate in the face of package branches.
-        prejoin = store.using(
-            Branch,
-            LeftJoin(Product, Branch.product == Product.id),
-            Join(Owner, Branch.owner == Owner.id))
-        return prejoin.find(
-            Branch,
-            Branch.branch_type == branch_type,
-            Branch.next_mirror_time <= UTC_NOW).order_by(
-                Branch.next_mirror_time)
-
     def acquireBranchToPull(self):
         """See `IBranchPuller`."""
         store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
 
=== modified file 'lib/lp/code/model/tests/test_branchpuller.py'
--- lib/lp/code/model/tests/test_branchpuller.py	2009-09-28 23:51:54 +0000
+++ lib/lp/code/model/tests/test_branchpuller.py	2010-02-24 02:03:16 +0000
@@ -17,7 +17,6 @@
 from canonical.database.constants import UTC_NOW
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.code.enums import BranchType
-from lp.code.interfaces.branch import BranchTypeError
 from lp.code.interfaces.branchpuller import IBranchPuller
 from lp.testing import TestCaseWithFactory, login_person
 
@@ -78,37 +77,6 @@
         branch.requestMirror()
         self.assertEqual(UTC_NOW, branch.next_mirror_time)
 
-    def test_requestMirrorDuringPull(self):
-        """Branches can have mirrors requested while they are being mirrored.
-        If so, they should not be removed from the pull queue when the mirror
-        is complete.
-        """
-        # We run these in separate transactions so as to have the times set to
-        # different values. This is closer to what happens in production.
-        branch = self.makeAnyBranch()
-        branch.startMirroring()
-        self.assertEqual(
-            [], list(self.branch_puller.getPullQueue(branch.branch_type)))
-        branch.requestMirror()
-        self.assertEqual(
-            [branch],
-            list(self.branch_puller.getPullQueue(branch.branch_type)))
-        branch.mirrorComplete('rev1')
-        self.assertEqual(
-            [branch],
-            list(self.branch_puller.getPullQueue(branch.branch_type)))
-
-    def test_startMirroringRemovesFromPullQueue(self):
-        # Starting a mirror removes the branch from the pull queue.
-        branch = self.makeAnyBranch()
-        branch.requestMirror()
-        self.assertEqual(
-            set([branch]),
-            set(self.branch_puller.getPullQueue(branch.branch_type)))
-        branch.startMirroring()
-        self.assertEqual(
-            set(), set(self.branch_puller.getPullQueue(branch.branch_type)))
-
     def test_mirroringResetsMirrorRequest(self):
         """Mirroring branches resets their mirror request times."""
         branch = self.makeAnyBranch()
@@ -129,44 +97,6 @@
         self.assertEqual(1, branch.mirror_failures)
         self.assertEqual(None, branch.next_mirror_time)
 
-    def test_pullQueueEmpty(self):
-        """Branches with no next_mirror_time are not in the pull queue."""
-        branch = self.makeAnyBranch()
-        self.assertIs(None, branch.next_mirror_time)
-        self.assertEqual(
-            [], list(self.branch_puller.getPullQueue(self.branch_type)))
-
-    def test_pastNextMirrorTimeInQueue(self):
-        """Branches with next_mirror_time in the past are mirrored."""
-        transaction.begin()
-        branch = self.makeAnyBranch()
-        branch.requestMirror()
-        queue = self.branch_puller.getPullQueue(branch.branch_type)
-        self.assertEqual([branch], list(queue))
-
-    def test_futureNextMirrorTimeInQueue(self):
-        """Branches with next_mirror_time in the future are not mirrored."""
-        transaction.begin()
-        branch = removeSecurityProxy(self.makeAnyBranch())
-        tomorrow = self.getNow() + timedelta(1)
-        branch.next_mirror_time = tomorrow
-        branch.syncUpdate()
-        transaction.commit()
-        self.assertEqual(
-            [], list(self.branch_puller.getPullQueue(branch.branch_type)))
-
-    def test_pullQueueOrder(self):
-        """Pull queue has the oldest mirror request times first."""
-        branches = []
-        for i in range(3):
-            branch = removeSecurityProxy(self.makeAnyBranch())
-            branch.next_mirror_time = self.getNow() - timedelta(hours=i+1)
-            branch.sync()
-            branches.append(branch)
-        self.assertEqual(
-            list(reversed(branches)),
-            list(self.branch_puller.getPullQueue(self.branch_type)))
-
 
 class TestMirroringForMirroredBranches(TestMirroringForHostedBranches):
 
@@ -231,18 +161,6 @@
     branch_type = BranchType.IMPORTED
 
 
-class TestRemoteBranches(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
-
-    def test_raises_branch_type_error(self):
-        # getPullQueue raises `BranchTypeError` if passed BranchType.REMOTE.
-        # It's impossible to mirror remote branches, so we shouldn't even try.
-        puller = getUtility(IBranchPuller)
-        self.assertRaises(
-            BranchTypeError, puller.getPullQueue, BranchType.REMOTE)
-
-
 class AcquireBranchToPullTests:
     """Tests for acquiring branches to pull.
 
 
=== modified file 'lib/lp/code/xmlrpc/codehosting.py'
--- lib/lp/code/xmlrpc/codehosting.py	2009-11-23 22:39:21 +0000
+++ lib/lp/code/xmlrpc/codehosting.py	2010-02-24 02:03:16 +0000
@@ -12,7 +12,6 @@
 
 
 import datetime
-import urllib
 
 import pytz
 
@@ -25,8 +24,7 @@
 
 from canonical.launchpad.ftests import login_person, logout
 from lp.code.enums import BranchType
-from lp.code.interfaces.branch import (
-    BranchCreationException, UnknownBranchTypeError)
+from lp.code.interfaces.branch import BranchCreationException
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.branchnamespace import (
     InvalidNamespace, lookup_branch_namespace, split_unique_name)
@@ -56,46 +54,6 @@
 
     implements(IBranchPuller)
 
-    def _getBranchPullInfo(self, branch):
-        """Return information the branch puller needs to pull this branch.
-
-        This is outside of the IBranch interface so that the authserver can
-        access the information without logging in as a particular user.
-
-        :return: (id, url, unique_name, default_stacked_on_url), where 'id'
-            is the branch database ID, 'url' is the URL to pull from,
-            'unique_name' is the `unique_name` property and
-            'default_stacked_on_url' is the URL of the branch to stack on by
-            default (normally of the form '/~foo/bar/baz'). If there is no
-            default stacked-on branch, then it's ''.
-        """
-        branch = removeSecurityProxy(branch)
-        if branch.branch_type == BranchType.REMOTE:
-            raise AssertionError(
-                'Remote branches should never be in the pull queue.')
-        default_branch = branch.target.default_stacked_on_branch
-        if default_branch is None:
-            default_branch = ''
-        elif (branch.branch_type == BranchType.MIRRORED
-              and default_branch.private):
-            default_branch = ''
-        else:
-            default_branch = '/' + default_branch.unique_name
-        return (
-            branch.id, branch.getPullURL(), branch.unique_name,
-            default_branch)
-
-    def getBranchPullQueue(self, branch_type):
-        """See `IBranchPuller`."""
-        try:
-            branch_type = BranchType.items[branch_type]
-        except KeyError:
-            raise UnknownBranchTypeError(
-                'Unknown branch type: %r' % (branch_type,))
-        branches = getUtility(branchpuller.IBranchPuller).getPullQueue(
-            branch_type)
-        return [self._getBranchPullInfo(branch) for branch in branches]
-
     def acquireBranchToPull(self):
         """See `IBranchPuller`."""
         branch = getUtility(branchpuller.IBranchPuller).acquireBranchToPull()
 
=== modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py'
--- lib/lp/code/xmlrpc/tests/test_codehosting.py	2010-02-18 03:11:03 +0000
+++ lib/lp/code/xmlrpc/tests/test_codehosting.py	2010-02-24 02:03:16 +0000
@@ -399,105 +399,6 @@
         self.assertFaultEqual(faults.NoBranchWithID(branch_id), fault)
 
 
-class BranchPullQueueTest(TestCaseWithFactory):
-    """Tests for the pull queue methods of `IBranchPuller`."""
-
-    def setUp(self):
-        super(BranchPullQueueTest, self).setUp()
-        frontend = self.frontend()
-        self.storage = frontend.getPullerEndpoint()
-        self.factory = frontend.getLaunchpadObjectFactory()
-
-    def assertBranchQueues(self, hosted, mirrored, imported):
-        expected_hosted = [
-            self.storage._getBranchPullInfo(branch) for branch in hosted]
-        expected_mirrored = [
-            self.storage._getBranchPullInfo(branch) for branch in mirrored]
-        expected_imported = [
-            self.storage._getBranchPullInfo(branch) for branch in imported]
-        self.assertEqual(
-            expected_hosted, self.storage.getBranchPullQueue('HOSTED'))
-        self.assertEqual(
-            expected_mirrored, self.storage.getBranchPullQueue('MIRRORED'))
-        self.assertEqual(
-            expected_imported, self.storage.getBranchPullQueue('IMPORTED'))
-
-    def test_pullQueuesEmpty(self):
-        """getBranchPullQueue returns an empty list when there are no branches
-        to pull.
-        """
-        self.assertBranchQueues([], [], [])
-
-    def makeBranchAndRequestMirror(self, branch_type):
-        """Make a branch of the given type and call requestMirror on it."""
-        branch = self.factory.makeAnyBranch(branch_type=branch_type)
-        branch.requestMirror()
-        # The pull queues contain branches that have next_mirror_time strictly
-        # in the past, but requestMirror sets this field to UTC_NOW, so we
-        # push the time back slightly here to get the branch to show up in the
-        # queue.
-        naked_branch = removeSecurityProxy(branch)
-        naked_branch.next_mirror_time -= datetime.timedelta(seconds=1)
-        return branch
-
-    def test_getBranchPullInfo_no_default_stacked_branch(self):
-        # If there's no default stacked branch for the project that a branch
-        # is on, then _getBranchPullInfo returns (id, url, unique_name, '').
-        branch = self.factory.makeAnyBranch()
-        info = self.storage._getBranchPullInfo(branch)
-        self.assertEqual(
-            (branch.id, branch.getPullURL(), branch.unique_name, ''), info)
-
-    def test_getBranchPullInfo_default_stacked_branch(self):
-        # If there's a default stacked branch for the project that a branch is
-        # on, then _getBranchPullInfo returns (id, url, unique_name,
-        # default_branch_unique_name).
-        product = self.factory.makeProduct()
-        default_branch = self.factory.enableDefaultStackingForProduct(product)
-        branch = self.factory.makeProductBranch(product=product)
-        info = self.storage._getBranchPullInfo(branch)
-        self.assertEqual(
-            (branch.id, branch.getPullURL(), branch.unique_name,
-             '/' + default_branch.unique_name), info)
-
-    def test_getBranchPullInfo_private_branch(self):
-        # We don't want to stack mirrored branches onto private branches:
-        # mirrored branches are public by their nature. Thus, if the default
-        # stacked-on branch for the project is private and the branch is
-        # MIRRORED then we don't include the default stacked-on branch's
-        # details in the tuple.
-        product = self.factory.makeProduct()
-        default_branch = self.factory.makeProductBranch(
-            product=product, private=True)
-        self.factory.enableDefaultStackingForProduct(product, default_branch)
-        mirrored_branch = self.factory.makeProductBranch(
-            branch_type=BranchType.MIRRORED, product=product)
-        info = self.storage._getBranchPullInfo(mirrored_branch)
-        self.assertEqual(
-            (mirrored_branch.id, mirrored_branch.getPullURL(),
-             mirrored_branch.unique_name, ''), info)
-
-    def test_getBranchPullInfo_junk(self):
-        # _getBranchPullInfo returns (id, url, unique_name, '') for junk
-        # branches.
-        branch = self.factory.makePersonalBranch()
-        info = self.storage._getBranchPullInfo(branch)
-        self.assertEqual(
-            (branch.id, branch.getPullURL(), branch.unique_name, ''), info)
-
-    def test_requestMirrorPutsBranchInQueue_hosted(self):
-        branch = self.makeBranchAndRequestMirror(BranchType.HOSTED)
-        self.assertBranchQueues([branch], [], [])
-
-    def test_requestMirrorPutsBranchInQueue_mirrored(self):
-        branch = self.makeBranchAndRequestMirror(BranchType.MIRRORED)
-        self.assertBranchQueues([], [branch], [])
-
-    def test_requestMirrorPutsBranchInQueue_imported(self):
-        branch = self.makeBranchAndRequestMirror(BranchType.IMPORTED)
-        self.assertBranchQueues([], [], [branch])
-
-
 class AcquireBranchToPullTestsViaEndpoint(TestCaseWithFactory,
                                           AcquireBranchToPullTests):
     """Tests for `acquireBranchToPull` method of `IBranchPuller`."""
@@ -1175,7 +1076,6 @@
     suite = unittest.TestSuite()
     puller_tests = unittest.TestSuite(
         [loader.loadTestsFromTestCase(BranchPullerTest),
-         loader.loadTestsFromTestCase(BranchPullQueueTest),
          loader.loadTestsFromTestCase(AcquireBranchToPullTestsViaEndpoint),
          loader.loadTestsFromTestCase(BranchFileSystemTest),
          ])
 
=== modified file 'lib/lp/codehosting/inmemory.py'
--- lib/lp/codehosting/inmemory.py	2010-02-19 03:06:12 +0000
+++ lib/lp/codehosting/inmemory.py	2010-02-24 02:03:16 +0000
@@ -442,29 +442,6 @@
         self._branch_set = branch_set
         self._script_activity_set = script_activity_set
 
-    def _getBranchPullInfo(self, branch):
-        default_branch = ''
-        if branch.product is not None:
-            series = branch.product.development_focus
-            user_branch = series.branch
-            if (user_branch is not None
-                and not (
-                    user_branch.private
-                    and branch.branch_type == BranchType.MIRRORED)):
-                default_branch = '/' + user_branch.unique_name
-        return (
-            branch.id, branch.getPullURL(), branch.unique_name,
-            default_branch)
-
-    def getBranchPullQueue(self, branch_type):
-        queue = []
-        branch_type = BranchType.items[branch_type]
-        for branch in self._branch_set:
-            if (branch.branch_type == branch_type
-                and branch.next_mirror_time < UTC_NOW):
-                queue.append(self._getBranchPullInfo(branch))
-        return queue
-
     def acquireBranchToPull(self):
         branches = sorted(
             [branch for branch in self._branch_set