launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15812
[Merge] lp:~wgrant/launchpad/builderinteractor-privatise into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/builderinteractor-privatise into lp:launchpad with lp:~wgrant/launchpad/builderinteractor-extract as a prerequisite.
Commit message:
Privatise and reshuffle bits and pieces around BuilderInteractor.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/builderinteractor-privatise/+merge/182375
Privatise and reshuffle bits and pieces around BuilderInteractor.
--
https://code.launchpad.net/~wgrant/launchpad/builderinteractor-privatise/+merge/182375
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/builderinteractor-privatise into lp:launchpad.
=== modified file 'lib/lp/buildmaster/doc/buildfarmjobbehavior.txt'
--- lib/lp/buildmaster/doc/buildfarmjobbehavior.txt 2013-08-27 12:14:18 +0000
+++ lib/lp/buildmaster/doc/buildfarmjobbehavior.txt 2013-08-27 12:14:18 +0000
@@ -98,7 +98,7 @@
>>> from lp.buildmaster.interactor import BuilderInteractor
>>> interactor = BuilderInteractor(bob)
- >>> interactor.current_build_behavior.logStartBuild(None)
+ >>> interactor._current_build_behavior.logStartBuild(None)
Traceback (most recent call last):
...
BuildBehaviorMismatch: Builder was idle when asked to log the start of a
@@ -107,7 +107,7 @@
If a slave is working on a job while we think it is idle, it will always be
aborted.
- >>> interactor.current_build_behavior.verifySlaveBuildCookie('foo')
+ >>> interactor._current_build_behavior.verifySlaveBuildCookie('foo')
Traceback (most recent call last):
...
CorruptBuildCookie: No job assigned to builder
=== modified file 'lib/lp/buildmaster/doc/buildqueue.txt'
--- lib/lp/buildmaster/doc/buildqueue.txt 2012-12-26 01:32:19 +0000
+++ lib/lp/buildmaster/doc/buildqueue.txt 2013-08-27 12:14:18 +0000
@@ -99,16 +99,6 @@
(True, 1000)
-The BuildQueue item is responsible for providing the required build behavior
-for the item.
-
- >>> from zope.security.proxy import isinstance
- >>> from lp.soyuz.model.binarypackagebuildbehavior import (
- ... BinaryPackageBuildBehavior)
- >>> isinstance(bq.required_build_behavior, BinaryPackageBuildBehavior)
- True
-
-
== Dispatching and Reseting jobs ==
The sampledata contains an active job, being built by the 'bob'
=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py 2013-08-27 12:14:18 +0000
+++ lib/lp/buildmaster/interactor.py 2013-08-27 12:14:18 +0000
@@ -5,7 +5,6 @@
__all__ = [
'BuilderInteractor',
- 'ProxyWithConnectionTimeout',
]
import gzip
@@ -16,10 +15,7 @@
from urlparse import urlparse
import xmlrpclib
-from twisted.internet import (
- defer,
- reactor as default_reactor,
- )
+from twisted.internet import defer
from twisted.web import xmlrpc
from twisted.web.client import downloadPage
from zope.component import getUtility
@@ -35,6 +31,9 @@
CannotResumeHost,
CorruptBuildCookie,
)
+from lp.buildmaster.interfaces.buildfarmjobbehavior import (
+ IBuildFarmJobBehavior,
+ )
from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
from lp.services.config import config
from lp.services.helpers import filenameToContentType
@@ -50,38 +49,6 @@
noisy = False
-class ProxyWithConnectionTimeout(xmlrpc.Proxy):
- """Extend Twisted's Proxy to provide a configurable connection timeout."""
-
- def __init__(self, url, user=None, password=None, allowNone=False,
- useDateTime=False, timeout=None):
- xmlrpc.Proxy.__init__(
- self, url, user, password, allowNone, useDateTime)
- self.timeout = timeout
-
- def callRemote(self, method, *args):
- """Basically a carbon copy of the parent but passes the timeout
- to connectTCP."""
-
- def cancel(d):
- factory.deferred = None
- connector.disconnect()
- factory = self.queryFactory(
- self.path, self.host, method, self.user,
- self.password, self.allowNone, args, cancel, self.useDateTime)
- if self.secure:
- from twisted.internet import ssl
- connector = default_reactor.connectSSL(
- self.host, self.port or 443, factory,
- ssl.ClientContextFactory(),
- timeout=self.timeout)
- else:
- connector = default_reactor.connectTCP(
- self.host, self.port or 80, factory,
- timeout=self.timeout)
- return factory.deferred
-
-
class BuilderSlave(object):
"""Add in a few useful methods for the XMLRPC slave.
@@ -123,8 +90,8 @@
"""
rpc_url = urlappend(builder_url.encode('utf-8'), 'rpc')
if proxy is None:
- server_proxy = ProxyWithConnectionTimeout(
- rpc_url, allowNone=True, timeout=timeout)
+ server_proxy = xmlrpc.Proxy(
+ rpc_url, allowNone=True, connectTimeout=timeout)
server_proxy.queryFactory = QuietQueryFactory
else:
server_proxy = proxy
@@ -264,7 +231,7 @@
_cached_slave = None
_cached_slave_attrs = None
- # Tests can override current_build_behavior and slave.
+ # Tests can override _current_build_behavior and slave.
_override_behavior = None
_override_slave = None
@@ -293,11 +260,11 @@
return self._cached_slave
@property
- def current_build_behavior(self):
+ def _current_build_behavior(self):
"""Return the current build behavior."""
if self._override_behavior is not None:
return self._override_behavior
- # The current_build_behavior cache is invalidated when
+ # The _current_build_behavior cache is invalidated when
# builder.currentjob changes.
currentjob = self.builder.currentjob
if currentjob is None:
@@ -305,7 +272,8 @@
self._cached_build_behavior, IdleBuildBehavior):
self._cached_build_behavior = IdleBuildBehavior()
elif currentjob != self._cached_currentjob:
- self._cached_build_behavior = currentjob.required_build_behavior
+ self._cached_build_behavior = IBuildFarmJobBehavior(
+ currentjob.specific_job)
self._cached_build_behavior.setBuilderInteractor(self)
self._cached_currentjob = currentjob
return self._cached_build_behavior
@@ -332,30 +300,12 @@
if status['builder_status'] == 'BuilderStatus.BUILDING':
status['logtail'] = status_sentence[2]
- self.current_build_behavior.updateSlaveStatus(
+ self._current_build_behavior.updateSlaveStatus(
status_sentence, status)
return status
return d.addCallback(got_status)
- def slaveStatusSentence(self):
- """Get the slave status sentence for this builder.
-
- :return: A Deferred which fires when the slave dialog is complete.
- Its value is a tuple with the first element containing the
- slave status, build_id-queue-id and then optionally more
- elements depending on the status.
- """
- return self.slave.status()
-
- def verifySlaveBuildCookie(self, slave_build_id):
- """Verify that a slave's build cookie is consistent.
-
- This should delegate to the current `IBuildFarmJobBehavior`.
- """
- return self.current_build_behavior.verifySlaveBuildCookie(
- slave_build_id)
-
def isAvailable(self):
"""Whether or not a builder is available for building new jobs.
@@ -364,7 +314,7 @@
"""
if not self.builder.builderok:
return defer.succeed(False)
- d = self.slaveStatusSentence()
+ d = self.slave.status()
def catch_fault(failure):
failure.trap(xmlrpclib.Fault, socket.error)
@@ -395,7 +345,7 @@
'BuilderStatus.WAITING': 2
}
- d = self.slaveStatusSentence()
+ d = self.slave.status()
def got_status(status_sentence):
"""After we get the status, clean if we have to.
@@ -403,7 +353,7 @@
Always return status_sentence.
"""
# Isolate the BuilderStatus string, always the first token in
- # IBuilder.slaveStatusSentence().
+ # BuilderSlave.status().
status = status_sentence[0]
# If the cookie test below fails, it will request an abort of the
@@ -442,7 +392,8 @@
return
slave_build_id = status_sentence[ident_position[status]]
try:
- self.verifySlaveBuildCookie(slave_build_id)
+ self._current_build_behavior.verifySlaveBuildCookie(
+ slave_build_id)
except CorruptBuildCookie as reason:
if status == 'BuilderStatus.WAITING':
d = self.cleanSlave()
@@ -536,15 +487,15 @@
explaining what went wrong.
"""
needed_bfjb = type(removeSecurityProxy(
- build_queue_item.required_build_behavior))
- if not zope_isinstance(self.current_build_behavior, needed_bfjb):
+ IBuildFarmJobBehavior(build_queue_item.specific_job)))
+ if not zope_isinstance(self._current_build_behavior, needed_bfjb):
raise AssertionError(
"Inappropriate IBuildFarmJobBehavior: %r is not a %r" %
- (self.current_build_behavior, needed_bfjb))
- self.current_build_behavior.logStartBuild(logger)
+ (self._current_build_behavior, needed_bfjb))
+ self._current_build_behavior.logStartBuild(logger)
# Make sure the request is valid; an exception is raised if it's not.
- self.current_build_behavior.verifyBuildRequest(logger)
+ self._current_build_behavior.verifyBuildRequest(logger)
# Set the build behavior depending on the provided build queue item.
if not self.builder.builderok:
@@ -558,7 +509,7 @@
d = defer.succeed(None)
def ping_done(ignored):
- return self.current_build_behavior.dispatchBuildToSlave(
+ return self._current_build_behavior.dispatchBuildToSlave(
build_queue_item.id, logger)
def resume_done(ignored):
@@ -659,7 +610,7 @@
:return: A Deferred that fires when the slave dialog is finished.
"""
- return self.current_build_behavior.updateBuild(queueItem)
+ return self._current_build_behavior.updateBuild(queueItem)
def transferSlaveFileToLibrarian(self, file_sha1, filename, private):
"""Transfer a file from the slave to the librarian.
=== modified file 'lib/lp/buildmaster/interfaces/buildqueue.py'
--- lib/lp/buildmaster/interfaces/buildqueue.py 2013-01-07 02:40:55 +0000
+++ lib/lp/buildmaster/interfaces/buildqueue.py 2013-08-27 12:14:18 +0000
@@ -19,7 +19,6 @@
Bool,
Choice,
Datetime,
- Field,
Int,
Text,
Timedelta,
@@ -72,10 +71,6 @@
title=_('Job type'), required=True, vocabulary=BuildFarmJobType,
description=_("The type of this job."))
- required_build_behavior = Field(
- title=_('The builder behavior required to run this job.'),
- required=False, readonly=True)
-
estimated_duration = Timedelta(
title=_("Estimated Job Duration"), required=True,
description=_("Estimated job duration interval."))
=== modified file 'lib/lp/buildmaster/model/buildqueue.py'
--- lib/lp/buildmaster/model/buildqueue.py 2013-08-27 12:14:18 +0000
+++ lib/lp/buildmaster/model/buildqueue.py 2013-08-27 12:14:18 +0000
@@ -33,9 +33,6 @@
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildFarmJobType
from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
-from lp.buildmaster.interfaces.buildfarmjobbehavior import (
- IBuildFarmJobBehavior,
- )
from lp.buildmaster.interfaces.buildqueue import (
IBuildQueue,
IBuildQueueSet,
@@ -128,11 +125,6 @@
processor = ForeignKey(dbName='processor', foreignKey='Processor')
virtualized = BoolCol(dbName='virtualized')
- @property
- def required_build_behavior(self):
- """See `IBuildQueue`."""
- return IBuildFarmJobBehavior(self.specific_job)
-
@cachedproperty
def specific_job(self):
"""See `IBuildQueue`."""
=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py 2013-08-27 12:14:18 +0000
+++ lib/lp/buildmaster/tests/test_builder.py 2013-08-27 12:14:18 +0000
@@ -32,7 +32,6 @@
from lp.buildmaster.interactor import (
BuilderInteractor,
BuilderSlave,
- ProxyWithConnectionTimeout,
)
from lp.buildmaster.interfaces.builder import (
CannotFetchFile,
@@ -232,7 +231,7 @@
# At this point we should see a valid behaviour on the builder:
self.assertFalse(
zope_isinstance(
- interactor.current_build_behavior, IdleBuildBehavior))
+ interactor._current_build_behavior, IdleBuildBehavior))
# Now reset the job and try to rescue the builder.
candidate.destroySelf()
@@ -242,7 +241,7 @@
def check_builder(ignored):
self.assertIsInstance(
- removeSecurityProxy(interactor.current_build_behavior),
+ removeSecurityProxy(interactor._current_build_behavior),
IdleBuildBehavior)
return d.addCallback(check_builder)
@@ -473,7 +472,7 @@
AbortedSlave(), builder_status='BuilderStatus.ABORTED')
def test_isAvailable_with_not_builderok(self):
- # isAvailable() is a wrapper around slaveStatusSentence()
+ # isAvailable() is a wrapper around BuilderSlave.status()
builder = MockBuilder()
builder.builderok = False
d = BuilderInteractor(builder).isAvailable()
@@ -817,7 +816,7 @@
class TestCurrentBuildBehavior(TestCaseWithFactory):
"""This test ensures the get/set behavior of BuilderInteractor's
- current_build_behavior property.
+ _current_build_behavior property.
"""
layer = LaunchpadZopelessLayer
@@ -844,14 +843,14 @@
nor a current build.
"""
self.assertIsInstance(
- self.interactor.current_build_behavior, IdleBuildBehavior)
+ self.interactor._current_build_behavior, IdleBuildBehavior)
def test_current_job_behavior(self):
"""The current behavior is set automatically from the current job."""
# Set the builder attribute on the buildqueue record so that our
# builder will think it has a current build.
self.build.buildqueue_record.builder = self.builder
- behavior = removeSecurityProxy(self.interactor.current_build_behavior)
+ behavior = removeSecurityProxy(self.interactor._current_build_behavior)
self.assertIsInstance(behavior, BinaryPackageBuildBehavior)
self.assertEqual(behavior._builder, self.builder)
self.assertEqual(behavior._interactor, self.interactor)
@@ -1135,12 +1134,6 @@
self.assertTrue(d.called)
return assert_fails_with(d, CancelledError)
- def test_BuilderSlave_uses_ProxyWithConnectionTimeout(self):
- # Make sure that BuilderSlaves use the custom proxy class.
- slave = BuilderSlave.makeBuilderSlave(
- "url", "host", config.builddmaster.socket_timeout)
- self.assertIsInstance(slave._server, ProxyWithConnectionTimeout)
-
class TestSlaveWithLibrarian(TestCaseWithFactory):
"""Tests that need more of Launchpad to run."""
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-08-27 12:14:18 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-08-27 12:14:18 +0000
@@ -17,6 +17,9 @@
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interactor import BuilderInteractor
from lp.buildmaster.interfaces.builder import CorruptBuildCookie
+from lp.buildmaster.interfaces.buildfarmjobbehavior import (
+ IBuildFarmJobBehavior,
+ )
from lp.buildmaster.model.buildfarmjobbehavior import BuildFarmJobBehaviorBase
from lp.buildmaster.tests.mock_slaves import WaitingSlave
from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -183,8 +186,8 @@
def setUp(self):
super(TestGetUploadMethodsMixin, self).setUp()
self.build = self.makeBuild()
- self.behavior = removeSecurityProxy(
- self.build.buildqueue_record.required_build_behavior)
+ self.behavior = IBuildFarmJobBehavior(
+ self.build.buildqueue_record.specific_job)
def test_getUploadDirLeafCookie_parseable(self):
# getUploadDirLeaf should return a directory name
@@ -221,7 +224,7 @@
self.slave.valid_file_hashes.append('test_file_hash')
self.interactor = BuilderInteractor(self.builder, self.slave)
self.behavior = removeSecurityProxy(
- self.interactor.current_build_behavior)
+ self.interactor._current_build_behavior)
# We overwrite the buildmaster root to use a temp directory.
tempdir = tempfile.mkdtemp()
=== modified file 'lib/lp/code/model/tests/test_recipebuilder.py'
--- lib/lp/code/model/tests/test_recipebuilder.py 2013-08-27 12:14:18 +0000
+++ lib/lp/code/model/tests/test_recipebuilder.py 2013-08-27 12:14:18 +0000
@@ -372,7 +372,7 @@
queue_record.builder = self.factory.makeBuilder()
slave = WaitingSlave('BuildStatus.OK')
interactor = BuilderInteractor(queue_record.builder, slave)
- return removeSecurityProxy(interactor.current_build_behavior)
+ return removeSecurityProxy(interactor._current_build_behavior)
def assertDeferredNotifyCount(self, status, behavior, expected_count):
d = behavior.handleStatus(status, None, {'filemap': {}})
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py'
--- lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py 2013-08-27 12:14:18 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py 2013-08-27 12:14:18 +0000
@@ -23,6 +23,9 @@
BuilderSlave,
)
from lp.buildmaster.interfaces.builder import CannotBuild
+from lp.buildmaster.interfaces.buildfarmjobbehavior import (
+ IBuildFarmJobBehavior,
+ )
from lp.buildmaster.tests.mock_slaves import (
AbortedSlave,
AbortingSlave,
@@ -96,7 +99,7 @@
:return: A list of the calls we expect to be made.
"""
job = removeSecurityProxy(
- interactor.current_build_behavior).buildfarmjob
+ interactor._current_build_behavior).buildfarmjob
build_id = job.generateSlaveBuildCookie()
ds_name = build.distro_arch_series.distroseries.name
suite = ds_name + pocketsuffix[build.pocket]
@@ -218,7 +221,7 @@
transaction.commit()
build.distro_arch_series.addOrUpdateChroot(lf)
candidate = build.queueBuild()
- behavior = candidate.required_build_behavior
+ behavior = IBuildFarmJobBehavior(candidate.specific_job)
behavior.setBuilderInteractor(BuilderInteractor(builder))
e = self.assertRaises(
AssertionError, behavior.verifyBuildRequest, BufferLogger())
@@ -239,7 +242,7 @@
transaction.commit()
build.distro_arch_series.addOrUpdateChroot(lf)
candidate = build.queueBuild()
- behavior = candidate.required_build_behavior
+ behavior = IBuildFarmJobBehavior(candidate.specific_job)
behavior.setBuilderInteractor(BuilderInteractor(builder))
e = self.assertRaises(
AssertionError, behavior.verifyBuildRequest, BufferLogger())
@@ -258,7 +261,7 @@
transaction.commit()
build.distro_arch_series.addOrUpdateChroot(lf)
candidate = build.queueBuild()
- behavior = candidate.required_build_behavior
+ behavior = IBuildFarmJobBehavior(candidate.specific_job)
behavior.setBuilderInteractor(BuilderInteractor(builder))
e = self.assertRaises(
AssertionError, behavior.verifyBuildRequest, BufferLogger())
@@ -273,7 +276,7 @@
build = self.factory.makeBinaryPackageBuild(
builder=builder, archive=archive)
candidate = build.queueBuild()
- behavior = candidate.required_build_behavior
+ behavior = IBuildFarmJobBehavior(candidate.specific_job)
behavior.setBuilderInteractor(BuilderInteractor(builder))
e = self.assertRaises(
CannotBuild, behavior.verifyBuildRequest, BufferLogger())
@@ -284,7 +287,7 @@
# The uploadprocessor relies on this format.
build = self.factory.makeBinaryPackageBuild()
candidate = build.queueBuild()
- behavior = candidate.required_build_behavior
+ behavior = IBuildFarmJobBehavior(candidate.specific_job)
cookie = removeSecurityProxy(behavior).getBuildCookie()
expected_cookie = "PACKAGEBUILD-%d" % build.id
self.assertEqual(expected_cookie, cookie)
@@ -324,7 +327,7 @@
self.build.distro_arch_series.addOrUpdateChroot(lf)
self.candidate = self.build.queueBuild()
self.candidate.markAsBuilding(self.builder)
- self.behavior = self.candidate.required_build_behavior
+ self.behavior = IBuildFarmJobBehavior(self.candidate.specific_job)
self.behavior.setBuilderInteractor(BuilderInteractor(self.builder))
# This is required so that uploaded files from the buildd don't
# hang around between test runs.
Follow ups
