launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05653
[Merge] lp:~jtv/launchpad/bug-717969 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-717969 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #717969 in Launchpad itself: "storeBuildInfo is sometimes ineffective"
https://bugs.launchpad.net/launchpad/+bug/717969
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-717969/+merge/83022
= Bug 717969 =
The buildmaster was showing very strange symptoms. In a nutshell, cases where observed where it went through steps A, B, and C; we could see that steps A and C were completed successfully; but the expected results from step B were just not there.
The cause turns out to be a dangerous mix of global state: our database transaction management is global to a thread (all of the thread's changes are committed or all are aborted) but our use of twisted makes a single python thread/process flit back and forth between servicing multiple logical threads of execution.
Each of those threads of execution might, from its point of view: make some changes to ORM-backed objects, then initiate an asynchronous request (talking to the librarian or a build slave), block for the result to arrive, make some more ORM changes, block again, and later commit or abort the database transaction.
That is purely from the point of view of a logical thread of execution. Twisted experts are quick to point out that “nothing blocks” in Twisted, but that is exactly the problem: while one logical thread of execution “blocks,” another gets to execute. And who's to say whether that other one might not commit or abort the first one's changes prematurely? With the numbers of builders we drive, it's bound to happen regularly.
We discussed a few architectural solutions: farming off the ORM changes to a worker thread, doing all the work in threads, and so on. The real problem is that the ORM changes are inlined in the buildmaster, but largely hidden away in the various build-related class hierarchies. Twisted is meant for glue between systems, not for interleaving complex and dispersed stateful machinery with the glue's callback structure.
Ideally, we should separate the “glue” from the “machinery.” But it's complex and fragile, so the next step from here is to get the code under control to the point where we can reason reliably about it. Once we know more about what the code really does, we'll have more freedom to reorganize it.
This branch represents that next step from here: make the buildmaster run in read-only transaction policy. It will not be able to make any changes to the database (or ORM-backed objects), except in sections of code that are explicitly wrapped in read-write transaction policies. Now we know exactly where the buildmaster may change the database — doing it anywhere else would be an error. We keep the read-write sections absolutely minimal, and try to avoid method calls where Twisted context switches might hide. Unfortunately neither Twisted nor Storm seems to have a facility for forbidding them altogether.
Every read-write section commits immediately. That means more fine-grained state changes. I can't be 100% certain that early commits will not produce any unwanted object states, and I can't be 100% certain that the read-write policies cover all code paths that need them. But we've run several types of build through the patched build farm, and we've stress-tested it against about 5 concurrent builders working all-out. As expected, we found code that still lacked an explicit read-write policy — but only one of them. There may be more, but only production use will find them all.
For Q/A, we once again perform builds on staging and/or dogfood, of as many kinds as we can. Include concurrent builds, and successes as well as failures. Verify that the build products (tarballs, packages, changes files, build logs) all go into the librarian, and that the builds end up in a proper terminal state. The contents of the build logs should be consistent with that end state.
As for tests… sorry, I just run them all!
No lint that I could help, copyrights updated, imports formatted as per policy. Thank you for reading this far.
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/bug-717969/+merge/83022
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-717969 into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
--- lib/lp/buildmaster/interfaces/builder.py 2011-07-14 21:49:37 +0000
+++ lib/lp/buildmaster/interfaces/builder.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213
@@ -25,10 +25,10 @@
export_as_webservice_entry,
export_read_operation,
exported,
+ operation_for_version,
operation_parameters,
+ operation_returns_collection_of,
operation_returns_entry,
- operation_returns_collection_of,
- operation_for_version,
)
from lazr.restful.fields import (
Reference,
@@ -50,12 +50,12 @@
from lp.app.validators.name import name_validator
from lp.app.validators.url import builder_url_validator
from lp.registry.interfaces.role import IHasOwner
-from lp.soyuz.interfaces.processor import IProcessor
from lp.services.fields import (
Description,
PersonChoice,
Title,
)
+from lp.soyuz.interfaces.processor import IProcessor
class BuildDaemonError(Exception):
@@ -195,6 +195,8 @@
def setSlaveForTesting(proxy):
"""Sets the RPC proxy through which to operate the build slave."""
+ # XXX JeroenVermeulen 2011-11-09, bug=888010: Don't use this.
+ # It's a trap. See bug for details.
def verifySlaveBuildCookie(slave_build_id):
"""Verify that a slave's build cookie is consistent.
=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py 2011-10-31 09:23:47 +0000
+++ lib/lp/buildmaster/manager.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Soyuz buildd slave manager logic."""
@@ -23,10 +23,6 @@
from zope.component import getUtility
from lp.buildmaster.enums import BuildStatus
-from lp.buildmaster.interfaces.buildfarmjobbehavior import (
- BuildBehaviorMismatch,
- )
-from lp.buildmaster.model.builder import Builder
from lp.buildmaster.interfaces.builder import (
BuildDaemonError,
BuildSlaveFailure,
@@ -34,6 +30,11 @@
CannotFetchFile,
CannotResumeHost,
)
+from lp.buildmaster.interfaces.buildfarmjobbehavior import (
+ BuildBehaviorMismatch,
+ )
+from lp.buildmaster.model.builder import Builder
+from lp.services.database.transaction_policy import DatabaseTransactionPolicy
BUILDD_MANAGER_LOG_NAME = "slave-scanner"
@@ -138,51 +139,58 @@
1. Print the error in the log
2. Increment and assess failure counts on the builder and job.
"""
- # Make sure that pending database updates are removed as it
- # could leave the database in an inconsistent state (e.g. The
- # job says it's running but the buildqueue has no builder set).
+ # Since this is a failure path, we could be in a broken
+ # transaction. Get us a fresh one.
transaction.abort()
# If we don't recognise the exception include a stack trace with
# the error.
error_message = failure.getErrorMessage()
- if failure.check(
+ familiar_error = failure.check(
BuildSlaveFailure, CannotBuild, BuildBehaviorMismatch,
- CannotResumeHost, BuildDaemonError, CannotFetchFile):
- self.logger.info("Scanning %s failed with: %s" % (
- self.builder_name, error_message))
+ CannotResumeHost, BuildDaemonError, CannotFetchFile)
+ if familiar_error:
+ self.logger.info(
+ "Scanning %s failed with: %s",
+ self.builder_name, error_message)
else:
- self.logger.info("Scanning %s failed with: %s\n%s" % (
+ self.logger.info(
+ "Scanning %s failed with: %s\n%s",
self.builder_name, failure.getErrorMessage(),
- failure.getTraceback()))
+ failure.getTraceback())
# Decide if we need to terminate the job or fail the
# builder.
try:
builder = get_builder(self.builder_name)
- builder.gotFailure()
- if builder.currentjob is not None:
+ if builder.currentjob is None:
+ self.logger.info(
+ "Builder %s failed a probe, count: %s",
+ self.builder_name, builder.failure_count)
+ else:
build_farm_job = builder.getCurrentBuildFarmJob()
- build_farm_job.gotFailure()
self.logger.info(
"builder %s failure count: %s, "
- "job '%s' failure count: %s" % (
- self.builder_name,
- builder.failure_count,
- build_farm_job.title,
- build_farm_job.failure_count))
- else:
- self.logger.info(
- "Builder %s failed a probe, count: %s" % (
- self.builder_name, builder.failure_count))
- assessFailureCounts(builder, failure.getErrorMessage())
+ "job '%s' failure count: %s",
+ self.builder_name,
+ builder.failure_count,
+ build_farm_job.title,
+ build_farm_job.failure_count)
+
transaction.commit()
+
+ with DatabaseTransactionPolicy(read_only=False):
+ builder.gotFailure()
+ if builder.currentjob is not None:
+ build_farm_job.gotFailure()
+ assessFailureCounts(builder, failure.getErrorMessage())
+ transaction.commit()
except:
# Catastrophic code failure! Not much we can do.
+ transaction.abort()
self.logger.error(
"Miserable failure when trying to examine failure counts:\n",
exc_info=True)
- transaction.abort()
def checkCancellation(self, builder):
"""See if there is a pending cancellation request.
@@ -236,14 +244,9 @@
"""
# We need to re-fetch the builder object on each cycle as the
# Storm store is invalidated over transaction boundaries.
-
self.builder = get_builder(self.builder_name)
def status_updated(ignored):
- # Commit the changes done while possibly rescuing jobs, to
- # avoid holding table locks.
- transaction.commit()
-
# See if we think there's an active build on the builder.
buildqueue = self.builder.getBuildQueue()
@@ -253,14 +256,10 @@
return self.builder.updateBuild(buildqueue)
def build_updated(ignored):
- # Commit changes done while updating the build, to avoid
- # holding table locks.
- transaction.commit()
-
# If the builder is in manual mode, don't dispatch anything.
if self.builder.manual:
self.logger.debug(
- '%s is in manual mode, not dispatching.' %
+ '%s is in manual mode, not dispatching.',
self.builder.name)
return
@@ -278,22 +277,33 @@
job = self.builder.currentjob
if job is not None and not self.builder.builderok:
self.logger.info(
- "%s was made unavailable, resetting attached "
- "job" % self.builder.name)
- job.reset()
- transaction.commit()
+ "%s was made unavailable; resetting attached job.",
+ self.builder.name)
+ transaction.abort()
+ with DatabaseTransactionPolicy(read_only=False):
+ job.reset()
+ transaction.commit()
return
# See if there is a job we can dispatch to the builder slave.
+ # XXX JeroenVermeulen 2011-10-11, bug=872112: The job's
+ # failure count will be reset once the job has started
+ # successfully. Because of intervening commits, you may see
+ # a build with a nonzero failure count that's actually going
+ # to succeed later (and have a failure count of zero). Or
+ # it may fail yet end up with a lower failure count than you
+ # saw earlier.
d = self.builder.findAndStartJob()
def job_started(candidate):
if self.builder.currentjob is not None:
# After a successful dispatch we can reset the
# failure_count.
- self.builder.resetFailureCount()
- transaction.commit()
+ transaction.abort()
+ with DatabaseTransactionPolicy(read_only=False):
+ self.builder.resetFailureCount()
+ transaction.commit()
return self.builder.slave
else:
return None
@@ -372,6 +382,7 @@
self.logger = self._setupLogger()
self.new_builders_scanner = NewBuildersScanner(
manager=self, clock=clock)
+ self.transaction_policy = DatabaseTransactionPolicy(read_only=True)
def _setupLogger(self):
"""Set up a 'slave-scanner' logger that redirects to twisted.
@@ -390,16 +401,28 @@
logger.setLevel(level)
return logger
+ def enterReadOnlyDatabasePolicy(self):
+ """Set the database transaction policy to read-only.
+
+ Any previously pending changes are committed first.
+ """
+ transaction.commit()
+ self.transaction_policy.__enter__()
+
+ def exitReadOnlyDatabasePolicy(self, *args):
+ """Reset database transaction policy to the default read-write."""
+ self.transaction_policy.__exit__(None, None, None)
+
def startService(self):
"""Service entry point, called when the application starts."""
+ # Avoiding circular imports.
+ from lp.buildmaster.interfaces.builder import IBuilderSet
+
+ self.enterReadOnlyDatabasePolicy()
# Get a list of builders and set up scanners on each one.
-
- # Avoiding circular imports.
- from lp.buildmaster.interfaces.builder import IBuilderSet
- builder_set = getUtility(IBuilderSet)
- builders = [builder.name for builder in builder_set]
- self.addScanForBuilders(builders)
+ self.addScanForBuilders(
+ [builder.name for builder in getUtility(IBuilderSet)])
self.new_builders_scanner.scheduleScan()
# Events will now fire in the SlaveScanner objects to scan each
@@ -420,6 +443,7 @@
# stopped, so we can wait on them all at once here before
# exiting.
d = defer.DeferredList(deferreds, consumeErrors=True)
+ d.addCallback(self.exitReadOnlyDatabasePolicy)
return d
def addScanForBuilders(self, builders):
=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py 2011-11-20 23:37:23 +0000
+++ lib/lp/buildmaster/model/builder.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009,2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0611,W0212
@@ -18,7 +18,6 @@
import os
import socket
import tempfile
-import transaction
import xmlrpclib
from lazr.restful.utils import safe_hasattr
@@ -34,6 +33,7 @@
Count,
Sum,
)
+import transaction
from twisted.internet import (
defer,
reactor as default_reactor,
@@ -76,11 +76,12 @@
specific_job_classes,
)
from lp.registry.interfaces.person import validate_public_person
+from lp.services.database.transaction_policy import DatabaseTransactionPolicy
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
from lp.services.propertycache import cachedproperty
+from lp.services.twistedsupport import cancel_on_timeout
from lp.services.twistedsupport.processmonitor import ProcessWithTimeout
-from lp.services.twistedsupport import cancel_on_timeout
# XXX Michael Nelson 2010-01-13 bug=491330
# These dependencies on soyuz will be removed when getBuildRecords()
# is moved.
@@ -537,6 +538,8 @@
def setSlaveForTesting(self, proxy):
"""See IBuilder."""
+ # XXX JeroenVermeulen 2011-11-09, bug=888010: Don't use this.
+ # It's a trap. See bug for details.
self.slave = proxy
def startBuild(self, build_queue_item, logger):
@@ -664,10 +667,13 @@
bytes_written = out_file.tell()
out_file.seek(0)
- library_file = getUtility(ILibraryFileAliasSet).create(
- filename, bytes_written, out_file,
- contentType=filenameToContentType(filename),
- restricted=private)
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ library_file = getUtility(ILibraryFileAliasSet).create(
+ filename, bytes_written, out_file,
+ contentType=filenameToContentType(filename),
+ restricted=private)
+ transaction.commit()
finally:
# Remove the temporary file. getFile() closes the file
# object.
@@ -705,7 +711,7 @@
def acquireBuildCandidate(self):
"""Acquire a build candidate in an atomic fashion.
- When retrieiving a candidate we need to mark it as building
+ When retrieving a candidate we need to mark it as building
immediately so that it is not dispatched by another builder in the
build manager.
@@ -715,12 +721,15 @@
can be in this code at the same time.
If there's ever more than one build manager running at once, then
- this code will need some sort of mutex.
+ this code will need some sort of mutex, or run in a single
+ transaction.
"""
candidate = self._findBuildCandidate()
if candidate is not None:
- candidate.markAsBuilding(self)
transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ candidate.markAsBuilding(self)
+ transaction.commit()
return candidate
def _findBuildCandidate(self):
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py 2011-11-21 12:23:40 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py 2011-06-09 10:50:25 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213
@@ -16,8 +16,8 @@
import socket
import xmlrpclib
+import transaction
from twisted.internet import defer
-
from zope.component import getUtility
from zope.interface import implements
from zope.security.proxy import removeSecurityProxy
@@ -32,6 +32,7 @@
IBuildFarmJobBehavior,
)
from lp.services import encoding
+from lp.services.database.transaction_policy import DatabaseTransactionPolicy
from lp.services.job.interfaces.job import JobStatus
@@ -70,6 +71,25 @@
if slave_build_cookie != expected_cookie:
raise CorruptBuildCookie("Invalid slave build cookie.")
+ def _getBuilderStatusHandler(self, status_text, logger):
+ """Look up the handler method for a given builder status.
+
+ If status is not a known one, logs an error and returns None.
+ """
+ builder_status_handlers = {
+ 'BuilderStatus.IDLE': self.updateBuild_IDLE,
+ 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
+ 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
+ 'BuilderStatus.ABORTED': self.updateBuild_ABORTED,
+ 'BuilderStatus.WAITING': self.updateBuild_WAITING,
+ }
+ handler = builder_status_handlers.get(status_text)
+ if handler is None:
+ logger.critical(
+ "Builder on %s returned unknown status %s; failing it.",
+ self._builder.url, status_text)
+ return handler
+
def updateBuild(self, queueItem):
"""See `IBuildFarmJobBehavior`."""
logger = logging.getLogger('slave-scanner')
@@ -77,6 +97,7 @@
d = self._builder.slaveStatus()
def got_failure(failure):
+ transaction.abort()
failure.trap(xmlrpclib.Fault, socket.error)
info = failure.value
info = ("Could not contact the builder %s, caught a (%s)"
@@ -84,27 +105,22 @@
raise BuildSlaveFailure(info)
def got_status(slave_status):
- builder_status_handlers = {
- 'BuilderStatus.IDLE': self.updateBuild_IDLE,
- 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
- 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
- 'BuilderStatus.ABORTED': self.updateBuild_ABORTED,
- 'BuilderStatus.WAITING': self.updateBuild_WAITING,
- }
-
builder_status = slave_status['builder_status']
- if builder_status not in builder_status_handlers:
- logger.critical(
- "Builder on %s returned unknown status %s, failing it"
- % (self._builder.url, builder_status))
- self._builder.failBuilder(
+ status_handler = self._getBuilderStatusHandler(
+ builder_status, logger)
+ if status_handler is None:
+ error = (
"Unknown status code (%s) returned from status() probe."
% builder_status)
- # XXX: This will leave the build and job in a bad state, but
- # should never be possible, since our builder statuses are
- # known.
- queueItem._builder = None
- queueItem.setDateStarted(None)
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ self._builder.failBuilder(error)
+ # XXX: This will leave the build and job in a bad
+ # state, but should never be possible since our
+ # builder statuses are known.
+ queueItem._builder = None
+ queueItem.setDateStarted(None)
+ transaction.commit()
return
# Since logtail is a xmlrpclib.Binary container and it is
@@ -114,9 +130,8 @@
# will simply remove the proxy.
logtail = removeSecurityProxy(slave_status.get('logtail'))
- method = builder_status_handlers[builder_status]
return defer.maybeDeferred(
- method, queueItem, slave_status, logtail, logger)
+ status_handler, queueItem, slave_status, logtail, logger)
d.addErrback(got_failure)
d.addCallback(got_status)
@@ -128,22 +143,32 @@
Log this and reset the record.
"""
logger.warn(
- "Builder %s forgot about buildqueue %d -- resetting buildqueue "
- "record" % (queueItem.builder.url, queueItem.id))
- queueItem.reset()
+ "Builder %s forgot about buildqueue %d -- "
+ "resetting buildqueue record.",
+ queueItem.builder.url, queueItem.id)
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ queueItem.reset()
+ transaction.commit()
def updateBuild_BUILDING(self, queueItem, slave_status, logtail, logger):
"""Build still building, collect the logtail"""
- if queueItem.job.status != JobStatus.RUNNING:
- queueItem.job.start()
- queueItem.logtail = encoding.guess(str(logtail))
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ if queueItem.job.status != JobStatus.RUNNING:
+ queueItem.job.start()
+ queueItem.logtail = encoding.guess(str(logtail))
+ transaction.commit()
def updateBuild_ABORTING(self, queueItem, slave_status, logtail, logger):
"""Build was ABORTED.
Master-side should wait until the slave finish the process correctly.
"""
- queueItem.logtail = "Waiting for slave process to be terminated"
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ queueItem.logtail = "Waiting for slave process to be terminated"
+ transaction.commit()
def updateBuild_ABORTED(self, queueItem, slave_status, logtail, logger):
"""ABORTING process has successfully terminated.
@@ -151,11 +176,16 @@
Clean the builder for another jobs.
"""
d = queueItem.builder.cleanSlave()
+
def got_cleaned(ignored):
- queueItem.builder = None
- if queueItem.job.status != JobStatus.FAILED:
- queueItem.job.fail()
- queueItem.specific_job.jobAborted()
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ queueItem.builder = None
+ if queueItem.job.status != JobStatus.FAILED:
+ queueItem.job.fail()
+ queueItem.specific_job.jobAborted()
+ transaction.commit()
+
return d.addCallback(got_cleaned)
def extractBuildStatus(self, slave_status):
=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py 2011-08-31 19:41:51 +0000
+++ lib/lp/buildmaster/model/packagebuild.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -9,11 +9,11 @@
]
+from cStringIO import StringIO
import datetime
import logging
import os.path
-from cStringIO import StringIO
from lazr.delegates import delegates
import pytz
from storm.expr import Desc
@@ -24,6 +24,7 @@
Storm,
Unicode,
)
+import transaction
from zope.component import getUtility
from zope.interface import (
classProvides,
@@ -43,8 +44,8 @@
MAIN_STORE,
)
from lp.buildmaster.enums import (
+ BuildFarmJobType,
BuildStatus,
- BuildFarmJobType,
)
from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
from lp.buildmaster.interfaces.packagebuild import (
@@ -57,9 +58,8 @@
BuildFarmJobDerived,
)
from lp.buildmaster.model.buildqueue import BuildQueue
-from lp.registry.interfaces.pocket import (
- PackagePublishingPocket,
- )
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.database.transaction_policy import DatabaseTransactionPolicy
from lp.soyuz.adapters.archivedependencies import (
default_component_dependency_name,
)
@@ -179,19 +179,24 @@
def storeBuildInfo(build, librarian, slave_status):
"""See `IPackageBuild`."""
def got_log(lfa_id):
+ dependencies = slave_status.get('dependencies')
+ if dependencies is not None:
+ dependencies = unicode(dependencies)
+
# log, builder and date_finished are read-only, so we must
# currently remove the security proxy to set them.
naked_build = removeSecurityProxy(build)
- naked_build.log = lfa_id
- naked_build.builder = build.buildqueue_record.builder
- # XXX cprov 20060615 bug=120584: Currently buildduration includes
- # the scanner latency, it should really be asking the slave for
- # the duration spent building locally.
- naked_build.date_finished = datetime.datetime.now(pytz.UTC)
- if slave_status.get('dependencies') is not None:
- build.dependencies = unicode(slave_status.get('dependencies'))
- else:
- build.dependencies = None
+
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ naked_build.log = lfa_id
+ naked_build.builder = build.buildqueue_record.builder
+ # XXX cprov 20060615 bug=120584: Currently buildduration
+ # includes the scanner latency. It should really be asking
+ # the slave for the duration spent building locally.
+ naked_build.date_finished = datetime.datetime.now(pytz.UTC)
+ build.dependencies = dependencies
+ transaction.commit()
d = build.getLogFromSlave(build)
return d.addCallback(got_log)
@@ -299,15 +304,30 @@
if method is None:
logger.critical("Unknown BuildStatus '%s' for builder '%s'"
% (status, self.buildqueue_record.builder.url))
- return
- d = method(librarian, slave_status, logger, send_notification)
- return d
+ return None
+ else:
+ return method(librarian, slave_status, logger, send_notification)
+
+ def _destroy_buildqueue_record(self, unused_arg):
+ """Destroy this build's `BuildQueue` record."""
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ self.buildqueue_record.destroySelf()
+ transaction.commit()
def _release_builder_and_remove_queue_item(self):
# Release the builder for another job.
d = self.buildqueue_record.builder.cleanSlave()
# Remove BuildQueue record.
- return d.addCallback(lambda x: self.buildqueue_record.destroySelf())
+ return d.addCallback(self._destroy_buildqueue_record)
+
+ def _notify_if_appropriate(self, appropriate=True, extra_info=None):
+ """If `appropriate`, call `self.notify` in a write transaction."""
+ if appropriate:
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ self.notify(extra_info=extra_info)
+ transaction.commit()
def _handleStatus_OK(self, librarian, slave_status, logger,
send_notification):
@@ -323,12 +343,15 @@
self.buildqueue_record.specific_job.build.title,
self.buildqueue_record.builder.name))
- # If this is a binary package build, discard it if its source is
- # no longer published.
+ # If this is a binary package build for a source that is no
+ # longer published, discard it.
if self.build_farm_job_type == BuildFarmJobType.PACKAGEBUILD:
build = self.buildqueue_record.specific_job.build
if not build.current_source_publication:
- build.status = BuildStatus.SUPERSEDED
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ build.status = BuildStatus.SUPERSEDED
+ transaction.commit()
return self._release_builder_and_remove_queue_item()
# Explode before collect a binary that is denied in this
@@ -377,18 +400,26 @@
# files from the slave.
if successful_copy_from_slave:
logger.info(
- "Gathered %s %d completely. Moving %s to uploader queue."
- % (self.__class__.__name__, self.id, upload_leaf))
+ "Gathered %s %d completely. "
+ "Moving %s to uploader queue.",
+ self.__class__.__name__, self.id, upload_leaf)
target_dir = os.path.join(root, "incoming")
- self.status = BuildStatus.UPLOADING
+ resulting_status = BuildStatus.UPLOADING
else:
logger.warning(
- "Copy from slave for build %s was unsuccessful.", self.id)
- self.status = BuildStatus.FAILEDTOUPLOAD
- if send_notification:
- self.notify(
- extra_info='Copy from slave was unsuccessful.')
+ "Copy from slave for build %s was unsuccessful.",
+ self.id)
target_dir = os.path.join(root, "failed")
+ resulting_status = BuildStatus.FAILEDTOUPLOAD
+
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ self.status = resulting_status
+ transaction.commit()
+
+ if not successful_copy_from_slave:
+ self._notify_if_appropriate(
+ send_notification, "Copy from slave was unsuccessful.")
if not os.path.exists(target_dir):
os.mkdir(target_dir)
@@ -396,10 +427,6 @@
# Release the builder for another job.
d = self._release_builder_and_remove_queue_item()
- # Commit so there are no race conditions with archiveuploader
- # about self.status.
- Store.of(self).commit()
-
# Move the directory used to grab the binaries into
# the incoming directory so the upload processor never
# sees half-finished uploads.
@@ -423,14 +450,15 @@
set the job status as FAILEDTOBUILD, store available info and
remove Buildqueue entry.
"""
- self.status = BuildStatus.FAILEDTOBUILD
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ self.status = BuildStatus.FAILEDTOBUILD
+ transaction.commit()
def build_info_stored(ignored):
- if send_notification:
- self.notify()
+ self._notify_if_appropriate(send_notification)
d = self.buildqueue_record.builder.cleanSlave()
- return d.addCallback(
- lambda x: self.buildqueue_record.destroySelf())
+ return d.addCallback(self._destroy_buildqueue_record)
d = self.storeBuildInfo(self, librarian, slave_status)
return d.addCallback(build_info_stored)
@@ -448,11 +476,9 @@
def build_info_stored(ignored):
logger.critical("***** %s is MANUALDEPWAIT *****"
% self.buildqueue_record.builder.name)
- if send_notification:
- self.notify()
+ self._notify_if_appropriate(send_notification)
d = self.buildqueue_record.builder.cleanSlave()
- return d.addCallback(
- lambda x: self.buildqueue_record.destroySelf())
+ return d.addCallback(self._destroy_buildqueue_record)
d = self.storeBuildInfo(self, librarian, slave_status)
return d.addCallback(build_info_stored)
@@ -468,17 +494,24 @@
self.status = BuildStatus.CHROOTWAIT
def build_info_stored(ignored):
- logger.critical("***** %s is CHROOTWAIT *****" %
- self.buildqueue_record.builder.name)
- if send_notification:
- self.notify()
+ logger.critical(
+ "***** %s is CHROOTWAIT *****",
+ self.buildqueue_record.builder.name)
+
+ self._notify_if_appropriate(send_notification)
d = self.buildqueue_record.builder.cleanSlave()
- return d.addCallback(
- lambda x: self.buildqueue_record.destroySelf())
+ return d.addCallback(self._destroy_buildqueue_record)
d = self.storeBuildInfo(self, librarian, slave_status)
return d.addCallback(build_info_stored)
+ def _reset_buildqueue_record(self, ignored_arg=None):
+ """Reset the `BuildQueue` record, in a write transaction."""
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ self.buildqueue_record.reset()
+ transaction.commit()
+
def _handleStatus_BUILDERFAIL(self, librarian, slave_status, logger,
send_notification):
"""Handle builder failures.
@@ -492,11 +525,8 @@
self.buildqueue_record.builder.failBuilder(
"Builder returned BUILDERFAIL when asked for its status")
- def build_info_stored(ignored):
- # simply reset job
- self.buildqueue_record.reset()
d = self.storeBuildInfo(self, librarian, slave_status)
- return d.addCallback(build_info_stored)
+ return d.addCallback(self._reset_buildqueue_record)
def _handleStatus_GIVENBACK(self, librarian, slave_status, logger,
send_notification):
@@ -516,7 +546,7 @@
# the next Paris Summit, infinity has some ideas about how
# to use this content. For now we just ensure it's stored.
d = self.buildqueue_record.builder.cleanSlave()
- self.buildqueue_record.reset()
+ self._reset_buildqueue_record()
return d
d = self.storeBuildInfo(self, librarian, slave_status)
=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py 2011-11-20 23:37:23 +0000
+++ lib/lp/buildmaster/tests/test_builder.py 2011-11-22 13:50:40 +0000
@@ -155,7 +155,7 @@
d = lostbuilding_builder.updateStatus(BufferLogger())
def check_slave_status(failure):
self.assertIn('abort', slave.call_log)
- # 'Fault' comes from the LostBuildingBrokenSlave, this is
+ # 'Fault' comes from the LostBuildingBrokenSlave. This is
# just testing that the value is passed through.
self.assertIsInstance(failure.value, xmlrpclib.Fault)
return d.addBoth(check_slave_status)
=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py 2011-11-20 23:37:23 +0000
+++ lib/lp/buildmaster/tests/test_manager.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the renovated slave scanner aka BuilddManager."""
@@ -12,17 +12,13 @@
assert_fails_with,
AsynchronousDeferredRunTest,
)
-
import transaction
-
from twisted.internet import (
defer,
reactor,
task,
)
-from twisted.internet.task import (
- deferLater,
- )
+from twisted.internet.task import deferLater
from twisted.python.failure import Failure
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -34,7 +30,6 @@
ANONYMOUS,
login,
)
-from lp.services.log.logger import BufferLogger
from canonical.testing.layers import (
LaunchpadScriptLayer,
LaunchpadZopelessLayer,
@@ -58,6 +53,8 @@
OkSlave,
)
from lp.registry.interfaces.distribution import IDistributionSet
+from lp.services.database.transaction_policy import DatabaseTransactionPolicy
+from lp.services.log.logger import BufferLogger
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.testing import (
TestCase,
@@ -68,7 +65,7 @@
from lp.testing.sampledata import BOB_THE_BUILDER_NAME
-class TestSlaveScannerScan(TestCase):
+class TestSlaveScannerScan(TestCaseWithFactory):
"""Tests `SlaveScanner.scan` method.
This method uses the old framework for scanning and dispatching builds.
@@ -83,6 +80,9 @@
'bob' builder.
"""
super(TestSlaveScannerScan, self).setUp()
+ self.read_only = DatabaseTransactionPolicy(read_only=True)
+ self.slave = self.useFixture(BuilddSlaveTestSetup())
+
# Creating the required chroots needed for dispatching.
test_publisher = make_publisher()
ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
@@ -90,6 +90,15 @@
test_publisher.setUpDefaultDistroSeries(hoary)
test_publisher.addFakeChroots()
+ def _enterReadOnly(self):
+ """Go into read-only transaction policy."""
+ self.read_only.__enter__()
+ self.addCleanup(self._exitReadOnly)
+
+ def _exitReadOnly(self):
+ """Leave read-only transaction policy."""
+ self.read_only.__exit__(None, None, None)
+
def _resetBuilder(self, builder):
"""Reset the given builder and its job."""
@@ -148,6 +157,7 @@
# Run 'scan' and check its result.
self.layer.txn.commit()
self.layer.switchDbUser(config.builddmaster.dbuser)
+ self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.scan)
d.addCallback(self._checkDispatch, builder)
@@ -160,11 +170,11 @@
to the asynchonous dispatcher and the builder remained active
and IDLE.
"""
- self.assertTrue(slave is None, "Unexpected slave.")
+ self.assertIs(None, slave, "Unexpected slave.")
builder = getUtility(IBuilderSet).get(builder.id)
self.assertTrue(builder.builderok)
- self.assertTrue(builder.currentjob is None)
+ self.assertIs(None, builder.currentjob)
def testNoDispatchForMissingChroots(self):
# When a required chroot is not present the `scan` method
@@ -186,6 +196,7 @@
# Run 'scan' and check its result.
self.layer.switchDbUser(config.builddmaster.dbuser)
+ self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.singleCycle)
d.addCallback(self._checkNoDispatch, builder)
@@ -211,7 +222,6 @@
def testScanRescuesJobFromBrokenBuilder(self):
# The job assigned to a broken builder is rescued.
- self.useFixture(BuilddSlaveTestSetup())
# Sampledata builder is enabled and is assigned to an active job.
builder = getUtility(IBuilderSet)[BOB_THE_BUILDER_NAME]
@@ -227,6 +237,7 @@
# Run 'scan' and check its result.
self.layer.switchDbUser(config.builddmaster.dbuser)
+ self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.scan)
d.addCallback(self._checkJobRescued, builder, job)
@@ -262,15 +273,17 @@
# Run 'scan' and check its result.
self.layer.switchDbUser(config.builddmaster.dbuser)
+ self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.scan)
d.addCallback(self._checkJobUpdated, builder, job)
return d
def test_scan_with_nothing_to_dispatch(self):
- factory = LaunchpadObjectFactory()
- builder = factory.makeBuilder()
+ builder = self.factory.makeBuilder()
builder.setSlaveForTesting(OkSlave())
+ transaction.commit()
+ self._enterReadOnly()
scanner = self._getScanner(builder_name=builder.name)
d = scanner.scan()
return d.addCallback(self._checkNoDispatch, builder)
@@ -281,6 +294,8 @@
self._resetBuilder(builder)
builder.setSlaveForTesting(OkSlave())
builder.manual = True
+ transaction.commit()
+ self._enterReadOnly()
scanner = self._getScanner()
d = scanner.scan()
d.addCallback(self._checkNoDispatch, builder)
@@ -292,6 +307,8 @@
self._resetBuilder(builder)
builder.setSlaveForTesting(OkSlave())
builder.builderok = False
+ transaction.commit()
+ self._enterReadOnly()
scanner = self._getScanner()
d = scanner.scan()
# Because the builder is not ok, we can't use _checkNoDispatch.
@@ -304,6 +321,8 @@
self._resetBuilder(builder)
builder.setSlaveForTesting(BrokenSlave())
builder.failure_count = 0
+ transaction.commit()
+ self._enterReadOnly()
scanner = self._getScanner(builder_name=builder.name)
d = scanner.scan()
return assert_fails_with(d, xmlrpclib.Fault)
=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py 2011-09-10 08:59:08 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for `IPackageBuild`."""
@@ -22,9 +22,7 @@
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
-from lp.archiveuploader.uploadprocessor import (
- parse_build_upload_leaf_name,
- )
+from lp.archiveuploader.uploadprocessor import parse_build_upload_leaf_name
from lp.buildmaster.enums import (
BuildFarmJobType,
BuildStatus,
@@ -34,12 +32,11 @@
IPackageBuildSet,
IPackageBuildSource,
)
+from lp.buildmaster.model.builder import BuilderSlave
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
from lp.buildmaster.model.packagebuild import PackageBuild
from lp.buildmaster.tests.mock_slaves import WaitingSlave
-from lp.registry.interfaces.pocket import (
- PackagePublishingPocket,
- )
+from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.testing import (
login,
login_person,
@@ -285,10 +282,7 @@
class TestHandleStatusMixin:
- """Tests for `IPackageBuild`s handleStatus method.
-
- This should be run with a Trial TestCase.
- """
+ """Tests for `IPackageBuild`s handleStatus method."""
layer = LaunchpadZopelessLayer
@@ -307,7 +301,7 @@
self.build.buildqueue_record.setDateStarted(UTC_NOW)
self.slave = WaitingSlave('BuildStatus.OK')
self.slave.valid_file_hashes.append('test_file_hash')
- builder.setSlaveForTesting(self.slave)
+ self.patch(BuilderSlave, 'makeBuilderSlave', FakeMethod(self.slave))
# We overwrite the buildmaster root to use a temp directory.
tempdir = tempfile.mkdtemp()
@@ -348,7 +342,7 @@
def got_status(ignored):
self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
self.assertResultCount(0, "failed")
- self.assertIdentical(None, self.build.buildqueue_record)
+ self.assertIs(None, self.build.buildqueue_record)
d = self.build.handleStatus('OK', None, {
'filemap': {'/tmp/myfile.py': 'test_file_hash'},
@@ -390,14 +384,10 @@
def got_status(ignored):
if expected_notification:
- self.failIf(
- len(pop_notifications()) == 0,
- "No notifications received")
+ self.assertNotEqual(
+ 0, len(pop_notifications()), "No notifications received.")
else:
- self.failIf(
- len(pop_notifications()) > 0,
- "Notifications received")
-
+ self.assertContentEqual([], pop_notifications())
d = self.build.handleStatus(status, None, {})
return d.addCallback(got_status)
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2011-11-09 11:36:44 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2011-11-22 13:50:40 +0000
@@ -13,8 +13,8 @@
from pytz import utc
from storm.locals import Store
+from testtools.deferredruntest import AsynchronousDeferredRunTest
import transaction
-from twisted.trial.unittest import TestCase as TrialTestCase
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -28,6 +28,7 @@
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.buildqueue import IBuildQueue
+from lp.buildmaster.model.builder import BuilderSlave
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
from lp.buildmaster.model.packagebuild import PackageBuild
from lp.buildmaster.tests.mock_slaves import WaitingSlave
@@ -588,14 +589,11 @@
self.assertEquals(0, len(notifications))
-class TestBuildNotifications(TrialTestCase):
+class TestBuildNotifications(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
- def setUp(self):
- super(TestBuildNotifications, self).setUp()
- from lp.testing.factory import LaunchpadObjectFactory
- self.factory = LaunchpadObjectFactory()
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=20)
def prepare_build(self, fake_successful_upload=False):
queue_record = self.factory.makeSourcePackageRecipeBuildJob()
@@ -608,7 +606,7 @@
result=True)
queue_record.builder = self.factory.makeBuilder()
slave = WaitingSlave('BuildStatus.OK')
- queue_record.builder.setSlaveForTesting(slave)
+ self.patch(BuilderSlave, 'makeBuilderSlave', FakeMethod(slave))
return build
def assertDeferredNotifyCount(self, status, build, expected_count):
@@ -633,12 +631,9 @@
return self.assertDeferredNotifyCount(
"OK", self.prepare_build(), 0)
-#XXX 2011-05-20 gmb bug=785679
-# This test has been disabled since it broke intermittently in
-# buildbot (but does not fail in isolation locally).
-## def test_handleStatus_OK_successful_upload(self):
-## return self.assertDeferredNotifyCount(
-## "OK", self.prepare_build(True), 0)
+ def test_handleStatus_OK_successful_upload(self):
+ return self.assertDeferredNotifyCount(
+ "OK", self.prepare_build(True), 0)
class MakeSPRecipeBuildMixin:
@@ -666,5 +661,5 @@
class TestHandleStatusForSPRBuild(
- MakeSPRecipeBuildMixin, TestHandleStatusMixin, TrialTestCase):
+ MakeSPRecipeBuildMixin, TestHandleStatusMixin, TestCaseWithFactory):
"""IPackageBuild.handleStatus works with SPRecipe builds."""
=== modified file 'lib/lp/services/database/transaction_policy.py'
--- lib/lp/services/database/transaction_policy.py 2011-09-29 05:49:18 +0000
+++ lib/lp/services/database/transaction_policy.py 2011-11-22 13:50:40 +0000
@@ -133,8 +133,11 @@
def _isInTransaction(self):
"""Is our store currently in a transaction?"""
pg_connection = self.store._connection._raw_connection
- status = pg_connection.get_transaction_status()
- return status != TRANSACTION_STATUS_IDLE
+ if pg_connection is None:
+ return False
+ else:
+ status = pg_connection.get_transaction_status()
+ return status != TRANSACTION_STATUS_IDLE
def _checkNoTransaction(self, error_msg):
"""Verify that no transaction is ongoing.
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py 2011-11-09 11:36:44 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test Build features."""
@@ -10,7 +10,7 @@
import pytz
from storm.store import Store
-from twisted.trial.unittest import TestCase as TrialTestCase
+from testtools.deferredruntest import AsynchronousDeferredRunTest
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -25,6 +25,7 @@
from lp.buildmaster.interfaces.builder import IBuilderSet
from lp.buildmaster.interfaces.buildqueue import IBuildQueue
from lp.buildmaster.interfaces.packagebuild import IPackageBuild
+from lp.buildmaster.model.builder import BuilderSlave
from lp.buildmaster.model.buildqueue import BuildQueue
from lp.buildmaster.tests.mock_slaves import WaitingSlave
from lp.buildmaster.tests.test_packagebuild import (
@@ -53,6 +54,7 @@
logout,
TestCaseWithFactory,
)
+from lp.testing.fakemethod import FakeMethod
class TestBinaryPackageBuild(TestCaseWithFactory):
@@ -522,7 +524,9 @@
self.build = gedit_src_hist.createMissingBuilds()[0]
self.builder = self.factory.makeBuilder()
- self.builder.setSlaveForTesting(WaitingSlave('BuildStatus.OK'))
+ self.patch(
+ BuilderSlave, 'makeBuilderSlave',
+ FakeMethod(WaitingSlave('BuildStatus.OK')))
self.build.buildqueue_record.markAsBuilding(self.builder)
def testDependencies(self):
@@ -568,9 +572,12 @@
class TestHandleStatusForBinaryPackageBuild(
- MakeBinaryPackageBuildMixin, TestHandleStatusMixin, TrialTestCase):
+ MakeBinaryPackageBuildMixin, TestHandleStatusMixin, TestCaseWithFactory):
"""IPackageBuild.handleStatus works with binary builds."""
+ layer = LaunchpadZopelessLayer
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=20)
+
class TestBinaryPackageBuildWebservice(TestCaseWithFactory):
"""Test cases for BinaryPackageBuild on the webservice.
=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py 2011-10-19 08:00:29 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py 2011-11-22 13:50:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""An `IBuildFarmJobBehavior` for `TranslationTemplatesBuildJob`.
@@ -13,9 +13,10 @@
import datetime
import os
+import tempfile
+
import pytz
-import tempfile
-
+import transaction
from twisted.internet import defer
from zope.component import getUtility
from zope.interface import implements
@@ -28,6 +29,7 @@
)
from lp.buildmaster.model.buildfarmjobbehavior import BuildFarmJobBehaviorBase
from lp.registry.interfaces.productseries import IProductSeriesSet
+from lp.services.database.transaction_policy import DatabaseTransactionPolicy
from lp.translations.interfaces.translationimportqueue import (
ITranslationImportQueue,
)
@@ -132,13 +134,16 @@
def storeBuildInfo(build, queue_item, build_status):
"""See `IPackageBuild`."""
def got_log(lfa_id):
- build.build.log = lfa_id
- build.build.builder = queue_item.builder
- build.build.date_started = queue_item.date_started
- # XXX cprov 20060615 bug=120584: Currently buildduration includes
- # the scanner latency, it should really be asking the slave for
- # the duration spent building locally.
- build.build.date_finished = datetime.datetime.now(pytz.UTC)
+ transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ build.build.log = lfa_id
+ build.build.builder = queue_item.builder
+ build.build.date_started = queue_item.date_started
+ # XXX cprov 20060615 bug=120584: Currently buildduration
+ # includes the scanner latency. It should really be
+ # asking the slave for the duration spent building locally.
+ build.build.date_finished = datetime.datetime.now(pytz.UTC)
+ transaction.commit()
d = build.getLogFromSlave(build, queue_item)
return d.addCallback(got_log)
Follow ups
