Merge lp:~thumper/launchpad/stop-mirroring-hosted into lp:launchpad

Proposed by Tim Penhey
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11250
Proposed branch: lp:~thumper/launchpad/stop-mirroring-hosted
Merge into: lp:launchpad
Diff against target: 201 lines (+19/-30)
6 files modified
lib/lp/code/browser/tests/test_branch.py (+2/-2)
lib/lp/code/model/branch.py (+4/-4)
lib/lp/code/model/tests/test_branch.py (+9/-8)
lib/lp/code/model/tests/test_branchmergeproposaljobs.py (+0/-1)
lib/lp/code/model/tests/test_branchtarget.py (+1/-7)
lib/lp/code/xmlrpc/tests/test_codehosting.py (+3/-8)
To merge this branch: bzr merge lp:~thumper/launchpad/stop-mirroring-hosted
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+31116@code.launchpad.net

Commit message

Make the world explode if requestMirror, startMirror, or mirrorFailed are called on hosted branches.

Description of the change

This is the final branch in the stack that enforces not calling the branch mirror methods on hosted branches.

Exceptions are now raised if requestMirror, startMirror, or mirrorFailed are called on hosted branches.

Several tests that were doing this have been fixed.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

This looks basically fine. I'm not sure the comment in pending_writes is ideal now -- we shouldn't really have conversations in our source code :-) If you can fix that up a bit, please merge.

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/browser/tests/test_branch.py'
2--- lib/lp/code/browser/tests/test_branch.py 2010-07-09 10:11:47 +0000
3+++ lib/lp/code/browser/tests/test_branch.py 2010-07-28 22:53:45 +0000
4@@ -125,7 +125,7 @@
5
6 def testMirrorStatusMessageIsTruncated(self):
7 """mirror_status_message is truncated if the text is overly long."""
8- branch = self.factory.makeBranch()
9+ branch = self.factory.makeBranch(branch_type=BranchType.MIRRORED)
10 branch.mirrorFailed(
11 "on quick brown fox the dog jumps to" *
12 BranchMirrorStatusView.MAXIMUM_STATUS_MESSAGE_LENGTH)
13@@ -137,7 +137,7 @@
14
15 def testMirrorStatusMessage(self):
16 """mirror_status_message on the view is the same as on the branch."""
17- branch = self.factory.makeBranch()
18+ branch = self.factory.makeBranch(branch_type=BranchType.MIRRORED)
19 branch.mirrorFailed("This is a short error message.")
20 branch_view = BranchMirrorStatusView(branch, self.request)
21 self.assertTrue(
22
23=== modified file 'lib/lp/code/model/branch.py'
24--- lib/lp/code/model/branch.py 2010-07-25 12:55:56 +0000
25+++ lib/lp/code/model/branch.py 2010-07-28 22:53:45 +0000
26@@ -848,7 +848,7 @@
27 mirrored.
28 """
29 new_data_pushed = (
30- self.branch_type in (BranchType.HOSTED, BranchType.IMPORTED)
31+ self.branch_type == BranchType.IMPORTED
32 and self.next_mirror_time is not None)
33 # XXX 2010-04-22, MichaelHudson: This should really look for a branch
34 # scan job.
35@@ -896,7 +896,7 @@
36
37 def requestMirror(self):
38 """See `IBranch`."""
39- if self.branch_type == BranchType.REMOTE:
40+ if self.branch_type in (BranchType.REMOTE, BranchType.HOSTED):
41 raise BranchTypeError(self.unique_name)
42 branch = Store.of(self).find(
43 Branch,
44@@ -909,7 +909,7 @@
45
46 def startMirroring(self):
47 """See `IBranch`."""
48- if self.branch_type == BranchType.REMOTE:
49+ if self.branch_type in (BranchType.REMOTE, BranchType.HOSTED):
50 raise BranchTypeError(self.unique_name)
51 self.last_mirror_attempt = UTC_NOW
52 self.next_mirror_time = None
53@@ -948,7 +948,7 @@
54
55 def mirrorFailed(self, reason):
56 """See `IBranch`."""
57- if self.branch_type == BranchType.REMOTE:
58+ if self.branch_type in (BranchType.REMOTE, BranchType.HOSTED):
59 raise BranchTypeError(self.unique_name)
60 self.mirror_failures += 1
61 self.mirror_status_message = reason
62
63=== modified file 'lib/lp/code/model/tests/test_branch.py'
64--- lib/lp/code/model/tests/test_branch.py 2010-07-25 12:55:56 +0000
65+++ lib/lp/code/model/tests/test_branch.py 2010-07-28 22:53:45 +0000
66@@ -1954,11 +1954,12 @@
67 branch = self.factory.makeAnyBranch()
68 self.assertEqual(False, branch.pending_writes)
69
70- def test_requestMirror_for_hosted(self):
71- # If a hosted branch has a requested mirror, then someone has just
72- # pushed something up. Therefore, pending writes.
73+ def test_branchChanged_for_hosted(self):
74+ # If branchChanged has been called with a new tip revision id, there
75+ # are pending writes.
76 branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
77- branch.requestMirror()
78+ with person_logged_in(branch.owner):
79+ branch.branchChanged('', 'new-tip', None, None, None)
80 self.assertEqual(True, branch.pending_writes)
81
82 def test_requestMirror_for_imported(self):
83@@ -1980,7 +1981,7 @@
84 # If a branch has been pulled (mirrored) but not scanned, then we have
85 # yet to load the revisions into the database. This means there are
86 # pending writes.
87- branch = self.factory.makeAnyBranch()
88+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
89 branch.startMirroring()
90 rev_id = self.factory.getUniqueString('rev-id')
91 removeSecurityProxy(branch).branchChanged(
92@@ -1990,7 +1991,7 @@
93 def test_pulled_and_scanned(self):
94 # If a branch has been pulled and scanned, then there are no pending
95 # writes.
96- branch = self.factory.makeAnyBranch()
97+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
98 branch.startMirroring()
99 rev_id = self.factory.getUniqueString('rev-id')
100 removeSecurityProxy(branch).branchChanged(
101@@ -2004,14 +2005,14 @@
102 def test_first_mirror_started(self):
103 # If we have started mirroring the branch for the first time, then
104 # there are probably pending writes.
105- branch = self.factory.makeAnyBranch()
106+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
107 branch.startMirroring()
108 self.assertEqual(True, branch.pending_writes)
109
110 def test_following_mirror_started(self):
111 # If we have started mirroring the branch, then there are probably
112 # pending writes.
113- branch = self.factory.makeAnyBranch()
114+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
115 branch.startMirroring()
116 rev_id = self.factory.getUniqueString('rev-id')
117 removeSecurityProxy(branch).branchChanged(
118
119=== modified file 'lib/lp/code/model/tests/test_branchmergeproposaljobs.py'
120--- lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2010-04-30 04:51:04 +0000
121+++ lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2010-07-28 22:53:45 +0000
122@@ -123,7 +123,6 @@
123 def test_getOopsMailController(self):
124 """The registrant is notified about merge proposal creation issues."""
125 bmp = self.factory.makeBranchMergeProposal()
126- bmp.source_branch.requestMirror()
127 job = MergeProposalCreatedJob.create(bmp)
128 ctrl = job.getOopsMailController('1234')
129 self.assertEqual([bmp.registrant.preferredemail.email], ctrl.to_addrs)
130
131=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
132--- lib/lp/code/model/tests/test_branchtarget.py 2010-04-23 08:26:27 +0000
133+++ lib/lp/code/model/tests/test_branchtarget.py 2010-07-28 22:53:45 +0000
134@@ -107,7 +107,6 @@
135 development_package = self.original.development_version
136 default_branch = self.factory.makePackageBranch(
137 sourcepackage=development_package)
138- default_branch.startMirroring()
139 removeSecurityProxy(default_branch).branchChanged(
140 '', self.factory.getUniqueString(), None, None, None)
141 ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches
142@@ -347,7 +346,6 @@
143 # default stacked-on branch.
144 branch = self.factory.makeProductBranch(product=self.original)
145 self._setDevelopmentFocus(self.original, branch)
146- branch.startMirroring()
147 removeSecurityProxy(branch).branchChanged(
148 '', 'rev1', None, None, None)
149 target = IBranchTarget(self.original)
150@@ -468,17 +466,13 @@
151 # the current user, even if those branches have already been mirrored.
152 branch = self.factory.makeAnyBranch(private=True)
153 naked_branch = removeSecurityProxy(branch)
154- naked_branch.startMirroring()
155 naked_branch.branchChanged(
156 '', self.factory.getUniqueString(), None, None, None)
157 self.assertIs(None, check_default_stacked_on(branch))
158
159 def test_been_mirrored(self):
160- # `check_default_stacked_on` returns None if passed a remote branch.
161- # We have no Bazaar data for remote branches, so stacking on one is
162- # futile.
163+ # `check_default_stacked_on` returns the branch if it has revisions.
164 branch = self.factory.makeAnyBranch()
165- branch.startMirroring()
166 removeSecurityProxy(branch).branchChanged(
167 '', self.factory.getUniqueString(), None, None, None)
168 self.assertEqual(branch, check_default_stacked_on(branch))
169
170=== modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py'
171--- lib/lp/code/xmlrpc/tests/test_codehosting.py 2010-04-21 22:44:02 +0000
172+++ lib/lp/code/xmlrpc/tests/test_codehosting.py 2010-07-28 22:53:45 +0000
173@@ -410,17 +410,11 @@
174 message = "No such source package: 'ningnangnong'."
175 self.assertEqual(faults.NotFound(message), fault)
176
177- def test_initialMirrorRequest(self):
178- # The default 'next_mirror_time' for a newly created hosted branch
179- # should be None.
180- branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
181- self.assertIs(None, branch.next_mirror_time)
182-
183 def test_requestMirror(self):
184 # requestMirror should set the next_mirror_time field to be the
185 # current time.
186 requester = self.factory.makePerson()
187- branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
188+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
189 self.codehosting_api.requestMirror(requester.id, branch.id)
190 self.assertSqlAttributeEqualsDate(
191 branch, 'next_mirror_time', UTC_NOW)
192@@ -428,7 +422,8 @@
193 def test_requestMirror_private(self):
194 # requestMirror can be used to request the mirror of a private branch.
195 requester = self.factory.makePerson()
196- branch = self.factory.makeAnyBranch(owner=requester, private=True)
197+ branch = self.factory.makeAnyBranch(
198+ owner=requester, private=True, branch_type=BranchType.MIRRORED)
199 branch = removeSecurityProxy(branch)
200 self.codehosting_api.requestMirror(requester.id, branch.id)
201 self.assertSqlAttributeEqualsDate(