Merge lp:~mwhudson/launchpad/no-hosted-area-fix-reclaim-branch-space into lp:launchpad

[Michael Hudson-Doyle]

Hi Tim,

This near-trivial branch fixes the reclaim branch space and modifiedbranch scripts to work without the hosted/mirror script. They're both now simpler :-)

Cheers,
mwh

[Tim Penhey (thumper)]

> lib/lp/codehosting/scripts/tests/test_modifiedbranches.py
> ...
> # A hosted branch prints mirrored locations.

This doesn't really make any sense by itself any more.

Perhaps just something like:

# A branch location is the physical disk directory.

review: Approve

=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py	2010-03-17 18:44:32 +0000
+++ lib/lp/code/model/branchjob.py	2010-04-27 02:22:38 +0000
@@ -944,13 +944,8 @@
         return self.metadata['branch_id']
 
     def run(self):
-        mirrored_path = os.path.join(
+        branch_path = os.path.join(
             config.codehosting.mirrored_branches_root,
             branch_id_to_path(self.branch_id))
-        hosted_path = os.path.join(
-            config.codehosting.hosted_branches_root,
-            branch_id_to_path(self.branch_id))
-        if os.path.exists(mirrored_path):
-            shutil.rmtree(mirrored_path)
-        if os.path.exists(hosted_path):
-            shutil.rmtree(hosted_path)
+        if os.path.exists(branch_path):
+            shutil.rmtree(branch_path)
 
=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
--- lib/lp/code/model/tests/test_branchjob.py	2010-03-15 21:18:52 +0000
+++ lib/lp/code/model/tests/test_branchjob.py	2010-04-27 02:22:38 +0000
@@ -1264,13 +1264,8 @@
 
     layer = LaunchpadZopelessLayer
 
-    def cleanHostedAndMirroredAreas(self):
-        """Ensure that hosted and mirrored branch areas are present and empty.
-        """
-        hosted = config.codehosting.hosted_branches_root
-        shutil.rmtree(hosted, ignore_errors=True)
-        os.makedirs(hosted)
-        self.addCleanup(shutil.rmtree, hosted)
+    def cleanBranchArea(self):
+        """Ensure that the branc areas is present and empty."""
         mirrored = config.codehosting.mirrored_branches_root
         shutil.rmtree(mirrored, ignore_errors=True)
         os.makedirs(mirrored)
@@ -1278,7 +1273,7 @@
 
     def setUp(self):
         TestCaseWithFactory.setUp(self)
-        self.cleanHostedAndMirroredAreas()
+        self.cleanBranchArea()
 
     def test_providesInterface(self):
         # ReclaimBranchSpaceJob implements IReclaimBranchSpaceJob.
@@ -1324,7 +1319,7 @@
             job_count += 1
         self.assertTrue(job_count > 0, "No jobs ran!")
 
-    def test_run_branch_in_neither_area(self):
+    def test_run_no_branch_on_disk(self):
         # Running a job to reclaim space for a branch that was never pushed to
         # does nothing quietly.
         branch_id = self.factory.getUniqueInteger()
@@ -1333,51 +1328,19 @@
         # Just "assertNotRaises"
         self.runReadyJobs()
 
-    def test_run_branch_in_hosted_area(self):
+    def test_run_with_branch_on_disk(self):
         # Running a job to reclaim space for a branch that was pushed to
         # but never mirrored removes the branch from the hosted area.
         branch_id = self.factory.getUniqueInteger()
         job = getUtility(IReclaimBranchSpaceJobSource).create(branch_id)
         self.makeJobReady(job)
-        hosted_branch_path = os.path.join(
-            config.codehosting.hosted_branches_root,
-            branch_id_to_path(branch_id), '.bzr')
-        os.makedirs(hosted_branch_path)
-        self.runReadyJobs()
-        self.assertFalse(os.path.exists(hosted_branch_path))
-
-    def test_run_branch_in_mirrored_area(self):
-        # Running a job to reclaim space for a branch that only exists in the
-        # mirrored area (e.g. a MIRRORED branch) removes the branch from the
-        # mirrored area.
-        branch_id = self.factory.getUniqueInteger()
-        job = getUtility(IReclaimBranchSpaceJobSource).create(branch_id)
-        self.makeJobReady(job)
-        mirrored_branch_path = os.path.join(
-            config.codehosting.mirrored_branches_root,
-            branch_id_to_path(branch_id), '.bzr')
-        os.makedirs(mirrored_branch_path)
-        self.runReadyJobs()
-        self.assertFalse(os.path.exists(mirrored_branch_path))
-
-    def test_run_branch_in_both_areas(self):
-        # Running a job to reclaim space for a branch is present in both the
-        # mirrored and hosted area removes the branch from both areas.
-        branch_id = self.factory.getUniqueInteger()
-        job = getUtility(IReclaimBranchSpaceJobSource).create(branch_id)
-        self.makeJobReady(job)
-        hosted_branch_path = os.path.join(
-            config.codehosting.hosted_branches_root,
-            branch_id_to_path(branch_id), '.bzr')
-        mirrored_branch_path = os.path.join(
-            config.codehosting.mirrored_branches_root,
-            branch_id_to_path(branch_id), '.bzr')
-        os.makedirs(hosted_branch_path)
-        os.makedirs(mirrored_branch_path)
-        self.runReadyJobs()
-        self.assertFalse(
-            os.path.exists(hosted_branch_path)
-            or os.path.exists(mirrored_branch_path))
+        branch_path = os.path.join(
+            config.codehosting.mirrored_branches_root,
+            branch_id_to_path(branch_id), '.bzr')
+        os.makedirs(branch_path)
+        self.runReadyJobs()
+        self.assertFalse(os.path.exists(branch_path))
+
 
 
 def test_suite():
 
=== modified file 'lib/lp/codehosting/scripts/modifiedbranches.py'
--- lib/lp/codehosting/scripts/modifiedbranches.py	2009-08-20 00:21:24 +0000
+++ lib/lp/codehosting/scripts/modifiedbranches.py	2010-04-27 02:22:38 +0000
@@ -29,11 +29,6 @@
     returned.  It is possible that the branch will have been modified only in
     the web UI and not actually received any more revisions, and will be a
     false positive.
-
-    If the branch is REMOTE it is ignored.
-    If the branch is HOSTED, both the hosted and mirrored area are returned.
-    If the branch is an IMPORT or MIRROR branch, only the mirrored area is
-    shown.
     """
 
     description = (
@@ -89,12 +84,10 @@
         # Make the datetime timezone aware.
         return last_modified.replace(tzinfo=pytz.UTC)
 
-    def branch_locations(self, branch):
-        """Return a list of branch paths for the given branch."""
+    def branch_location(self, branch):
+        """Return the  branch path for the given branch."""
         path = branch_id_to_path(branch.id)
-        yield os.path.join(config.codehosting.mirrored_branches_root, path)
-        if branch.branch_type == BranchType.HOSTED:
-            yield os.path.join(config.codehosting.hosted_branches_root, path)
+        return os.path.join(config.codehosting.mirrored_branches_root, path)
 
     def process_location(self, location):
         """Strip the defined prefix, and append the suffix as configured."""
@@ -122,8 +115,8 @@
         collection = collection.scannedSince(last_modified)
         for branch in collection.getBranches():
             self.logger.info(branch.unique_name)
-            for location in self.branch_locations(branch):
-                self.update_locations(self.process_location(location))
+            location = self.branch_location(branch)
+            self.update_locations(self.process_location(location))
 
         for location in sorted(self.locations):
             print location
 
=== modified file 'lib/lp/codehosting/scripts/tests/test_modifiedbranches.py'
--- lib/lp/codehosting/scripts/tests/test_modifiedbranches.py	2009-08-20 00:21:24 +0000
+++ lib/lp/codehosting/scripts/tests/test_modifiedbranches.py	2010-04-27 02:22:38 +0000
@@ -26,42 +26,16 @@
 
     layer = DatabaseFunctionalLayer
 
-    def assertHostedLocation(self, branch, location):
-        """Assert that the location is the hosted location for the branch."""
-        path = branch_id_to_path(branch.id)
-        self.assertEqual(
-            os.path.join(config.codehosting.hosted_branches_root, path),
-            location)
-
-    def assertMirroredLocation(self, branch, location):
-        """Assert that the location is the mirror location for the branch."""
+    def test_branch(self):
+        # A branch location is the physical disk directory.
+        branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
+        script = ModifiedBranchesScript('modified-branches', test_args=[])
+        location = script.branch_location(branch)
         path = branch_id_to_path(branch.id)
         self.assertEqual(
             os.path.join(config.codehosting.mirrored_branches_root, path),
             location)
 
-    def test_hosted_branch(self):
-        # A hosted branch prints both the hosted and mirrored locations.
-        branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
-        script = ModifiedBranchesScript('modified-branches', test_args=[])
-        [mirrored, hosted] = script.branch_locations(branch)
-        self.assertHostedLocation(branch, hosted)
-        self.assertMirroredLocation(branch, mirrored)
-
-    def test_mirrored_branch(self):
-        # A mirrored branch prints only the mirrored location.
-        branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
-        script = ModifiedBranchesScript('modified-branches', test_args=[])
-        [mirrored] = script.branch_locations(branch)
-        self.assertMirroredLocation(branch, mirrored)
-
-    def test_imported_branch(self):
-        # A mirrored branch prints only the mirrored location.
-        branch = self.factory.makeAnyBranch(branch_type=BranchType.IMPORTED)
-        script = ModifiedBranchesScript('modified-branches', test_args=[])
-        [mirrored] = script.branch_locations(branch)
-        self.assertMirroredLocation(branch, mirrored)
-
 
 class TestModifiedBranchesLastModifiedEpoch(TestCase):
     """Test the calculation of the last modifed date."""