Merge lp:~mwhudson/launchpad/no-hosted-area-remove-mirrorComplete into lp:launchpad

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 10828
Proposed branch: lp:~mwhudson/launchpad/no-hosted-area-remove-mirrorComplete
Merge into: lp:launchpad
Prerequisite: lp:~mwhudson/launchpad/no-hosted-area-make-puller-call-branchChanged
Diff against target: 510 lines (+51/-209)
13 files modified
lib/lp/code/configure.zcml (+0/-1)
lib/lp/code/interfaces/branch.py (+0/-7)
lib/lp/code/interfaces/codehosting.py (+0/-24)
lib/lp/code/mail/tests/test_codehandler.py (+4/-2)
lib/lp/code/model/branch.py (+2/-19)
lib/lp/code/model/tests/test_branch.py (+6/-15)
lib/lp/code/model/tests/test_branchpuller.py (+4/-2)
lib/lp/code/model/tests/test_branchtarget.py (+10/-5)
lib/lp/code/stories/branches/xx-branch-mirror-failures.txt (+1/-1)
lib/lp/code/xmlrpc/codehosting.py (+2/-21)
lib/lp/code/xmlrpc/tests/test_codehosting.py (+11/-79)
lib/lp/codehosting/inmemory.py (+3/-21)
lib/lp/testing/factory.py (+8/-12)
To merge this branch: bzr merge lp:~mwhudson/launchpad/no-hosted-area-remove-mirrorComplete
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+23892@code.launchpad.net

Description of the change

Hi Tim yet again!

This branch removes the now obsolete mirrorComplete branch method and also removes the long-obsolete 'startMirroring' method from the XML-RPC endpoint (it's still a branch method: acquireBranchToPull calls it).

Cheers,
mwh

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

We must be getting close now...

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/configure.zcml'
2--- lib/lp/code/configure.zcml 2010-04-27 02:05:59 +0000
3+++ lib/lp/code/configure.zcml 2010-04-27 02:06:23 +0000
4@@ -521,7 +521,6 @@
5 getPullURL
6 requestMirror
7 startMirroring
8- mirrorComplete
9 mirrorFailed
10 branch_format
11 repository_format
12
13=== modified file 'lib/lp/code/interfaces/branch.py'
14--- lib/lp/code/interfaces/branch.py 2010-04-27 02:05:59 +0000
15+++ lib/lp/code/interfaces/branch.py 2010-04-27 02:06:23 +0000
16@@ -1105,13 +1105,6 @@
17 branch.
18 """
19
20- def mirrorComplete(last_revision_id):
21- """Signal that a mirror attempt has completed successfully.
22-
23- :param last_revision_id: The revision ID of the tip of the mirrored
24- branch.
25- """
26-
27 def mirrorFailed(reason):
28 """Signal that a mirror attempt failed.
29
30
31=== modified file 'lib/lp/code/interfaces/codehosting.py'
32--- lib/lp/code/interfaces/codehosting.py 2010-04-27 02:05:59 +0000
33+++ lib/lp/code/interfaces/codehosting.py 2010-04-27 02:06:23 +0000
34@@ -82,30 +82,6 @@
35 or (), the empty tuple, if there is no branch to pull.
36 """
37
38- def startMirroring(branchID):
39- """Notify Launchpad that the given branch has started mirroring.
40-
41- The last_mirror_attempt field of the given branch record will be
42- updated appropriately.
43-
44- :param branchID: The database ID of the given branch.
45- :returns: True if the branch status was successfully updated.
46- `NoBranchWithID` fault if there's no branch with the given id.
47- """
48-
49- def mirrorComplete(branchID, lastRevisionID):
50- """Notify Launchpad that the branch has been successfully mirrored.
51-
52- In the Launchpad database, the last_mirrored field will be updated to
53- match the last_mirror_attempt value, the mirror_failures counter will
54- be reset to zero and the next_mirror_time will be set to NULL.
55-
56- :param branchID: The database ID of the given branch.
57- :param lastRevisionID: The last revision ID mirrored.
58- :returns: True if the branch status was successfully updated.
59- `NoBranchWithID` fault if there's no branch with the given id.
60- """
61-
62 def mirrorFailed(branchID, reason):
63 """Notify Launchpad that the branch could not be mirrored.
64
65
66=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
67--- lib/lp/code/mail/tests/test_codehandler.py 2010-04-27 02:05:59 +0000
68+++ lib/lp/code/mail/tests/test_codehandler.py 2010-04-27 02:06:23 +0000
69@@ -872,7 +872,8 @@
70 target_tree.commit('rev1')
71 # Make sure that the created branch has been mirrored.
72 db_target_branch.startMirroring()
73- db_target_branch.mirrorComplete('rev1')
74+ removeSecurityProxy(db_target_branch).branchChanged(
75+ '', 'rev1', None, None, None)
76 source_tree = target_tree.bzrdir.sprout('source').open_workingtree()
77 source_tree.commit('rev2')
78 message = self.factory.makeBundleMergeDirectiveEmail(
79@@ -1000,7 +1001,8 @@
80 target_tree.commit('rev1')
81 # Make sure that the created branch has been mirrored.
82 db_target_branch.startMirroring()
83- db_target_branch.mirrorComplete('rev1')
84+ removeSecurityProxy(db_target_branch).branchChanged(
85+ '', 'rev1', None, None, None)
86
87 db_source_branch, source_tree = self.create_branch_and_tree(
88 'lpsource', db_target_branch.product, hosted=True,
89
90=== modified file 'lib/lp/code/model/branch.py'
91--- lib/lp/code/model/branch.py 2010-04-27 02:05:59 +0000
92+++ lib/lp/code/model/branch.py 2010-04-27 02:06:23 +0000
93@@ -832,6 +832,8 @@
94 new_data_pushed = (
95 self.branch_type in (BranchType.HOSTED, BranchType.IMPORTED)
96 and self.next_mirror_time is not None)
97+ # XXX 2010-04-22, MichaelHudson: This should really look for a branch
98+ # scan job.
99 pulled_but_not_scanned = self.last_mirrored_id != self.last_scanned_id
100 pull_in_progress = (
101 self.last_mirror_attempt is not None
102@@ -931,25 +933,6 @@
103 self.branch_format = branch_format
104 self.repository_format = repository_format
105
106- def mirrorComplete(self, last_revision_id):
107- """See `IBranch`."""
108- if self.branch_type == BranchType.REMOTE:
109- raise BranchTypeError(self.unique_name)
110- assert self.last_mirror_attempt != None, (
111- "startMirroring must be called before mirrorComplete.")
112- self.last_mirrored = self.last_mirror_attempt
113- self.mirror_failures = 0
114- self.mirror_status_message = None
115- if (self.next_mirror_time is None
116- and self.branch_type == BranchType.MIRRORED):
117- # No mirror was requested since we started mirroring.
118- increment = getUtility(IBranchPuller).MIRROR_TIME_INCREMENT
119- self.next_mirror_time = (
120- datetime.now(pytz.timezone('UTC')) + increment)
121- self.last_mirrored_id = last_revision_id
122- from lp.code.model.branchjob import BranchScanJob
123- BranchScanJob.create(self)
124-
125 def mirrorFailed(self, reason):
126 """See `IBranch`."""
127 if self.branch_type == BranchType.REMOTE:
128
129=== modified file 'lib/lp/code/model/tests/test_branch.py'
130--- lib/lp/code/model/tests/test_branch.py 2010-04-27 02:05:59 +0000
131+++ lib/lp/code/model/tests/test_branch.py 2010-04-27 02:06:23 +0000
132@@ -1794,28 +1794,18 @@
133 branch = self.factory.makeAnyBranch()
134 branch.startMirroring()
135 rev_id = self.factory.getUniqueString('rev-id')
136- branch.mirrorComplete(rev_id)
137+ removeSecurityProxy(branch).branchChanged(
138+ '', rev_id, None, None, None)
139 self.assertEqual(True, branch.pending_writes)
140
141- def test_mirrorComplete_creates_scan_job(self):
142- # After a branch has been pulled, it should have created a
143- # BranchScanJob to complete the process.
144- branch = self.factory.makeAnyBranch()
145- branch.startMirroring()
146- rev_id = self.factory.getUniqueString('rev-id')
147- branch.mirrorComplete(rev_id)
148-
149- store = Store.of(branch)
150- scan_jobs = store.find(BranchJob, job_type=BranchJobType.SCAN_BRANCH)
151- self.assertEqual(scan_jobs.count(), 1)
152-
153 def test_pulled_and_scanned(self):
154 # If a branch has been pulled and scanned, then there are no pending
155 # writes.
156 branch = self.factory.makeAnyBranch()
157 branch.startMirroring()
158 rev_id = self.factory.getUniqueString('rev-id')
159- branch.mirrorComplete(rev_id)
160+ removeSecurityProxy(branch).branchChanged(
161+ '', rev_id, None, None, None)
162 # Cheat! The actual API for marking a branch as scanned is
163 # updateScannedDetails. That requires a revision in the database
164 # though.
165@@ -1835,7 +1825,8 @@
166 branch = self.factory.makeAnyBranch()
167 branch.startMirroring()
168 rev_id = self.factory.getUniqueString('rev-id')
169- branch.mirrorComplete(rev_id)
170+ removeSecurityProxy(branch).branchChanged(
171+ '', rev_id, None, None, None)
172 # Cheat! The actual API for marking a branch as scanned is
173 # updateScannedDetails. That requires a revision in the database
174 # though.
175
176=== modified file 'lib/lp/code/model/tests/test_branchpuller.py'
177--- lib/lp/code/model/tests/test_branchpuller.py 2010-04-27 02:05:59 +0000
178+++ lib/lp/code/model/tests/test_branchpuller.py 2010-04-27 02:06:23 +0000
179@@ -84,7 +84,8 @@
180 branch.requestMirror()
181 transaction.commit()
182 branch.startMirroring()
183- branch.mirrorComplete('rev1')
184+ removeSecurityProxy(branch).branchChanged(
185+ '', 'rev1', None, None, None)
186 self.assertEqual(None, branch.next_mirror_time)
187
188 def test_mirrorFailureResetsMirrorRequest(self):
189@@ -152,7 +153,8 @@
190 branch.requestMirror()
191 transaction.commit()
192 branch.startMirroring()
193- branch.mirrorComplete('rev1')
194+ removeSecurityProxy(branch).branchChanged(
195+ '', 'rev1', None, None, None)
196 self.assertInFuture(branch.next_mirror_time, self.increment)
197 self.assertEqual(0, branch.mirror_failures)
198
199
200=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
201--- lib/lp/code/model/tests/test_branchtarget.py 2010-04-13 15:08:13 +0000
202+++ lib/lp/code/model/tests/test_branchtarget.py 2010-04-27 02:06:23 +0000
203@@ -108,7 +108,8 @@
204 default_branch = self.factory.makePackageBranch(
205 sourcepackage=development_package)
206 default_branch.startMirroring()
207- default_branch.mirrorComplete(self.factory.getUniqueString())
208+ removeSecurityProxy(default_branch).branchChanged(
209+ '', self.factory.getUniqueString(), None, None, None)
210 ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches
211 run_with_login(
212 ubuntu_branches.teamowner,
213@@ -347,7 +348,8 @@
214 branch = self.factory.makeProductBranch(product=self.original)
215 self._setDevelopmentFocus(self.original, branch)
216 branch.startMirroring()
217- branch.mirrorComplete('rev1')
218+ removeSecurityProxy(branch).branchChanged(
219+ '', 'rev1', None, None, None)
220 target = IBranchTarget(self.original)
221 self.assertEqual(branch, target.default_stacked_on_branch)
222
223@@ -450,7 +452,8 @@
224 # life.
225 branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
226 branch.startMirroring()
227- branch.mirrorComplete(self.factory.getUniqueString())
228+ removeSecurityProxy(branch).branchChanged(
229+ '', self.factory.getUniqueString(), None, None, None)
230 removeSecurityProxy(branch).branch_type = BranchType.REMOTE
231 self.assertIs(None, check_default_stacked_on(branch))
232
233@@ -466,7 +469,8 @@
234 branch = self.factory.makeAnyBranch(private=True)
235 naked_branch = removeSecurityProxy(branch)
236 naked_branch.startMirroring()
237- naked_branch.mirrorComplete(self.factory.getUniqueString())
238+ naked_branch.branchChanged(
239+ '', self.factory.getUniqueString(), None, None, None)
240 self.assertIs(None, check_default_stacked_on(branch))
241
242 def test_been_mirrored(self):
243@@ -475,7 +479,8 @@
244 # futile.
245 branch = self.factory.makeAnyBranch()
246 branch.startMirroring()
247- branch.mirrorComplete('rev1')
248+ removeSecurityProxy(branch).branchChanged(
249+ '', self.factory.getUniqueString(), None, None, None)
250 self.assertEqual(branch, check_default_stacked_on(branch))
251
252
253
254=== modified file 'lib/lp/code/stories/branches/xx-branch-mirror-failures.txt'
255--- lib/lp/code/stories/branches/xx-branch-mirror-failures.txt 2009-06-12 16:36:02 +0000
256+++ lib/lp/code/stories/branches/xx-branch-mirror-failures.txt 2010-04-27 02:06:23 +0000
257@@ -163,7 +163,7 @@
258 >>> login(ANONYMOUS)
259 >>> mirror_branch = getUtility(IBranchLookup).getByUniqueName(mirror_name)
260 >>> mirror_branch.startMirroring()
261- >>> mirror_branch.mirrorComplete('some-revision-id')
262+ >>> mirror_branch.branchChanged('', 'some-revision-id', None, None, None)
263 >>> logout()
264
265 >>> browser.open(branch_location)
266
267=== modified file 'lib/lp/code/xmlrpc/codehosting.py'
268--- lib/lp/code/xmlrpc/codehosting.py 2010-04-27 02:05:59 +0000
269+++ lib/lp/code/xmlrpc/codehosting.py 2010-04-27 02:06:23 +0000
270@@ -123,22 +123,13 @@
271 else:
272 return ()
273
274- def mirrorComplete(self, branch_id, last_revision_id):
275- """See `ICodehostingAPI`."""
276- branch = getUtility(IBranchLookup).get(branch_id)
277- if branch is None:
278- return faults.NoBranchWithID(branch_id)
279- # See comment in startMirroring.
280- branch = removeSecurityProxy(branch)
281- branch.mirrorComplete(last_revision_id)
282- return True
283-
284 def mirrorFailed(self, branch_id, reason):
285 """See `ICodehostingAPI`."""
286 branch = getUtility(IBranchLookup).get(branch_id)
287 if branch is None:
288 return faults.NoBranchWithID(branch_id)
289- # See comment in startMirroring.
290+ # The puller runs as no user and may pull private branches. We need to
291+ # bypass Zope's security proxy to set the mirroring information.
292 removeSecurityProxy(branch).mirrorFailed(reason)
293 return True
294
295@@ -151,16 +142,6 @@
296 date_completed=date_completed, hostname=hostname)
297 return True
298
299- def startMirroring(self, branch_id):
300- """See `ICodehostingAPI`."""
301- branch = getUtility(IBranchLookup).get(branch_id)
302- if branch is None:
303- return faults.NoBranchWithID(branch_id)
304- # The puller runs as no user and may pull private branches. We need to
305- # bypass Zope's security proxy to set the mirroring information.
306- removeSecurityProxy(branch).startMirroring()
307- return True
308-
309 def createBranch(self, login_id, branch_path):
310 """See `ICodehostingAPI`."""
311 def create_branch(requester):
312
313=== modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py'
314--- lib/lp/code/xmlrpc/tests/test_codehosting.py 2010-04-27 02:05:59 +0000
315+++ lib/lp/code/xmlrpc/tests/test_codehosting.py 2010-04-27 02:06:23 +0000
316@@ -179,34 +179,17 @@
317 self.assertIs(self.branch_lookup.get(branch_id), None)
318 return branch_id
319
320- def test_startMirroring(self):
321- # startMirroring updates last_mirror_attempt to 'now', leaves
322- # last_mirrored alone and returns True when passed the id of an
323- # existing branch.
324- branch = self.factory.makeAnyBranch()
325- self.assertUnmirrored(branch)
326-
327- success = self.codehosting_api.startMirroring(branch.id)
328- self.assertEqual(success, True)
329-
330- self.assertSqlAttributeEqualsDate(
331- branch, 'last_mirror_attempt', UTC_NOW)
332- self.assertIs(None, branch.last_mirrored)
333-
334- def test_startMirroringInvalidBranch(self):
335- # startMirroring returns False when given a branch id which does not
336- # exist.
337- invalid_id = self.getUnusedBranchID()
338- fault = self.codehosting_api.startMirroring(invalid_id)
339- self.assertEqual(faults.NoBranchWithID(invalid_id), fault)
340-
341 def test_mirrorFailed(self):
342- branch = self.factory.makeAnyBranch()
343+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
344 self.assertUnmirrored(branch)
345
346- self.codehosting_api.startMirroring(branch.id)
347+ branch.requestMirror()
348+ self.assertEquals(
349+ branch.id, self.codehosting_api.acquireBranchToPull([])[0])
350+
351 failure_message = self.factory.getUniqueString()
352- success = self.codehosting_api.mirrorFailed(branch.id, failure_message)
353+ success = self.codehosting_api.mirrorFailed(
354+ branch.id, failure_message)
355 self.assertEqual(True, success)
356 self.assertMirrorFailed(branch, failure_message)
357
358@@ -216,60 +199,6 @@
359 fault = self.codehosting_api.mirrorFailed(branch_id, failure_message)
360 self.assertEqual(faults.NoBranchWithID(branch_id), fault)
361
362- def test_mirrorComplete(self):
363- # mirrorComplete marks the branch as having been successfully
364- # mirrored, with no failures and no status message.
365- branch = self.factory.makeAnyBranch()
366- self.assertUnmirrored(branch)
367-
368- self.codehosting_api.startMirroring(branch.id)
369- revision_id = self.factory.getUniqueString()
370- success = self.codehosting_api.mirrorComplete(branch.id, revision_id)
371- self.assertEqual(True, success)
372- self.assertMirrorSucceeded(branch, revision_id)
373-
374- def test_mirrorCompleteWithNoBranchID(self):
375- # mirrorComplete returns a Fault if there's no branch with the given
376- # ID.
377- branch_id = self.getUnusedBranchID()
378- fault = self.codehosting_api.mirrorComplete(
379- branch_id, self.factory.getUniqueString())
380- self.assertEqual(faults.NoBranchWithID(branch_id), fault)
381-
382- def test_mirrorComplete_resets_failure_count(self):
383- # mirrorComplete marks the branch as successfully mirrored and removes
384- # all memory of failure.
385-
386- # First, mark the branch as failed.
387- branch = self.factory.makeAnyBranch()
388- self.codehosting_api.startMirroring(branch.id)
389- failure_message = self.factory.getUniqueString()
390- self.codehosting_api.mirrorFailed(branch.id, failure_message)
391- self.assertMirrorFailed(branch, failure_message)
392-
393- # Start and successfully finish a mirror.
394- self.codehosting_api.startMirroring(branch.id)
395- revision_id = self.factory.getUniqueString()
396- self.codehosting_api.mirrorComplete(branch.id, revision_id)
397-
398- # Confirm that it succeeded.
399- self.assertMirrorSucceeded(branch, revision_id)
400-
401- def test_mirrorComplete_resets_mirror_request(self):
402- # After successfully mirroring a hosted branch, next_mirror_time
403- # should be set to NULL.
404- branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
405-
406- # Request that branch be mirrored. This sets next_mirror_time.
407- branch.requestMirror()
408-
409- # Simulate successfully mirroring the branch.
410- self.codehosting_api.startMirroring(branch.id)
411- self.codehosting_api.mirrorComplete(
412- branch.id, self.factory.getUniqueString())
413-
414- self.assertIs(None, branch.next_mirror_time)
415-
416 def test_recordSuccess(self):
417 # recordSuccess must insert the given data into ScriptActivity.
418 started = datetime.datetime(2007, 07, 05, 19, 32, 1, tzinfo=UTC)
419@@ -939,7 +868,10 @@
420
421 def startMirroring(self, branch):
422 """See `AcquireBranchToPullTests`."""
423- self.codehosting_api.startMirroring(branch.id)
424+ # This is a bit random, but it works. acquireBranchToPull marks the
425+ # branch it returns as started mirroring, but we should check that the
426+ # one we want is returned...
427+ self.assertBranchIsAquired(branch, branch.branch_type)
428
429 def test_branch_type_returned_mirrored(self):
430 branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
431
432=== modified file 'lib/lp/codehosting/inmemory.py'
433--- lib/lp/codehosting/inmemory.py 2010-04-27 02:05:59 +0000
434+++ lib/lp/codehosting/inmemory.py 2010-04-27 02:06:23 +0000
435@@ -480,7 +480,9 @@
436 key=operator.attrgetter('next_mirror_time'))
437 if branches:
438 branch = branches[-1]
439- self.startMirroring(branch.id)
440+ # Mark it as started mirroring.
441+ branch.last_mirror_attempt = UTC_NOW
442+ branch.next_mirror_time = None
443 default_branch = branch.target.default_stacked_on_branch
444 if default_branch is None:
445 default_branch_name = ''
446@@ -494,26 +496,6 @@
447 else:
448 return ()
449
450- def startMirroring(self, branch_id):
451- branch = self._branch_set.get(branch_id)
452- if branch is None:
453- return faults.NoBranchWithID(branch_id)
454- branch.last_mirror_attempt = UTC_NOW
455- branch.next_mirror_time = None
456- return True
457-
458- def mirrorComplete(self, branch_id, last_revision_id):
459- branch = self._branch_set.get(branch_id)
460- if branch is None:
461- return faults.NoBranchWithID(branch_id)
462- branch.last_mirrored_id = last_revision_id
463- branch.last_mirrored = UTC_NOW
464- branch.mirror_failures = 0
465- for stacked_branch in self._branch_set:
466- if stacked_branch.stacked_on is branch:
467- stacked_branch.requestMirror()
468- return True
469-
470 def mirrorFailed(self, branch_id, reason):
471 branch = self._branch_set.get(branch_id)
472 if branch is None:
473
474=== modified file 'lib/lp/testing/factory.py'
475--- lib/lp/testing/factory.py 2010-04-27 02:05:59 +0000
476+++ lib/lp/testing/factory.py 2010-04-27 02:06:23 +0000
477@@ -863,13 +863,10 @@
478 """
479 if branch is None:
480 branch = self.makeBranch(product=product)
481- # 'branch' might be private, so we remove the security proxy to get at
482- # the methods.
483- naked_branch = removeSecurityProxy(branch)
484- naked_branch.startMirroring()
485- naked_branch.mirrorComplete('rev1')
486- # Likewise, we might not have permission to set the branch of the
487- # development focus series.
488+ # We just remove the security proxies to be able to change the objects
489+ # here.
490+ removeSecurityProxy(branch).branchChanged(
491+ '', 'rev1', None, None, None)
492 naked_series = removeSecurityProxy(product.development_focus)
493 naked_series.branch = branch
494 return branch
495@@ -881,11 +878,10 @@
496 :param branch: The branch that should be the default stacked-on
497 branch.
498 """
499- # 'branch' might be private, so we remove the security proxy to get at
500- # the methods.
501- naked_branch = removeSecurityProxy(branch)
502- naked_branch.startMirroring()
503- naked_branch.mirrorComplete('rev1')
504+ # We just remove the security proxies to be able to change the branch
505+ # here.
506+ removeSecurityProxy(branch).branchChanged(
507+ '', 'rev1', None, None, None)
508 ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches
509 run_with_login(
510 ubuntu_branches.teamowner,