Merge lp:~mwhudson/launchpad/no-hosted-area-fix-directbranchcommit 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-fix-directbranchcommit
Merge into: lp:launchpad
Prerequisite: lp:~mwhudson/launchpad/no-hosted-area-safe-getBzrBranch
Diff against target: 305 lines (+26/-82)
7 files modified
lib/lp/code/model/directbranchcommit.py (+2/-19)
lib/lp/code/model/tests/test_diff.py (+1/-1)
lib/lp/code/tests/test_directbranchcommit.py (+3/-21)
lib/lp/testing/__init__.py (+12/-32)
lib/lp/translations/scripts/tests/test_translations_to_branch.py (+5/-6)
lib/lp/translations/scripts/translations_to_branch.py (+2/-2)
lib/lp/translations/tests/test_translationtemplatesbuildjob.py (+1/-1)
To merge this branch: bzr merge lp:~mwhudson/launchpad/no-hosted-area-fix-directbranchcommit
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+23980@code.launchpad.net

Description of the change

Hi Tim,

This branch simplifies DirectBranchCommit and makes it work with in the new world.

It changes the behaviour of the useBzrBranches() test helper some, which will probably break lots of tests that I'll fix in the next few pipes...

Cheers,
mwh

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

Looks like a great change.

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/directbranchcommit.py'
2--- lib/lp/code/model/directbranchcommit.py 2010-04-27 02:23:05 +0000
3+++ lib/lp/code/model/directbranchcommit.py 2010-04-27 02:23:25 +0000
4@@ -12,13 +12,11 @@
5
6 import os.path
7
8-from bzrlib.branch import Branch
9 from bzrlib.generate_ids import gen_file_id
10 from bzrlib.revision import NULL_REVISION
11 from bzrlib.transform import TransformPreview, ROOT_PARENT
12
13 from canonical.launchpad.interfaces import IMasterObject
14-from lp.codehosting.vfs import make_branch_mirrorer
15
16
17 class ConcurrentUpdateError(Exception):
18@@ -49,7 +47,7 @@
19 is_locked = False
20 commit_builder = None
21
22- def __init__(self, db_branch, committer=None, to_mirror=False):
23+ def __init__(self, db_branch, committer=None):
24 """Create context for direct commit to branch.
25
26 Before constructing a `DirectBranchCommit`, set up a server that
27@@ -68,11 +66,8 @@
28
29 :param db_branch: a Launchpad `Branch` object.
30 :param committer: the `Person` writing to the branch.
31- :param to_mirror: If True, write to the mirrored copy of the branch
32- instead of the hosted copy. (Mainly useful for tests)
33 """
34 self.db_branch = db_branch
35- self.to_mirror = to_mirror
36
37 self.last_scanned_id = self.db_branch.last_scanned_id
38
39@@ -83,15 +78,7 @@
40 # Directories we create on the branch, and their ids.
41 self.path_ids = {}
42
43- if to_mirror:
44- self.bzrbranch = Branch.open(self.db_branch.warehouse_url)
45- else:
46- # Have the opening done through a branch mirrorer. It will
47- # pick the right policy. In case we're writing to a hosted
48- # branch stacked on a mirrored branch, the mirrorer knows
49- # how to do the right thing.
50- mirrorer = make_branch_mirrorer(self.db_branch.branch_type)
51- self.bzrbranch = mirrorer.open(self.db_branch.getPullURL())
52+ self.bzrbranch = self.db_branch.getBzrBranch()
53
54 self.bzrbranch.lock_write()
55 self.is_locked = True
56@@ -167,10 +154,6 @@
57
58 If it does, raise `ConcurrentUpdateError`.
59 """
60- # A different last_scanned_id does not indicate a race for mirrored
61- # branches -- last_scanned_id is a proxy for the mirrored branch.
62- if self.to_mirror:
63- return
64 assert self.is_locked, "Getting revision on un-locked branch."
65 last_revision = self.bzrbranch.last_revision()
66 if last_revision != self.last_scanned_id:
67
68=== modified file 'lib/lp/code/model/tests/test_diff.py'
69--- lib/lp/code/model/tests/test_diff.py 2010-04-27 02:23:05 +0000
70+++ lib/lp/code/model/tests/test_diff.py 2010-04-27 02:23:25 +0000
71@@ -44,7 +44,7 @@
72
73 This will create or modify the file, as needed.
74 """
75- committer = DirectBranchCommit(branch, to_mirror=True)
76+ committer = DirectBranchCommit(branch)
77 committer.writeFile(path, contents)
78 try:
79 return committer.commit('committing')
80
81=== modified file 'lib/lp/code/tests/test_directbranchcommit.py'
82--- lib/lp/code/tests/test_directbranchcommit.py 2009-11-18 14:34:45 +0000
83+++ lib/lp/code/tests/test_directbranchcommit.py 2010-04-27 02:23:25 +0000
84@@ -20,10 +20,10 @@
85
86 def setUp(self):
87 super(DirectBranchCommitTestCase, self).setUp()
88- self.useBzrBranches()
89+ self.useBzrBranches(direct_database=True)
90
91 self.series = self.factory.makeProductSeries()
92- self.db_branch, self.tree = self.create_branch_and_tree(hosted=True)
93+ self.db_branch, self.tree = self.create_branch_and_tree()
94
95 self.series.translations_branch = self.db_branch
96
97@@ -46,7 +46,7 @@
98
99 def _getContents(self):
100 """Return branch contents as dict mapping filenames to contents."""
101- return map_branch_contents(self.committer.db_branch.getPullURL())
102+ return map_branch_contents(self.committer.db_branch.getBzrBranch())
103
104
105 class TestDirectBranchCommit(DirectBranchCommitTestCase):
106@@ -229,23 +229,5 @@
107 self.assertEqual(dir_id, self.committer._getDir('foo/bar'))
108
109
110-class TestDirectBranchCommitMirror(TestCaseWithFactory):
111-
112- layer = ZopelessDatabaseLayer
113-
114- def test_direct_branch_commit_respects_to_mirror(self):
115- # The "to_mirror" argument causes the commit to apply to the mirrored
116- # copy of the branch.
117- self.useBzrBranches()
118- branch = self.factory.makeBranch()
119- bzr_branch = self.createBzrBranch(branch)
120- dbc = DirectBranchCommit(branch, to_mirror=True)
121- try:
122- dbc.writeFile('path', 'contents')
123- dbc.commit('making commit to mirrored area.')
124- finally:
125- dbc.unlock()
126-
127-
128 def test_suite():
129 return TestLoader().loadTestsFromName(__name__)
130
131=== modified file 'lib/lp/testing/__init__.py'
132--- lib/lp/testing/__init__.py 2010-04-27 02:23:05 +0000
133+++ lib/lp/testing/__init__.py 2010-04-27 02:23:25 +0000
134@@ -411,7 +411,7 @@
135 self.addCleanup(logout)
136 from lp.testing.factory import LaunchpadObjectFactory
137 self.factory = LaunchpadObjectFactory()
138- self.real_bzr_server = False
139+ self.direct_database_server = False
140
141 def getUserBrowser(self, url=None, user=None, password='test'):
142 """Return a Browser logged in as a fresh user, maybe opened at `url`.
143@@ -443,11 +443,6 @@
144 """
145 if format is not None and isinstance(format, basestring):
146 format = format_registry.get(format)()
147- transport = get_transport(branch_url)
148- if not self.real_bzr_server:
149- # for real bzr servers, the prefix always exists.
150- transport.create_prefix()
151- self.addCleanup(transport.delete_tree, '.')
152 return BzrDir.create_branch_convenience(
153 branch_url, format=format)
154
155@@ -468,7 +463,7 @@
156 else:
157 db_branch = self.factory.makeProductBranch(product, **kwargs)
158 branch_url = 'lp-internal:///' + db_branch.unique_name
159- if self.real_bzr_server:
160+ if not self.direct_database_server:
161 transaction.commit()
162 bzr_branch = self.createBranchAtURL(branch_url, format=format)
163 if tree_location is None:
164@@ -533,34 +528,20 @@
165 os.environ['BZR_HOME'] = os.getcwd()
166 self.addCleanup(restore_bzr_home)
167
168- def useBzrBranches(self, real_server=False, direct_database=False):
169+ def useBzrBranches(self, direct_database=False):
170 """Prepare for using bzr branches.
171
172- This sets up support for lp-hosted and lp-mirrored URLs,
173- changes to a temp directory, and overrides the bzr home directory.
174+ This sets up support for lp-internal URLs, changes to a temp
175+ directory, and overrides the bzr home directory.
176
177- :param real_server: If true, use the "real" code hosting server,
178- using an xmlrpc server, etc.
179+ :param direct_database: If true, translate branch locations by
180+ directly querying the database, not the internal XML-RPC server.
181 """
182- from lp.codehosting.scanner.tests.test_bzrsync import (
183- FakeTransportServer)
184 self.useTempBzrHome()
185- self.real_bzr_server = real_server
186- if real_server:
187- server = get_rw_server(
188- direct_database=direct_database)
189- server.start_server()
190- self.addCleanup(server.destroy)
191- else:
192- os.mkdir('lp-mirrored')
193- mirror_server = FakeTransportServer(get_transport('lp-mirrored'))
194- mirror_server.start_server()
195- self.addCleanup(mirror_server.stop_server)
196- os.mkdir('lp-hosted')
197- hosted_server = FakeTransportServer(
198- get_transport('lp-hosted'), url_prefix='lp-hosted:///')
199- hosted_server.start_server()
200- self.addCleanup(hosted_server.stop_server)
201+ self.direct_database_server = direct_database
202+ server = get_rw_server(direct_database=direct_database)
203+ server.start_server()
204+ self.addCleanup(server.destroy)
205
206
207 class BrowserTestCase(TestCaseWithFactory):
208@@ -855,7 +836,7 @@
209
210 # XXX: This doesn't seem to be a generically useful testing function. Perhaps
211 # it should go into a sub-module? -- jml
212-def map_branch_contents(branch_url):
213+def map_branch_contents(branch):
214 """Return all files in branch at `branch_url`.
215
216 :param branch_url: the URL for an accessible branch.
217@@ -863,7 +844,6 @@
218 files are included.
219 """
220 contents = {}
221- branch = BzrBranch.open(branch_url)
222 tree = branch.basis_tree()
223 tree.lock_read()
224 try:
225
226=== modified file 'lib/lp/translations/scripts/tests/test_translations_to_branch.py'
227--- lib/lp/translations/scripts/tests/test_translations_to_branch.py 2010-04-27 02:23:05 +0000
228+++ lib/lp/translations/scripts/tests/test_translations_to_branch.py 2010-04-27 02:23:25 +0000
229@@ -40,7 +40,7 @@
230 # End-to-end test of the script doing its work.
231
232 # Set up a server for hosted branches.
233- self.useBzrBranches(real_server=True)
234+ self.useBzrBranches(direct_database=False)
235
236 # Set up a product and translatable series.
237 product = self.factory.makeProduct(name='committobranch')
238@@ -48,8 +48,7 @@
239 series = product.getSeries('trunk')
240
241 # Set up a translations_branch for the series.
242- db_branch, tree = self.create_branch_and_tree(
243- hosted=True, product=product)
244+ db_branch, tree = self.create_branch_and_tree(product=product)
245 removeSecurityProxy(db_branch).last_scanned_id = 'null:'
246 product.official_rosetta = True
247 series.translations_branch = db_branch
248@@ -87,7 +86,7 @@
249
250 # The branch now contains a snapshot of the translation. (Only
251 # one file: the Dutch translation we set up earlier).
252- branch_contents = map_branch_contents(db_branch.getPullURL())
253+ branch_contents = map_branch_contents(db_branch.getBzrBranch())
254 expected_contents = {
255 'po/nl.po': """
256 # Dutch translation for .*
257@@ -163,11 +162,11 @@
258 self.useBzrBranches()
259
260 base_branch, base_tree = self.create_branch_and_tree(
261- 'base', name='base', hosted=True)
262+ 'base', name='base')
263 self._setUpBranch(base_branch, base_tree, "Base branch.")
264
265 stacked_branch, stacked_tree = self.create_branch_and_tree(
266- 'stacked', name='stacked', hosted=True)
267+ 'stacked', name='stacked')
268 stacked_tree.branch.set_stacked_on_url('/' + base_branch.unique_name)
269 stacked_branch.stacked_on = base_branch
270 self._setUpBranch(stacked_branch, stacked_tree, "Stacked branch.")
271
272=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
273--- lib/lp/translations/scripts/translations_to_branch.py 2010-03-08 19:46:02 +0000
274+++ lib/lp/translations/scripts/translations_to_branch.py 2010-04-27 02:23:25 +0000
275@@ -17,7 +17,7 @@
276 from storm.expr import Join, SQL
277
278 from canonical.launchpad.helpers import shortlist
279-from lp.codehosting.vfs import get_multi_server
280+from lp.codehosting.vfs import get_rw_server
281 from lp.translations.interfaces.potemplate import IPOTemplateSet
282 from canonical.launchpad.webapp.interfaces import (
283 IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
284@@ -270,7 +270,7 @@
285 # testing.
286 productseries = productseries.order_by(ProductSeries.id)
287
288- bzrserver = get_multi_server(write_hosted=True)
289+ bzrserver = get_rw_server()
290 bzrserver.start_server()
291 try:
292 self._exportToBranches(productseries)
293
294=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
295--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-16 13:31:48 +0000
296+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-27 02:23:25 +0000
297@@ -219,7 +219,7 @@
298 # If the feature is enabled, a TipChanged event for a branch that
299 # generates templates will schedule a templates build.
300 branch = self._makeTranslationBranch()
301- commit = DirectBranchCommit(branch, to_mirror=True)
302+ commit = DirectBranchCommit(branch)
303 commit.writeFile('POTFILES.in', 'foo')
304 commit.commit('message')
305 notify(events.TipChanged(branch, None, False))