Launchpad itself

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

  1. no-hosted-area-fix-reclaim-branch-space
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 10828
Proposed branch: lp:~mwhudson/launchpad/no-hosted-area-fix-reclaim-branch-space
Merge into: lp:launchpad
Prerequisite: lp:~mwhudson/launchpad/no-hosted-area-fix-branch-distro
Diff against target: 213 lines (+25/-100)
4 files modified
lib/lp/code/model/branchjob.py (+3/-8)
lib/lp/code/model/tests/test_branchjob.py (+12/-49)
lib/lp/codehosting/scripts/modifiedbranches.py (+5/-12)
lib/lp/codehosting/scripts/tests/test_modifiedbranches.py (+5/-31)
To merge this branch: bzr merge lp:~mwhudson/launchpad/no-hosted-area-fix-reclaim-branch-space
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+23975@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

> 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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/model/branchjob.py'
2--- lib/lp/code/model/branchjob.py 2010-03-17 18:44:32 +0000
3+++ lib/lp/code/model/branchjob.py 2010-04-27 02:22:38 +0000
4@@ -944,13 +944,8 @@
5 return self.metadata['branch_id']
6
7 def run(self):
8- mirrored_path = os.path.join(
9+ branch_path = os.path.join(
10 config.codehosting.mirrored_branches_root,
11 branch_id_to_path(self.branch_id))
12- hosted_path = os.path.join(
13- config.codehosting.hosted_branches_root,
14- branch_id_to_path(self.branch_id))
15- if os.path.exists(mirrored_path):
16- shutil.rmtree(mirrored_path)
17- if os.path.exists(hosted_path):
18- shutil.rmtree(hosted_path)
19+ if os.path.exists(branch_path):
20+ shutil.rmtree(branch_path)
21
22=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
23--- lib/lp/code/model/tests/test_branchjob.py 2010-03-15 21:18:52 +0000
24+++ lib/lp/code/model/tests/test_branchjob.py 2010-04-27 02:22:38 +0000
25@@ -1264,13 +1264,8 @@
26
27 layer = LaunchpadZopelessLayer
28
29- def cleanHostedAndMirroredAreas(self):
30- """Ensure that hosted and mirrored branch areas are present and empty.
31- """
32- hosted = config.codehosting.hosted_branches_root
33- shutil.rmtree(hosted, ignore_errors=True)
34- os.makedirs(hosted)
35- self.addCleanup(shutil.rmtree, hosted)
36+ def cleanBranchArea(self):
37+ """Ensure that the branc areas is present and empty."""
38 mirrored = config.codehosting.mirrored_branches_root
39 shutil.rmtree(mirrored, ignore_errors=True)
40 os.makedirs(mirrored)
41@@ -1278,7 +1273,7 @@
42
43 def setUp(self):
44 TestCaseWithFactory.setUp(self)
45- self.cleanHostedAndMirroredAreas()
46+ self.cleanBranchArea()
47
48 def test_providesInterface(self):
49 # ReclaimBranchSpaceJob implements IReclaimBranchSpaceJob.
50@@ -1324,7 +1319,7 @@
51 job_count += 1
52 self.assertTrue(job_count > 0, "No jobs ran!")
53
54- def test_run_branch_in_neither_area(self):
55+ def test_run_no_branch_on_disk(self):
56 # Running a job to reclaim space for a branch that was never pushed to
57 # does nothing quietly.
58 branch_id = self.factory.getUniqueInteger()
59@@ -1333,51 +1328,19 @@
60 # Just "assertNotRaises"
61 self.runReadyJobs()
62
63- def test_run_branch_in_hosted_area(self):
64+ def test_run_with_branch_on_disk(self):
65 # Running a job to reclaim space for a branch that was pushed to
66 # but never mirrored removes the branch from the hosted area.
67 branch_id = self.factory.getUniqueInteger()
68 job = getUtility(IReclaimBranchSpaceJobSource).create(branch_id)
69 self.makeJobReady(job)
70- hosted_branch_path = os.path.join(
71- config.codehosting.hosted_branches_root,
72- branch_id_to_path(branch_id), '.bzr')
73- os.makedirs(hosted_branch_path)
74- self.runReadyJobs()
75- self.assertFalse(os.path.exists(hosted_branch_path))
76-
77- def test_run_branch_in_mirrored_area(self):
78- # Running a job to reclaim space for a branch that only exists in the
79- # mirrored area (e.g. a MIRRORED branch) removes the branch from the
80- # mirrored area.
81- branch_id = self.factory.getUniqueInteger()
82- job = getUtility(IReclaimBranchSpaceJobSource).create(branch_id)
83- self.makeJobReady(job)
84- mirrored_branch_path = os.path.join(
85- config.codehosting.mirrored_branches_root,
86- branch_id_to_path(branch_id), '.bzr')
87- os.makedirs(mirrored_branch_path)
88- self.runReadyJobs()
89- self.assertFalse(os.path.exists(mirrored_branch_path))
90-
91- def test_run_branch_in_both_areas(self):
92- # Running a job to reclaim space for a branch is present in both the
93- # mirrored and hosted area removes the branch from both areas.
94- branch_id = self.factory.getUniqueInteger()
95- job = getUtility(IReclaimBranchSpaceJobSource).create(branch_id)
96- self.makeJobReady(job)
97- hosted_branch_path = os.path.join(
98- config.codehosting.hosted_branches_root,
99- branch_id_to_path(branch_id), '.bzr')
100- mirrored_branch_path = os.path.join(
101- config.codehosting.mirrored_branches_root,
102- branch_id_to_path(branch_id), '.bzr')
103- os.makedirs(hosted_branch_path)
104- os.makedirs(mirrored_branch_path)
105- self.runReadyJobs()
106- self.assertFalse(
107- os.path.exists(hosted_branch_path)
108- or os.path.exists(mirrored_branch_path))
109+ branch_path = os.path.join(
110+ config.codehosting.mirrored_branches_root,
111+ branch_id_to_path(branch_id), '.bzr')
112+ os.makedirs(branch_path)
113+ self.runReadyJobs()
114+ self.assertFalse(os.path.exists(branch_path))
115+
116
117
118 def test_suite():
119
120=== modified file 'lib/lp/codehosting/scripts/modifiedbranches.py'
121--- lib/lp/codehosting/scripts/modifiedbranches.py 2009-08-20 00:21:24 +0000
122+++ lib/lp/codehosting/scripts/modifiedbranches.py 2010-04-27 02:22:38 +0000
123@@ -29,11 +29,6 @@
124 returned. It is possible that the branch will have been modified only in
125 the web UI and not actually received any more revisions, and will be a
126 false positive.
127-
128- If the branch is REMOTE it is ignored.
129- If the branch is HOSTED, both the hosted and mirrored area are returned.
130- If the branch is an IMPORT or MIRROR branch, only the mirrored area is
131- shown.
132 """
133
134 description = (
135@@ -89,12 +84,10 @@
136 # Make the datetime timezone aware.
137 return last_modified.replace(tzinfo=pytz.UTC)
138
139- def branch_locations(self, branch):
140- """Return a list of branch paths for the given branch."""
141+ def branch_location(self, branch):
142+ """Return the branch path for the given branch."""
143 path = branch_id_to_path(branch.id)
144- yield os.path.join(config.codehosting.mirrored_branches_root, path)
145- if branch.branch_type == BranchType.HOSTED:
146- yield os.path.join(config.codehosting.hosted_branches_root, path)
147+ return os.path.join(config.codehosting.mirrored_branches_root, path)
148
149 def process_location(self, location):
150 """Strip the defined prefix, and append the suffix as configured."""
151@@ -122,8 +115,8 @@
152 collection = collection.scannedSince(last_modified)
153 for branch in collection.getBranches():
154 self.logger.info(branch.unique_name)
155- for location in self.branch_locations(branch):
156- self.update_locations(self.process_location(location))
157+ location = self.branch_location(branch)
158+ self.update_locations(self.process_location(location))
159
160 for location in sorted(self.locations):
161 print location
162
163=== modified file 'lib/lp/codehosting/scripts/tests/test_modifiedbranches.py'
164--- lib/lp/codehosting/scripts/tests/test_modifiedbranches.py 2009-08-20 00:21:24 +0000
165+++ lib/lp/codehosting/scripts/tests/test_modifiedbranches.py 2010-04-27 02:22:38 +0000
166@@ -26,42 +26,16 @@
167
168 layer = DatabaseFunctionalLayer
169
170- def assertHostedLocation(self, branch, location):
171- """Assert that the location is the hosted location for the branch."""
172- path = branch_id_to_path(branch.id)
173- self.assertEqual(
174- os.path.join(config.codehosting.hosted_branches_root, path),
175- location)
176-
177- def assertMirroredLocation(self, branch, location):
178- """Assert that the location is the mirror location for the branch."""
179+ def test_branch(self):
180+ # A branch location is the physical disk directory.
181+ branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
182+ script = ModifiedBranchesScript('modified-branches', test_args=[])
183+ location = script.branch_location(branch)
184 path = branch_id_to_path(branch.id)
185 self.assertEqual(
186 os.path.join(config.codehosting.mirrored_branches_root, path),
187 location)
188
189- def test_hosted_branch(self):
190- # A hosted branch prints both the hosted and mirrored locations.
191- branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
192- script = ModifiedBranchesScript('modified-branches', test_args=[])
193- [mirrored, hosted] = script.branch_locations(branch)
194- self.assertHostedLocation(branch, hosted)
195- self.assertMirroredLocation(branch, mirrored)
196-
197- def test_mirrored_branch(self):
198- # A mirrored branch prints only the mirrored location.
199- branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
200- script = ModifiedBranchesScript('modified-branches', test_args=[])
201- [mirrored] = script.branch_locations(branch)
202- self.assertMirroredLocation(branch, mirrored)
203-
204- def test_imported_branch(self):
205- # A mirrored branch prints only the mirrored location.
206- branch = self.factory.makeAnyBranch(branch_type=BranchType.IMPORTED)
207- script = ModifiedBranchesScript('modified-branches', test_args=[])
208- [mirrored] = script.branch_locations(branch)
209- self.assertMirroredLocation(branch, mirrored)
210-
211
212 class TestModifiedBranchesLastModifiedEpoch(TestCase):
213 """Test the calculation of the last modifed date."""