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
=== modified file 'lib/lp/buildmaster/doc/builder.txt'
--- lib/lp/buildmaster/doc/builder.txt 2010-09-22 11:26:19 +0000
+++ lib/lp/buildmaster/doc/builder.txt 2010-09-23 12:39:48 +0000
@@ -43,16 +43,6 @@
43 ... builder.current_build_behavior, BinaryPackageBuildBehavior)43 ... builder.current_build_behavior, BinaryPackageBuildBehavior)
44 True44 True
4545
46A builder has an XML-RPC proxy in the 'slave' attribute, which allows
47us to easily call methods on the slave machines.
48
49 >>> s = builder.slave
50
51The base URL of the proxy matches the builder's URL.
52
53 >>> s.urlbase == builder.url
54 True
55
5646
57BuilderSet47BuilderSet
58==========48==========
5949
=== 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 12:39:48 +0000
@@ -80,6 +80,7 @@
80from lp.services.job.model.job import Job80from lp.services.job.model.job import Job
81from lp.services.osutils import until_no_eintr81from lp.services.osutils import until_no_eintr
82from lp.services.propertycache import cachedproperty82from lp.services.propertycache import cachedproperty
83from lp.services.twistedsupport.xmlrpc import BlockingProxy
83# XXX Michael Nelson 2010-01-13 bug=49133084# XXX Michael Nelson 2010-01-13 bug=491330
84# These dependencies on soyuz will be removed when getBuildRecords()85# These dependencies on soyuz will be removed when getBuildRecords()
85# is moved.86# is moved.
@@ -113,7 +114,11 @@
113114
114115
115class BuilderSlave(object):116class BuilderSlave(object):
116 """Add in a few useful methods for the XMLRPC slave."""117 """Add in a few useful methods for the XMLRPC slave.
118
119 :ivar url: The URL of the actual builder. The XML-RPC resource and
120 the filecache live beneath this.
121 """
117122
118 # WARNING: If you change the API for this, you should also change the APIs123 # WARNING: If you change the API for this, you should also change the APIs
119 # of the mocks in soyuzbuilderhelpers to match. Otherwise, you will have124 # of the mocks in soyuzbuilderhelpers to match. Otherwise, you will have
@@ -137,43 +142,55 @@
137 # should make a test double that doesn't do any XML-RPC and can be used to142 # should make a test double that doesn't do any XML-RPC and can be used to
138 # make testing easier & tests faster.143 # make testing easier & tests faster.
139144
140 def __init__(self, urlbase, vm_host):145 def __init__(self, proxy, builder_url, vm_host):
141 """Initialise a Server with specific parameter to our buildfarm."""146 """Initialize a BuilderSlave.
142 self.vm_host = vm_host147
143 self.urlbase = urlbase148 :param proxy: An XML-RPC proxy, implementing 'callRemote'. It must
144 rpc_url = urlappend(urlbase, "rpc")149 support passing and returning None objects.
145 self._server = xmlrpclib.Server(150 :param builder_url: The URL of the builder.
151 :param vm_host: The VM host to use when resuming.
152 """
153 self.url = builder_url
154 self._vm_host = vm_host
155 self._file_cache_url = urlappend(builder_url, 'filecache')
156 self._server = proxy
157
158 @classmethod
159 def makeBlockingSlave(cls, builder_url, vm_host):
160 rpc_url = urlappend(builder_url, 'rpc')
161 server_proxy = xmlrpclib.ServerProxy(
146 rpc_url, transport=TimeoutTransport(), allow_none=True)162 rpc_url, transport=TimeoutTransport(), allow_none=True)
163 return cls(BlockingProxy(server_proxy), builder_url, vm_host)
147164
148 def abort(self):165 def abort(self):
149 """Abort the current build."""166 """Abort the current build."""
150 return self._server.abort()167 return self._server.callRemote('abort')
151168
152 def clean(self):169 def clean(self):
153 """Clean up the waiting files and reset the slave's internal state."""170 """Clean up the waiting files and reset the slave's internal state."""
154 return self._server.clean()171 return self._server.callRemote('clean')
155172
156 def echo(self, *args):173 def echo(self, *args):
157 """Echo the arguments back."""174 """Echo the arguments back."""
158 return self._server.echo(*args)175 return self._server.callRemote('echo', *args)
159176
160 def info(self):177 def info(self):
161 """Return the protocol version and the builder methods supported."""178 """Return the protocol version and the builder methods supported."""
162 return self._server.info()179 return self._server.callRemote('info')
163180
164 def status(self):181 def status(self):
165 """Return the status of the build daemon."""182 """Return the status of the build daemon."""
166 return self._server.status()183 return self._server.callRemote('status')
167184
168 def ensurepresent(self, sha1sum, url, username, password):185 def ensurepresent(self, sha1sum, url, username, password):
169 """Attempt to ensure the given file is present."""186 """Attempt to ensure the given file is present."""
170 return self._server.ensurepresent(sha1sum, url, username, password)187 return self._server.callRemote(
188 'ensurepresent', sha1sum, url, username, password)
171189
172 def getFile(self, sha_sum):190 def getFile(self, sha_sum):
173 """Construct a file-like object to return the named file."""191 """Construct a file-like object to return the named file."""
174 filelocation = "filecache/%s" % sha_sum192 file_url = urlappend(self._file_cache_url, sha_sum)
175 fileurl = urlappend(self.urlbase, filelocation)193 return urllib2.urlopen(file_url)
176 return urllib2.urlopen(fileurl)
177194
178 def resume(self):195 def resume(self):
179 """Resume a virtual builder.196 """Resume a virtual builder.
@@ -188,7 +205,7 @@
188 # always want to do this asynchronously, there's no need for the205 # always want to do this asynchronously, there's no need for the
189 # duplication.206 # duplication.
190 resume_command = config.builddmaster.vm_resume_command % {207 resume_command = config.builddmaster.vm_resume_command % {
191 'vm_host': self.vm_host}208 'vm_host': self._vm_host}
192 resume_argv = resume_command.split()209 resume_argv = resume_command.split()
193 resume_process = subprocess.Popen(210 resume_process = subprocess.Popen(
194 resume_argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE)211 resume_argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -203,9 +220,10 @@
203 :param libraryfilealias: An `ILibraryFileAlias`.220 :param libraryfilealias: An `ILibraryFileAlias`.
204 """221 """
205 url = libraryfilealias.http_url222 url = libraryfilealias.http_url
206 logger.debug("Asking builder on %s to ensure it has file %s "223 logger.debug(
207 "(%s, %s)" % (self.urlbase, libraryfilealias.filename,224 "Asking builder on %s to ensure it has file %s (%s, %s)" % (
208 url, libraryfilealias.content.sha1))225 self._file_cache_url, libraryfilealias.filename, url,
226 libraryfilealias.content.sha1))
209 self.sendFileToSlave(libraryfilealias.content.sha1, url)227 self.sendFileToSlave(libraryfilealias.content.sha1, url)
210228
211 def sendFileToSlave(self, sha1, url, username="", password=""):229 def sendFileToSlave(self, sha1, url, username="", password=""):
@@ -226,8 +244,8 @@
226 the build job type.244 the build job type.
227 """245 """
228 try:246 try:
229 return self._server.build(247 return self._server.callRemote(
230 buildid, builder_type, chroot_sha1, filemap, args)248 'build', buildid, builder_type, chroot_sha1, filemap, args)
231 except xmlrpclib.Fault, info:249 except xmlrpclib.Fault, info:
232 raise BuildSlaveFailure(info)250 raise BuildSlaveFailure(info)
233251
@@ -444,7 +462,7 @@
444 # the slave object, which is usually an XMLRPC client, with a462 # the slave object, which is usually an XMLRPC client, with a
445 # stub object that removes the need to actually create a buildd463 # stub object that removes the need to actually create a buildd
446 # slave in various states - which can be hard to create.464 # slave in various states - which can be hard to create.
447 return BuilderSlave(self.url, self.vm_host)465 return BuilderSlave.makeBlockingSlave(self.url, self.vm_host)
448466
449 def setSlaveForTesting(self, proxy):467 def setSlaveForTesting(self, proxy):
450 """See IBuilder."""468 """See IBuilder."""
451469
=== modified file 'lib/lp/buildmaster/tests/mock_slaves.py'
--- lib/lp/buildmaster/tests/mock_slaves.py 2010-09-22 14:21:18 +0000
+++ lib/lp/buildmaster/tests/mock_slaves.py 2010-09-23 12:39:48 +0000
@@ -43,7 +43,7 @@
43 self.builderok = True43 self.builderok = True
44 self.manual = False44 self.manual = False
45 self.url = 'http://fake:0000'45 self.url = 'http://fake:0000'
46 slave.urlbase = self.url46 slave.url = self.url
47 self.name = name47 self.name = name
48 self.virtualized = True48 self.virtualized = True
4949
5050
=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py 2010-09-22 11:26:19 +0000
+++ lib/lp/buildmaster/tests/test_builder.py 2010-09-23 12:39:48 +0000
@@ -129,6 +129,13 @@
129 # single EINTR.129 # single EINTR.
130 self.assertEqual(0, builder.handleTimeout.call_count)130 self.assertEqual(0, builder.handleTimeout.call_count)
131131
132 def test_slave(self):
133 # Builder.slave is a BuilderSlave that points at the actual Builder.
134 # The Builder is only ever used in scripts that run outside of the
135 # security context.
136 builder = removeSecurityProxy(self.factory.makeBuilder())
137 self.assertEqual(builder.url, builder.slave.url)
138
132139
133class Test_rescueBuilderIfLost(TestCaseWithFactory):140class Test_rescueBuilderIfLost(TestCaseWithFactory):
134 """Tests for lp.buildmaster.model.builder.rescueBuilderIfLost."""141 """Tests for lp.buildmaster.model.builder.rescueBuilderIfLost."""
@@ -501,7 +508,7 @@
501508
502 Points to a fixed URL that is also used by `BuilddSlaveTestSetup`.509 Points to a fixed URL that is also used by `BuilddSlaveTestSetup`.
503 """510 """
504 return BuilderSlave(self.TEST_URL, 'vmhost')511 return BuilderSlave.makeBlockingSlave(self.TEST_URL, 'vmhost')
505512
506 def makeCacheFile(self, tachandler, filename):513 def makeCacheFile(self, tachandler, filename):
507 """Make a cache file available on the remote slave.514 """Make a cache file available on the remote slave.
508515
=== modified file 'lib/lp/codehosting/inmemory.py'
--- lib/lp/codehosting/inmemory.py 2010-08-20 20:31:18 +0000
+++ lib/lp/codehosting/inmemory.py 2010-09-23 12:39:48 +0000
@@ -16,6 +16,7 @@
16 escape,16 escape,
17 unescape,17 unescape,
18 )18 )
19from twisted.internet import defer
19from zope.component import (20from zope.component import (
20 adapter,21 adapter,
21 getSiteManager,22 getSiteManager,
@@ -913,8 +914,11 @@
913 def __init__(self, endpoint):914 def __init__(self, endpoint):
914 self.endpoint = endpoint915 self.endpoint = endpoint
915916
916 def callRemote(self, method_name, *args):917 def _callRemote(self, method_name, *args):
917 result = getattr(self.endpoint, method_name)(*args)918 result = getattr(self.endpoint, method_name)(*args)
918 if isinstance(result, Fault):919 if isinstance(result, Fault):
919 raise result920 raise result
920 return result921 return result
922
923 def callRemote(self, method_name, *args):
924 return defer.maybeDeferred(self._callRemote, method_name, *args)
921925
=== modified file 'lib/lp/codehosting/vfs/__init__.py'
--- lib/lp/codehosting/vfs/__init__.py 2010-08-21 07:49:13 +0000
+++ lib/lp/codehosting/vfs/__init__.py 2010-09-23 12:39:48 +0000
@@ -5,7 +5,6 @@
55
6__all__ = [6__all__ = [
7 'AsyncLaunchpadTransport',7 'AsyncLaunchpadTransport',
8 'BlockingProxy',
9 'branch_id_to_path',8 'branch_id_to_path',
10 'BranchFileSystemClient',9 'BranchFileSystemClient',
11 'get_lp_server',10 'get_lp_server',
@@ -25,7 +24,6 @@
25 make_branch_mirrorer,24 make_branch_mirrorer,
26 )25 )
27from lp.codehosting.vfs.branchfsclient import (26from lp.codehosting.vfs.branchfsclient import (
28 BlockingProxy,
29 BranchFileSystemClient,27 BranchFileSystemClient,
30 )28 )
3129
3230
=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
--- lib/lp/codehosting/vfs/branchfs.py 2010-08-20 20:31:18 +0000
+++ lib/lp/codehosting/vfs/branchfs.py 2010-09-23 12:39:48 +0000
@@ -102,7 +102,6 @@
102 )102 )
103from lp.codehosting.bzrutils import get_stacked_on_url103from lp.codehosting.bzrutils import get_stacked_on_url
104from lp.codehosting.vfs.branchfsclient import (104from lp.codehosting.vfs.branchfsclient import (
105 BlockingProxy,
106 BranchFileSystemClient,105 BranchFileSystemClient,
107 )106 )
108from lp.codehosting.vfs.transport import (107from lp.codehosting.vfs.transport import (
@@ -112,7 +111,10 @@
112 get_readonly_transport,111 get_readonly_transport,
113 TranslationError,112 TranslationError,
114 )113 )
115from lp.services.twistedsupport.xmlrpc import trap_fault114from lp.services.twistedsupport.xmlrpc import (
115 DeferredBlockingProxy,
116 trap_fault,
117 )
116118
117119
118class BadUrl(Exception):120class BadUrl(Exception):
@@ -189,7 +191,7 @@
189def get_ro_server():191def get_ro_server():
190 """Get a Launchpad internal server for scanning branches."""192 """Get a Launchpad internal server for scanning branches."""
191 proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)193 proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)
192 codehosting_endpoint = BlockingProxy(proxy)194 codehosting_endpoint = DeferredBlockingProxy(proxy)
193 branch_transport = get_readonly_transport(195 branch_transport = get_readonly_transport(
194 get_transport(config.codehosting.internal_branch_by_id_root))196 get_transport(config.codehosting.internal_branch_by_id_root))
195 return LaunchpadInternalServer(197 return LaunchpadInternalServer(
@@ -212,7 +214,7 @@
212 return DirectDatabaseLaunchpadServer('lp-internal:///', transport)214 return DirectDatabaseLaunchpadServer('lp-internal:///', transport)
213 else:215 else:
214 proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)216 proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)
215 codehosting_endpoint = BlockingProxy(proxy)217 codehosting_endpoint = DeferredBlockingProxy(proxy)
216 return LaunchpadInternalServer(218 return LaunchpadInternalServer(
217 'lp-internal:///', codehosting_endpoint, transport)219 'lp-internal:///', codehosting_endpoint, transport)
218220
@@ -719,7 +721,7 @@
719721
720 codehosting_client = xmlrpclib.ServerProxy(codehosting_endpoint_url)722 codehosting_client = xmlrpclib.ServerProxy(codehosting_endpoint_url)
721 lp_server = LaunchpadServer(723 lp_server = LaunchpadServer(
722 BlockingProxy(codehosting_client), user_id, branch_transport,724 DeferredBlockingProxy(codehosting_client), user_id, branch_transport,
723 seen_new_branch_hook)725 seen_new_branch_hook)
724 return lp_server726 return lp_server
725727
726728
=== modified file 'lib/lp/codehosting/vfs/branchfsclient.py'
--- lib/lp/codehosting/vfs/branchfsclient.py 2010-08-10 03:24:22 +0000
+++ lib/lp/codehosting/vfs/branchfsclient.py 2010-09-23 12:39:48 +0000
@@ -8,7 +8,6 @@
88
9__metaclass__ = type9__metaclass__ = type
10__all__ = [10__all__ = [
11 'BlockingProxy',
12 'BranchFileSystemClient',11 'BranchFileSystemClient',
13 'NotInCache',12 'NotInCache',
14 ]13 ]
@@ -20,15 +19,6 @@
20from lp.code.interfaces.codehosting import BRANCH_TRANSPORT19from lp.code.interfaces.codehosting import BRANCH_TRANSPORT
2120
2221
23class BlockingProxy:
24
25 def __init__(self, proxy):
26 self._proxy = proxy
27
28 def callRemote(self, method_name, *args):
29 return getattr(self._proxy, method_name)(*args)
30
31
32class NotInCache(Exception):22class NotInCache(Exception):
33 """Raised when we try to get a path from the cache that's not present."""23 """Raised when we try to get a path from the cache that's not present."""
3424
@@ -50,7 +40,8 @@
50 seen_new_branch_hook=None, _now=time.time):40 seen_new_branch_hook=None, _now=time.time):
51 """Construct a caching codehosting_endpoint.41 """Construct a caching codehosting_endpoint.
5242
53 :param codehosting_endpoint: An XML-RPC proxy that implements callRemote.43 :param codehosting_endpoint: An XML-RPC proxy that implements
44 callRemote and returns Deferreds.
54 :param user_id: The database ID of the user who will be making these45 :param user_id: The database ID of the user who will be making these
55 requests. An integer.46 requests. An integer.
56 :param expiry_time: If supplied, only cache the results of47 :param expiry_time: If supplied, only cache the results of
@@ -116,9 +107,8 @@
116 :param branch_path: The path to the branch to create.107 :param branch_path: The path to the branch to create.
117 :return: A `Deferred` that fires the ID of the created branch.108 :return: A `Deferred` that fires the ID of the created branch.
118 """109 """
119 return defer.maybeDeferred(110 return self._codehosting_endpoint.callRemote(
120 self._codehosting_endpoint.callRemote, 'createBranch',111 'createBranch', self._user_id, branch_path)
121 self._user_id, branch_path)
122112
123 def branchChanged(self, branch_id, stacked_on_url, last_revision_id,113 def branchChanged(self, branch_id, stacked_on_url, last_revision_id,
124 control_string, branch_string, repository_string):114 control_string, branch_string, repository_string):
@@ -126,8 +116,7 @@
126116
127 :param branch_id: The database ID of the branch.117 :param branch_id: The database ID of the branch.
128 """118 """
129 return defer.maybeDeferred(119 return self._codehosting_endpoint.callRemote(
130 self._codehosting_endpoint.callRemote,
131 'branchChanged', self._user_id, branch_id, stacked_on_url,120 'branchChanged', self._user_id, branch_id, stacked_on_url,
132 last_revision_id, control_string, branch_string,121 last_revision_id, control_string, branch_string,
133 repository_string)122 repository_string)
@@ -137,8 +126,7 @@
137 try:126 try:
138 return defer.succeed(self._getFromCache(path))127 return defer.succeed(self._getFromCache(path))
139 except NotInCache:128 except NotInCache:
140 deferred = defer.maybeDeferred(129 deferred = self._codehosting_endpoint.callRemote(
141 self._codehosting_endpoint.callRemote,
142 'translatePath', self._user_id, path)130 'translatePath', self._user_id, path)
143 deferred.addCallback(self._addToCache, path)131 deferred.addCallback(self._addToCache, path)
144 return deferred132 return deferred
145133
=== modified file 'lib/lp/codehosting/vfs/tests/test_transport.py'
--- lib/lp/codehosting/vfs/tests/test_transport.py 2010-08-20 20:31:18 +0000
+++ lib/lp/codehosting/vfs/tests/test_transport.py 2010-09-23 12:39:48 +0000
@@ -19,7 +19,7 @@
19from lp.codehosting.inmemory import InMemoryFrontend19from lp.codehosting.inmemory import InMemoryFrontend
20from lp.codehosting.tests.helpers import TestResultWrapper20from lp.codehosting.tests.helpers import TestResultWrapper
21from lp.codehosting.vfs.branchfs import LaunchpadInternalServer21from lp.codehosting.vfs.branchfs import LaunchpadInternalServer
22from lp.codehosting.vfs.branchfsclient import BlockingProxy22from lp.services.twistedsupport.xmlrpc import DeferredBlockingProxy
2323
2424
25class TestingServer(LaunchpadInternalServer):25class TestingServer(LaunchpadInternalServer):
@@ -44,7 +44,8 @@
44 # unreliable for tests that involve particular errors.44 # unreliable for tests that involve particular errors.
45 LaunchpadInternalServer.__init__(45 LaunchpadInternalServer.__init__(
46 self, 'lp-testing-%s:///' % id(self),46 self, 'lp-testing-%s:///' % id(self),
47 BlockingProxy(branchfs), LocalTransport(local_path_to_url('.')))47 DeferredBlockingProxy(branchfs),
48 LocalTransport(local_path_to_url('.')))
48 self._chroot_servers = []49 self._chroot_servers = []
4950
50 def get_bogus_url(self):51 def get_bogus_url(self):
5152
=== 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 12:39:48 +0000
@@ -10,7 +10,12 @@
10from twisted.python.failure import Failure10from twisted.python.failure import Failure
11from twisted.trial.unittest import TestCase11from twisted.trial.unittest import TestCase
1212
13from lp.services.twistedsupport.xmlrpc import trap_fault13from lp.services.twistedsupport import extract_result
14from lp.services.twistedsupport.xmlrpc import (
15 BlockingProxy,
16 DeferredBlockingProxy,
17 trap_fault,
18 )
14from lp.services.xmlrpc import LaunchpadFault19from lp.services.xmlrpc import LaunchpadFault
1520
1621
@@ -86,5 +91,47 @@
86 self.assertEqual(TestFaultOne.msg_template, fault.faultString)91 self.assertEqual(TestFaultOne.msg_template, fault.faultString)
8792
8893
94class TestBlockingProxy(TestCase):
95
96 def test_callRemote_calls_attribute(self):
97 class ExampleProxy:
98 def ok(self, a, b, c=None):
99 return (c, b, a)
100 proxy = BlockingProxy(ExampleProxy())
101 result = proxy.callRemote('ok', 2, 3, c=8)
102 self.assertEqual((8, 3, 2), result)
103
104 def test_callRemote_reraises_attribute(self):
105 class ExampleProxy:
106 def not_ok(self, a, b, c=None):
107 raise RuntimeError((c, b, a))
108 proxy = BlockingProxy(ExampleProxy())
109 error = self.assertRaises(
110 RuntimeError, proxy.callRemote, 'not_ok', 2, 3, c=8)
111 self.assertEqual(str(error), str((8, 3, 2)))
112
113
114class TestDeferredBlockingProxy(TestCase):
115
116 def test_callRemote_calls_attribute(self):
117 class ExampleProxy:
118 def ok(self, a, b, c=None):
119 return (c, b, a)
120 proxy = DeferredBlockingProxy(ExampleProxy())
121 d = proxy.callRemote('ok', 2, 3, c=8)
122 result = extract_result(d)
123 self.assertEqual((8, 3, 2), result)
124
125 def test_callRemote_traps_failure(self):
126 class ExampleProxy:
127 def not_ok(self, a, b, c=None):
128 raise RuntimeError((c, b, a))
129 proxy = DeferredBlockingProxy(ExampleProxy())
130 d = proxy.callRemote('not_ok', 2, 3, c=8)
131 error = self.assertRaises(RuntimeError, extract_result, d)
132 self.assertEqual(str(error), str((8, 3, 2)))
133
134
135
89def test_suite():136def test_suite():
90 return unittest.TestLoader().loadTestsFromName(__name__)137 return unittest.TestLoader().loadTestsFromName(__name__)
91138
=== modified file 'lib/lp/services/twistedsupport/xmlrpc.py'
--- lib/lp/services/twistedsupport/xmlrpc.py 2010-04-09 12:39:23 +0000
+++ lib/lp/services/twistedsupport/xmlrpc.py 2010-09-23 12:39:48 +0000
@@ -5,12 +5,52 @@
55
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'BlockingProxy',
9 'DeferredBlockingProxy',
8 'trap_fault',10 'trap_fault',
9 ]11 ]
1012
13from twisted.internet import defer
11from twisted.web.xmlrpc import Fault14from twisted.web.xmlrpc import Fault
1215
1316
17class BlockingProxy:
18 """Make an xmlrpclib.ServerProxy behave like a Twisted XML-RPC proxy.
19
20 This is useful for writing code that needs to work in both a synchronous
21 and asynchronous fashion.
22
23 Also, some people prefer the callRemote style of invocation, which is more
24 explicit.
25 """
26
27 def __init__(self, proxy):
28 """Construct a `BlockingProxy`.
29
30 :param proxy: An xmlrpclib.ServerProxy.
31 """
32 self._proxy = proxy
33
34 def callRemote(self, method_name, *args, **kwargs):
35 return getattr(self._proxy, method_name)(*args, **kwargs)
36
37
38class DeferredBlockingProxy(BlockingProxy):
39 """Make an xmlrpclib.ServerProxy behave more like a Twisted XML-RPC proxy.
40
41 This is almost exactly like 'BlockingProxy', except that this returns
42 Deferreds. It is guaranteed to be exactly as synchronous as the passed-in
43 proxy. That means if you pass in a normal xmlrpclib proxy you ought to be
44 able to use `lp.services.twistedsupport.extract_result` to get the result.
45 """
46
47 def callRemote(self, method_name, *args, **kwargs):
48 return defer.maybeDeferred(
49 super(DeferredBlockingProxy, self).callRemote,
50 method_name, *args, **kwargs)
51
52
53
14def trap_fault(failure, *fault_classes):54def trap_fault(failure, *fault_classes):
15 """Trap a fault, based on fault code.55 """Trap a fault, based on fault code.
1656