Launchpad itself

Merge lp:~jml/launchpad/better-slave-mock into lp:launchpad

  1. better-slave-mock
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Merged at revision: 11622
Proposed branch: lp:~jml/launchpad/better-slave-mock
Merge into: lp:launchpad
Diff against target: 510 lines (+159/-64)
11 files modified
lib/lp/buildmaster/doc/builder.txt (+0/-10)
lib/lp/buildmaster/model/builder.py (+41/-23)
lib/lp/buildmaster/tests/mock_slaves.py (+1/-1)
lib/lp/buildmaster/tests/test_builder.py (+8/-1)
lib/lp/codehosting/inmemory.py (+5/-1)
lib/lp/codehosting/vfs/__init__.py (+0/-2)
lib/lp/codehosting/vfs/branchfs.py (+7/-5)
lib/lp/codehosting/vfs/branchfsclient.py (+6/-18)
lib/lp/codehosting/vfs/tests/test_transport.py (+3/-2)
lib/lp/services/twistedsupport/tests/test_xmlrpc.py (+48/-1)
lib/lp/services/twistedsupport/xmlrpc.py (+40/-0)
To merge this branch: bzr merge lp:~jml/launchpad/better-slave-mock
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+36417@code.launchpad.net

Commit message

Move BlockingProxy to lp.services.twistedsupport.xmlrpc, make BuilderSlave use it.

Description of the change

This branch moves BlockingProxy from lp.codehosting.vfs and into lp.services.twistedsupport.xmlrpc. It then rejiggers the BuilderSlave class to use a callRemote style proxy rather than a getattr style one.

The branch also adds DeferredBlockingProxy, which is much like BlockingProxy, except its guaranteed to return Deferreds. It then changes codehosting to use this DeferredBlockingProxy. The net effect is to move generic code deeper down the stack, and make the codehosting vfs more focused on being a codehosting vfs.

I was going to actually improve the mocks, but I figured that would be easier to do when driven by actual tests.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (3.8 KiB)

Thanks Jonathan - a few notes below, but nothing that needs attention.

It was a good chance for me to learn a little more about twisted :)

> === modified file 'lib/lp/buildmaster/model/builder.py'
> --- lib/lp/buildmaster/model/builder.py 2010-09-21 18:43:27 +0000
> +++ lib/lp/buildmaster/model/builder.py 2010-09-23 08:14:12 +0000
> @@ -137,43 +138,55 @@
> # should make a test double that doesn't do any XML-RPC and can be used to
> # make testing easier & tests faster.
>
> - def __init__(self, urlbase, vm_host):
> - """Initialise a Server with specific parameter to our buildfarm."""
> - self.vm_host = vm_host
> - self.urlbase = urlbase
> - rpc_url = urlappend(urlbase, "rpc")
> - self._server = xmlrpclib.Server(
> + def __init__(self, proxy, file_cache_url, vm_host):
> + """Initialise a Server with specific parameter to our buildfarm.

Nothing of yours, but s/parameter/parameters/

> +
> + :param proxy: An XML-RPC proxy, implementing 'callRemote'. It must
> + support passing and returning None objects.
> + :param file_cache_url: The URL of the file cache.
> + :param vm_host: The VM host to use when resuming.
> + """
> + self._vm_host = vm_host
> + self._file_cache_url = file_cache_url
> + self._server = proxy
> +
> + @classmethod
> + def makeBlockingSlave(cls, url_base, vm_host):
> + rpc_url = urlappend(url_base, 'rpc')
> + server_proxy = xmlrpclib.ServerProxy(
> rpc_url, transport=TimeoutTransport(), allow_none=True)
> + file_cache_url = urlappend(url_base, 'filecache')
> + return cls(BlockingProxy(server_proxy), file_cache_url, vm_host)

I don't have a preference, but have noticed the code guys using static
methods unless you need to use the class for something other than
constructing (eg. lp.code.model.sourcepackagerecipe.SourcePackageRecipe.new())

> @@ -203,9 +216,10 @@
> :param libraryfilealias: An `ILibraryFileAlias`.
> """
> url = libraryfilealias.http_url
> - logger.debug("Asking builder on %s to ensure it has file %s "
> - "(%s, %s)" % (self.urlbase, libraryfilealias.filename,
> - url, libraryfilealias.content.sha1))
> + logger.debug(
> + "Asking builder on %s to ensure it has file %s (%s, %s)" % (

Ignore for now, but I wonder if we should start removing interpolation
operators from logger calls (to avoid interpolation errors). Ie.
logger.debug("...", arg1, arg2, arg3) (Although if our tests exercise all of
our logging calls, then we don't need to worry ;))

> === modified file 'lib/lp/services/twistedsupport/tests/test_xmlrpc.py'
> --- lib/lp/services/twistedsupport/tests/test_xmlrpc.py 2010-08-20 20:31:18 +0000
> +++ lib/lp/services/twistedsupport/tests/test_xmlrpc.py 2010-09-23 08:14:12 +0000
> @@ -86,5 +91,47 @@
> self.assertEqual(TestFaultOne.msg_template, fault.faultString)
>
>
> +class TestBlockingProxy(TestCase):
> +
> + def test_callRemote_calls_attribute(self):
> + class ExampleProxy:
> + def ok(self, a, b, c=None):
> + ...

Read more...

review: Approve (code)
Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (4.4 KiB)

On Thu, Sep 23, 2010 at 10:06 AM, Michael Nelson
<email address hidden> wrote:
> Review: Approve code
> Thanks Jonathan - a few notes below, but nothing that needs attention.
>
> It was a good chance for me to learn a little more about twisted :)
>
>> === modified file 'lib/lp/buildmaster/model/builder.py'
>> --- lib/lp/buildmaster/model/builder.py       2010-09-21 18:43:27 +0000
>> +++ lib/lp/buildmaster/model/builder.py       2010-09-23 08:14:12 +0000
>> @@ -137,43 +138,55 @@
>>      # should make a test double that doesn't do any XML-RPC and can be used to
>>      # make testing easier & tests faster.
>>
>> -    def __init__(self, urlbase, vm_host):
>> -        """Initialise a Server with specific parameter to our buildfarm."""
>> -        self.vm_host = vm_host
>> -        self.urlbase = urlbase
>> -        rpc_url = urlappend(urlbase, "rpc")
>> -        self._server = xmlrpclib.Server(
>> +    def __init__(self, proxy, file_cache_url, vm_host):
>> +        """Initialise a Server with specific parameter to our buildfarm.
>
> Nothing of yours, but s/parameter/parameters/
>

Fixed.

>> +
>> +        :param proxy: An XML-RPC proxy, implementing 'callRemote'. It must
>> +            support passing and returning None objects.
>> +        :param file_cache_url: The URL of the file cache.
>> +        :param vm_host: The VM host to use when resuming.
>> +        """
>> +        self._vm_host = vm_host
>> +        self._file_cache_url = file_cache_url
>> +        self._server = proxy
>> +
>> +    @classmethod
>> +    def makeBlockingSlave(cls, url_base, vm_host):
>> +        rpc_url = urlappend(url_base, 'rpc')
>> +        server_proxy = xmlrpclib.ServerProxy(
>>              rpc_url, transport=TimeoutTransport(), allow_none=True)
>> +        file_cache_url = urlappend(url_base, 'filecache')
>> +        return cls(BlockingProxy(server_proxy), file_cache_url, vm_host)
>
> I don't have a preference, but have noticed the code guys using static
> methods unless you need to use the class for something other than
> constructing (eg. lp.code.model.sourcepackagerecipe.SourcePackageRecipe.new())
>

I'm not sure why that is. My preference is for the classmethod
approach, which is often more suitable for subclassing.

>> @@ -203,9 +216,10 @@
>>          :param libraryfilealias: An `ILibraryFileAlias`.
>>          """
>>          url = libraryfilealias.http_url
>> -        logger.debug("Asking builder on %s to ensure it has file %s "
>> -                     "(%s, %s)" % (self.urlbase, libraryfilealias.filename,
>> -                                   url, libraryfilealias.content.sha1))
>> +        logger.debug(
>> +            "Asking builder on %s to ensure it has file %s (%s, %s)" % (
>
> Ignore for now, but I wonder if we should start removing interpolation
> operators from logger calls (to avoid interpolation errors).  Ie.
> logger.debug("...", arg1, arg2, arg3) (Although if our tests exercise all of
> our logging calls, then we don't need to worry ;))
>

Interesting thought.

>> === modified file 'lib/lp/services/twistedsupport/tests/test_xmlrpc.py'
>> --- lib/lp/services/twistedsupport/tests/test_xmlrpc.py       2010-08-20 20:31:18 +0000
>> ++...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/doc/builder.txt'
2--- lib/lp/buildmaster/doc/builder.txt 2010-09-22 11:26:19 +0000
3+++ lib/lp/buildmaster/doc/builder.txt 2010-09-23 12:39:48 +0000
4@@ -43,16 +43,6 @@
5 ... builder.current_build_behavior, BinaryPackageBuildBehavior)
6 True
7
8-A builder has an XML-RPC proxy in the 'slave' attribute, which allows
9-us to easily call methods on the slave machines.
10-
11- >>> s = builder.slave
12-
13-The base URL of the proxy matches the builder's URL.
14-
15- >>> s.urlbase == builder.url
16- True
17-
18
19 BuilderSet
20 ==========
21
22=== modified file 'lib/lp/buildmaster/model/builder.py'
23--- lib/lp/buildmaster/model/builder.py 2010-09-21 18:43:27 +0000
24+++ lib/lp/buildmaster/model/builder.py 2010-09-23 12:39:48 +0000
25@@ -80,6 +80,7 @@
26 from lp.services.job.model.job import Job
27 from lp.services.osutils import until_no_eintr
28 from lp.services.propertycache import cachedproperty
29+from lp.services.twistedsupport.xmlrpc import BlockingProxy
30 # XXX Michael Nelson 2010-01-13 bug=491330
31 # These dependencies on soyuz will be removed when getBuildRecords()
32 # is moved.
33@@ -113,7 +114,11 @@
34
35
36 class BuilderSlave(object):
37- """Add in a few useful methods for the XMLRPC slave."""
38+ """Add in a few useful methods for the XMLRPC slave.
39+
40+ :ivar url: The URL of the actual builder. The XML-RPC resource and
41+ the filecache live beneath this.
42+ """
43
44 # WARNING: If you change the API for this, you should also change the APIs
45 # of the mocks in soyuzbuilderhelpers to match. Otherwise, you will have
46@@ -137,43 +142,55 @@
47 # should make a test double that doesn't do any XML-RPC and can be used to
48 # make testing easier & tests faster.
49
50- def __init__(self, urlbase, vm_host):
51- """Initialise a Server with specific parameter to our buildfarm."""
52- self.vm_host = vm_host
53- self.urlbase = urlbase
54- rpc_url = urlappend(urlbase, "rpc")
55- self._server = xmlrpclib.Server(
56+ def __init__(self, proxy, builder_url, vm_host):
57+ """Initialize a BuilderSlave.
58+
59+ :param proxy: An XML-RPC proxy, implementing 'callRemote'. It must
60+ support passing and returning None objects.
61+ :param builder_url: The URL of the builder.
62+ :param vm_host: The VM host to use when resuming.
63+ """
64+ self.url = builder_url
65+ self._vm_host = vm_host
66+ self._file_cache_url = urlappend(builder_url, 'filecache')
67+ self._server = proxy
68+
69+ @classmethod
70+ def makeBlockingSlave(cls, builder_url, vm_host):
71+ rpc_url = urlappend(builder_url, 'rpc')
72+ server_proxy = xmlrpclib.ServerProxy(
73 rpc_url, transport=TimeoutTransport(), allow_none=True)
74+ return cls(BlockingProxy(server_proxy), builder_url, vm_host)
75
76 def abort(self):
77 """Abort the current build."""
78- return self._server.abort()
79+ return self._server.callRemote('abort')
80
81 def clean(self):
82 """Clean up the waiting files and reset the slave's internal state."""
83- return self._server.clean()
84+ return self._server.callRemote('clean')
85
86 def echo(self, *args):
87 """Echo the arguments back."""
88- return self._server.echo(*args)
89+ return self._server.callRemote('echo', *args)
90
91 def info(self):
92 """Return the protocol version and the builder methods supported."""
93- return self._server.info()
94+ return self._server.callRemote('info')
95
96 def status(self):
97 """Return the status of the build daemon."""
98- return self._server.status()
99+ return self._server.callRemote('status')
100
101 def ensurepresent(self, sha1sum, url, username, password):
102 """Attempt to ensure the given file is present."""
103- return self._server.ensurepresent(sha1sum, url, username, password)
104+ return self._server.callRemote(
105+ 'ensurepresent', sha1sum, url, username, password)
106
107 def getFile(self, sha_sum):
108 """Construct a file-like object to return the named file."""
109- filelocation = "filecache/%s" % sha_sum
110- fileurl = urlappend(self.urlbase, filelocation)
111- return urllib2.urlopen(fileurl)
112+ file_url = urlappend(self._file_cache_url, sha_sum)
113+ return urllib2.urlopen(file_url)
114
115 def resume(self):
116 """Resume a virtual builder.
117@@ -188,7 +205,7 @@
118 # always want to do this asynchronously, there's no need for the
119 # duplication.
120 resume_command = config.builddmaster.vm_resume_command % {
121- 'vm_host': self.vm_host}
122+ 'vm_host': self._vm_host}
123 resume_argv = resume_command.split()
124 resume_process = subprocess.Popen(
125 resume_argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
126@@ -203,9 +220,10 @@
127 :param libraryfilealias: An `ILibraryFileAlias`.
128 """
129 url = libraryfilealias.http_url
130- logger.debug("Asking builder on %s to ensure it has file %s "
131- "(%s, %s)" % (self.urlbase, libraryfilealias.filename,
132- url, libraryfilealias.content.sha1))
133+ logger.debug(
134+ "Asking builder on %s to ensure it has file %s (%s, %s)" % (
135+ self._file_cache_url, libraryfilealias.filename, url,
136+ libraryfilealias.content.sha1))
137 self.sendFileToSlave(libraryfilealias.content.sha1, url)
138
139 def sendFileToSlave(self, sha1, url, username="", password=""):
140@@ -226,8 +244,8 @@
141 the build job type.
142 """
143 try:
144- return self._server.build(
145- buildid, builder_type, chroot_sha1, filemap, args)
146+ return self._server.callRemote(
147+ 'build', buildid, builder_type, chroot_sha1, filemap, args)
148 except xmlrpclib.Fault, info:
149 raise BuildSlaveFailure(info)
150
151@@ -444,7 +462,7 @@
152 # the slave object, which is usually an XMLRPC client, with a
153 # stub object that removes the need to actually create a buildd
154 # slave in various states - which can be hard to create.
155- return BuilderSlave(self.url, self.vm_host)
156+ return BuilderSlave.makeBlockingSlave(self.url, self.vm_host)
157
158 def setSlaveForTesting(self, proxy):
159 """See IBuilder."""
160
161=== modified file 'lib/lp/buildmaster/tests/mock_slaves.py'
162--- lib/lp/buildmaster/tests/mock_slaves.py 2010-09-22 14:21:18 +0000
163+++ lib/lp/buildmaster/tests/mock_slaves.py 2010-09-23 12:39:48 +0000
164@@ -43,7 +43,7 @@
165 self.builderok = True
166 self.manual = False
167 self.url = 'http://fake:0000'
168- slave.urlbase = self.url
169+ slave.url = self.url
170 self.name = name
171 self.virtualized = True
172
173
174=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
175--- lib/lp/buildmaster/tests/test_builder.py 2010-09-22 11:26:19 +0000
176+++ lib/lp/buildmaster/tests/test_builder.py 2010-09-23 12:39:48 +0000
177@@ -129,6 +129,13 @@
178 # single EINTR.
179 self.assertEqual(0, builder.handleTimeout.call_count)
180
181+ def test_slave(self):
182+ # Builder.slave is a BuilderSlave that points at the actual Builder.
183+ # The Builder is only ever used in scripts that run outside of the
184+ # security context.
185+ builder = removeSecurityProxy(self.factory.makeBuilder())
186+ self.assertEqual(builder.url, builder.slave.url)
187+
188
189 class Test_rescueBuilderIfLost(TestCaseWithFactory):
190 """Tests for lp.buildmaster.model.builder.rescueBuilderIfLost."""
191@@ -501,7 +508,7 @@
192
193 Points to a fixed URL that is also used by `BuilddSlaveTestSetup`.
194 """
195- return BuilderSlave(self.TEST_URL, 'vmhost')
196+ return BuilderSlave.makeBlockingSlave(self.TEST_URL, 'vmhost')
197
198 def makeCacheFile(self, tachandler, filename):
199 """Make a cache file available on the remote slave.
200
201=== modified file 'lib/lp/codehosting/inmemory.py'
202--- lib/lp/codehosting/inmemory.py 2010-08-20 20:31:18 +0000
203+++ lib/lp/codehosting/inmemory.py 2010-09-23 12:39:48 +0000
204@@ -16,6 +16,7 @@
205 escape,
206 unescape,
207 )
208+from twisted.internet import defer
209 from zope.component import (
210 adapter,
211 getSiteManager,
212@@ -913,8 +914,11 @@
213 def __init__(self, endpoint):
214 self.endpoint = endpoint
215
216- def callRemote(self, method_name, *args):
217+ def _callRemote(self, method_name, *args):
218 result = getattr(self.endpoint, method_name)(*args)
219 if isinstance(result, Fault):
220 raise result
221 return result
222+
223+ def callRemote(self, method_name, *args):
224+ return defer.maybeDeferred(self._callRemote, method_name, *args)
225
226=== modified file 'lib/lp/codehosting/vfs/__init__.py'
227--- lib/lp/codehosting/vfs/__init__.py 2010-08-21 07:49:13 +0000
228+++ lib/lp/codehosting/vfs/__init__.py 2010-09-23 12:39:48 +0000
229@@ -5,7 +5,6 @@
230
231 __all__ = [
232 'AsyncLaunchpadTransport',
233- 'BlockingProxy',
234 'branch_id_to_path',
235 'BranchFileSystemClient',
236 'get_lp_server',
237@@ -25,7 +24,6 @@
238 make_branch_mirrorer,
239 )
240 from lp.codehosting.vfs.branchfsclient import (
241- BlockingProxy,
242 BranchFileSystemClient,
243 )
244
245
246=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
247--- lib/lp/codehosting/vfs/branchfs.py 2010-08-20 20:31:18 +0000
248+++ lib/lp/codehosting/vfs/branchfs.py 2010-09-23 12:39:48 +0000
249@@ -102,7 +102,6 @@
250 )
251 from lp.codehosting.bzrutils import get_stacked_on_url
252 from lp.codehosting.vfs.branchfsclient import (
253- BlockingProxy,
254 BranchFileSystemClient,
255 )
256 from lp.codehosting.vfs.transport import (
257@@ -112,7 +111,10 @@
258 get_readonly_transport,
259 TranslationError,
260 )
261-from lp.services.twistedsupport.xmlrpc import trap_fault
262+from lp.services.twistedsupport.xmlrpc import (
263+ DeferredBlockingProxy,
264+ trap_fault,
265+ )
266
267
268 class BadUrl(Exception):
269@@ -189,7 +191,7 @@
270 def get_ro_server():
271 """Get a Launchpad internal server for scanning branches."""
272 proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)
273- codehosting_endpoint = BlockingProxy(proxy)
274+ codehosting_endpoint = DeferredBlockingProxy(proxy)
275 branch_transport = get_readonly_transport(
276 get_transport(config.codehosting.internal_branch_by_id_root))
277 return LaunchpadInternalServer(
278@@ -212,7 +214,7 @@
279 return DirectDatabaseLaunchpadServer('lp-internal:///', transport)
280 else:
281 proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)
282- codehosting_endpoint = BlockingProxy(proxy)
283+ codehosting_endpoint = DeferredBlockingProxy(proxy)
284 return LaunchpadInternalServer(
285 'lp-internal:///', codehosting_endpoint, transport)
286
287@@ -719,7 +721,7 @@
288
289 codehosting_client = xmlrpclib.ServerProxy(codehosting_endpoint_url)
290 lp_server = LaunchpadServer(
291- BlockingProxy(codehosting_client), user_id, branch_transport,
292+ DeferredBlockingProxy(codehosting_client), user_id, branch_transport,
293 seen_new_branch_hook)
294 return lp_server
295
296
297=== modified file 'lib/lp/codehosting/vfs/branchfsclient.py'
298--- lib/lp/codehosting/vfs/branchfsclient.py 2010-08-10 03:24:22 +0000
299+++ lib/lp/codehosting/vfs/branchfsclient.py 2010-09-23 12:39:48 +0000
300@@ -8,7 +8,6 @@
301
302 __metaclass__ = type
303 __all__ = [
304- 'BlockingProxy',
305 'BranchFileSystemClient',
306 'NotInCache',
307 ]
308@@ -20,15 +19,6 @@
309 from lp.code.interfaces.codehosting import BRANCH_TRANSPORT
310
311
312-class BlockingProxy:
313-
314- def __init__(self, proxy):
315- self._proxy = proxy
316-
317- def callRemote(self, method_name, *args):
318- return getattr(self._proxy, method_name)(*args)
319-
320-
321 class NotInCache(Exception):
322 """Raised when we try to get a path from the cache that's not present."""
323
324@@ -50,7 +40,8 @@
325 seen_new_branch_hook=None, _now=time.time):
326 """Construct a caching codehosting_endpoint.
327
328- :param codehosting_endpoint: An XML-RPC proxy that implements callRemote.
329+ :param codehosting_endpoint: An XML-RPC proxy that implements
330+ callRemote and returns Deferreds.
331 :param user_id: The database ID of the user who will be making these
332 requests. An integer.
333 :param expiry_time: If supplied, only cache the results of
334@@ -116,9 +107,8 @@
335 :param branch_path: The path to the branch to create.
336 :return: A `Deferred` that fires the ID of the created branch.
337 """
338- return defer.maybeDeferred(
339- self._codehosting_endpoint.callRemote, 'createBranch',
340- self._user_id, branch_path)
341+ return self._codehosting_endpoint.callRemote(
342+ 'createBranch', self._user_id, branch_path)
343
344 def branchChanged(self, branch_id, stacked_on_url, last_revision_id,
345 control_string, branch_string, repository_string):
346@@ -126,8 +116,7 @@
347
348 :param branch_id: The database ID of the branch.
349 """
350- return defer.maybeDeferred(
351- self._codehosting_endpoint.callRemote,
352+ return self._codehosting_endpoint.callRemote(
353 'branchChanged', self._user_id, branch_id, stacked_on_url,
354 last_revision_id, control_string, branch_string,
355 repository_string)
356@@ -137,8 +126,7 @@
357 try:
358 return defer.succeed(self._getFromCache(path))
359 except NotInCache:
360- deferred = defer.maybeDeferred(
361- self._codehosting_endpoint.callRemote,
362+ deferred = self._codehosting_endpoint.callRemote(
363 'translatePath', self._user_id, path)
364 deferred.addCallback(self._addToCache, path)
365 return deferred
366
367=== modified file 'lib/lp/codehosting/vfs/tests/test_transport.py'
368--- lib/lp/codehosting/vfs/tests/test_transport.py 2010-08-20 20:31:18 +0000
369+++ lib/lp/codehosting/vfs/tests/test_transport.py 2010-09-23 12:39:48 +0000
370@@ -19,7 +19,7 @@
371 from lp.codehosting.inmemory import InMemoryFrontend
372 from lp.codehosting.tests.helpers import TestResultWrapper
373 from lp.codehosting.vfs.branchfs import LaunchpadInternalServer
374-from lp.codehosting.vfs.branchfsclient import BlockingProxy
375+from lp.services.twistedsupport.xmlrpc import DeferredBlockingProxy
376
377
378 class TestingServer(LaunchpadInternalServer):
379@@ -44,7 +44,8 @@
380 # unreliable for tests that involve particular errors.
381 LaunchpadInternalServer.__init__(
382 self, 'lp-testing-%s:///' % id(self),
383- BlockingProxy(branchfs), LocalTransport(local_path_to_url('.')))
384+ DeferredBlockingProxy(branchfs),
385+ LocalTransport(local_path_to_url('.')))
386 self._chroot_servers = []
387
388 def get_bogus_url(self):
389
390=== modified file 'lib/lp/services/twistedsupport/tests/test_xmlrpc.py'
391--- lib/lp/services/twistedsupport/tests/test_xmlrpc.py 2010-08-20 20:31:18 +0000
392+++ lib/lp/services/twistedsupport/tests/test_xmlrpc.py 2010-09-23 12:39:48 +0000
393@@ -10,7 +10,12 @@
394 from twisted.python.failure import Failure
395 from twisted.trial.unittest import TestCase
396
397-from lp.services.twistedsupport.xmlrpc import trap_fault
398+from lp.services.twistedsupport import extract_result
399+from lp.services.twistedsupport.xmlrpc import (
400+ BlockingProxy,
401+ DeferredBlockingProxy,
402+ trap_fault,
403+ )
404 from lp.services.xmlrpc import LaunchpadFault
405
406
407@@ -86,5 +91,47 @@
408 self.assertEqual(TestFaultOne.msg_template, fault.faultString)
409
410
411+class TestBlockingProxy(TestCase):
412+
413+ def test_callRemote_calls_attribute(self):
414+ class ExampleProxy:
415+ def ok(self, a, b, c=None):
416+ return (c, b, a)
417+ proxy = BlockingProxy(ExampleProxy())
418+ result = proxy.callRemote('ok', 2, 3, c=8)
419+ self.assertEqual((8, 3, 2), result)
420+
421+ def test_callRemote_reraises_attribute(self):
422+ class ExampleProxy:
423+ def not_ok(self, a, b, c=None):
424+ raise RuntimeError((c, b, a))
425+ proxy = BlockingProxy(ExampleProxy())
426+ error = self.assertRaises(
427+ RuntimeError, proxy.callRemote, 'not_ok', 2, 3, c=8)
428+ self.assertEqual(str(error), str((8, 3, 2)))
429+
430+
431+class TestDeferredBlockingProxy(TestCase):
432+
433+ def test_callRemote_calls_attribute(self):
434+ class ExampleProxy:
435+ def ok(self, a, b, c=None):
436+ return (c, b, a)
437+ proxy = DeferredBlockingProxy(ExampleProxy())
438+ d = proxy.callRemote('ok', 2, 3, c=8)
439+ result = extract_result(d)
440+ self.assertEqual((8, 3, 2), result)
441+
442+ def test_callRemote_traps_failure(self):
443+ class ExampleProxy:
444+ def not_ok(self, a, b, c=None):
445+ raise RuntimeError((c, b, a))
446+ proxy = DeferredBlockingProxy(ExampleProxy())
447+ d = proxy.callRemote('not_ok', 2, 3, c=8)
448+ error = self.assertRaises(RuntimeError, extract_result, d)
449+ self.assertEqual(str(error), str((8, 3, 2)))
450+
451+
452+
453 def test_suite():
454 return unittest.TestLoader().loadTestsFromName(__name__)
455
456=== modified file 'lib/lp/services/twistedsupport/xmlrpc.py'
457--- lib/lp/services/twistedsupport/xmlrpc.py 2010-04-09 12:39:23 +0000
458+++ lib/lp/services/twistedsupport/xmlrpc.py 2010-09-23 12:39:48 +0000
459@@ -5,12 +5,52 @@
460
461 __metaclass__ = type
462 __all__ = [
463+ 'BlockingProxy',
464+ 'DeferredBlockingProxy',
465 'trap_fault',
466 ]
467
468+from twisted.internet import defer
469 from twisted.web.xmlrpc import Fault
470
471
472+class BlockingProxy:
473+ """Make an xmlrpclib.ServerProxy behave like a Twisted XML-RPC proxy.
474+
475+ This is useful for writing code that needs to work in both a synchronous
476+ and asynchronous fashion.
477+
478+ Also, some people prefer the callRemote style of invocation, which is more
479+ explicit.
480+ """
481+
482+ def __init__(self, proxy):
483+ """Construct a `BlockingProxy`.
484+
485+ :param proxy: An xmlrpclib.ServerProxy.
486+ """
487+ self._proxy = proxy
488+
489+ def callRemote(self, method_name, *args, **kwargs):
490+ return getattr(self._proxy, method_name)(*args, **kwargs)
491+
492+
493+class DeferredBlockingProxy(BlockingProxy):
494+ """Make an xmlrpclib.ServerProxy behave more like a Twisted XML-RPC proxy.
495+
496+ This is almost exactly like 'BlockingProxy', except that this returns
497+ Deferreds. It is guaranteed to be exactly as synchronous as the passed-in
498+ proxy. That means if you pass in a normal xmlrpclib proxy you ought to be
499+ able to use `lp.services.twistedsupport.extract_result` to get the result.
500+ """
501+
502+ def callRemote(self, method_name, *args, **kwargs):
503+ return defer.maybeDeferred(
504+ super(DeferredBlockingProxy, self).callRemote,
505+ method_name, *args, **kwargs)
506+
507+
508+
509 def trap_fault(failure, *fault_classes):
510 """Trap a fault, based on fault code.
511