launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15839
[Merge] lp:~wgrant/launchpad/direct-action into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/direct-action into lp:launchpad.
Commit message:
BuilderInteractor.transferSlaveFileToLibrarian is now on BuildFarmJobBehaviorBase.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/direct-action/+merge/182818
Builder.transferSlaveFileToLibrarian recently moved to BuilderInteractor. But BuilderInteractor doesn't do any other DB interaction, so let's push it down to BuildFarmJobBehaviorBase, its sole callsite.
updateBuild/handleStatus also took an ILibrarianClient, which is stupid in itself, but then it was used by nothing whatsoever. Bai.
--
https://code.launchpad.net/~wgrant/launchpad/direct-action/+merge/182818
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/direct-action into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py 2013-08-29 04:12:56 +0000
+++ lib/lp/buildmaster/interactor.py 2013-08-29 05:08:28 +0000
@@ -7,11 +7,8 @@
'BuilderInteractor',
]
-import gzip
import logging
-import os
import socket
-import tempfile
from urlparse import urlparse
import xmlrpclib
@@ -19,7 +16,6 @@
from twisted.internet import defer
from twisted.web import xmlrpc
from twisted.web.client import downloadPage
-from zope.component import getUtility
from zope.security.proxy import (
isinstance as zope_isinstance,
removeSecurityProxy,
@@ -37,11 +33,7 @@
)
from lp.services import encoding
from lp.services.config import config
-from lp.services.helpers import filenameToContentType
from lp.services.job.interfaces.job import JobStatus
-from lp.services.librarian.interfaces import ILibraryFileAliasSet
-from lp.services.librarian.interfaces.client import ILibrarianClient
-from lp.services.librarian.utils import copy_and_close
from lp.services.twistedsupport import cancel_on_timeout
from lp.services.twistedsupport.processmonitor import ProcessWithTimeout
from lp.services.webapp import urlappend
@@ -537,20 +529,6 @@
d.addCallback(resume_done)
return d
- def _dispatchBuildCandidate(self, candidate):
- """Dispatch the pending job to the associated buildd slave.
-
- This method can only be executed in the builddmaster machine, since
- it will actually issues the XMLRPC call to the buildd-slave.
-
- :param candidate: The job to dispatch.
- """
- logger = self._getSlaveScannerLogger()
- # Using maybeDeferred ensures that any exceptions are also
- # wrapped up and caught later.
- d = defer.maybeDeferred(self._startBuild, candidate, logger)
- return d
-
def resetOrFail(self, logger, exception):
"""Handle "confirmed" build slave failures.
@@ -606,7 +584,9 @@
logger.debug("No build candidates available for builder.")
return defer.succeed(None)
- d = self._dispatchBuildCandidate(candidate)
+ # Using maybeDeferred ensures that any exceptions are also
+ # wrapped up and caught later.
+ d = defer.maybeDeferred(self._startBuild, candidate, logger)
return d.addCallback(lambda ignored: candidate)
def updateBuild(self, queueItem):
@@ -737,56 +717,7 @@
self._current_build_behavior.updateSlaveStatus(
status_sentence, status_dict)
d = self._current_build_behavior.handleStatus(
- self.extractBuildStatus(status_dict),
- getUtility(ILibrarianClient), status_dict)
- return d
-
- def transferSlaveFileToLibrarian(self, file_sha1, filename, private):
- """Transfer a file from the slave to the librarian.
-
- :param file_sha1: The file's sha1, which is how the file is addressed
- in the slave XMLRPC protocol. Specially, the file_sha1 'buildlog'
- will cause the build log to be retrieved and gzipped.
- :param filename: The name of the file to be given to the librarian
- file alias.
- :param private: True if the build is for a private archive.
- :return: A Deferred that calls back with a librarian file alias.
- """
- out_file_fd, out_file_name = tempfile.mkstemp(suffix=".buildlog")
- out_file = os.fdopen(out_file_fd, "r+")
-
- def got_file(ignored, filename, out_file, out_file_name):
- try:
- # If the requested file is the 'buildlog' compress it
- # using gzip before storing in Librarian.
- if file_sha1 == 'buildlog':
- out_file = open(out_file_name)
- filename += '.gz'
- out_file_name += '.gz'
- gz_file = gzip.GzipFile(out_file_name, mode='wb')
- copy_and_close(out_file, gz_file)
- os.remove(out_file_name.replace('.gz', ''))
-
- # Reopen the file, seek to its end position, count and seek
- # to beginning, ready for adding to the Librarian.
- out_file = open(out_file_name)
- out_file.seek(0, 2)
- bytes_written = out_file.tell()
- out_file.seek(0)
-
- library_file = getUtility(ILibraryFileAliasSet).create(
- filename, bytes_written, out_file,
- contentType=filenameToContentType(filename),
- restricted=private)
- finally:
- # Remove the temporary file. getFile() closes the file
- # object.
- os.remove(out_file_name)
-
- return library_file.id
-
- d = self.slave.getFile(file_sha1, out_file)
- d.addCallback(got_file, filename, out_file, out_file_name)
+ self.extractBuildStatus(status_dict), status_dict)
return d
def _getSlaveScannerLogger(self):
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2013-08-29 04:28:50 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2013-08-29 05:08:28 +0000
@@ -56,11 +56,10 @@
values added to it.
"""
- def handleStatus(status, librarian, status_dict):
+ def handleStatus(status, status_dict):
"""Update the build from a WAITING slave result.
:param status: The tail of the BuildStatus (eg. OK or PACKAGEFAIL).
- :param librarian: An `ILibrarianClient` to use for file uploads.
:param status_dict: Slave status dict from
`BuilderInteractor.slaveStatus` and `updateSlaveStatus`.
"""
=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py 2013-08-29 04:28:06 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py 2013-08-29 05:08:28 +0000
@@ -10,11 +10,15 @@
]
import datetime
+import gzip
import logging
+import os
import os.path
+import tempfile
import transaction
from twisted.internet import defer
+from zope.component import getUtility
from lp.buildmaster.enums import (
BuildFarmJobType,
@@ -22,6 +26,9 @@
)
from lp.buildmaster.interfaces.builder import BuildSlaveFailure
from lp.services.config import config
+from lp.services.helpers import filenameToContentType
+from lp.services.librarian.interfaces import ILibraryFileAliasSet
+from lp.services.librarian.utils import copy_and_close
SLAVE_LOG_FILENAME = 'buildlog'
@@ -71,9 +78,57 @@
timestamp = now.strftime("%Y%m%d-%H%M%S")
return '%s-%s' % (timestamp, build_cookie)
+ def transferSlaveFileToLibrarian(self, file_sha1, filename, private):
+ """Transfer a file from the slave to the librarian.
+
+ :param file_sha1: The file's sha1, which is how the file is addressed
+ in the slave XMLRPC protocol. Specially, the file_sha1 'buildlog'
+ will cause the build log to be retrieved and gzipped.
+ :param filename: The name of the file to be given to the librarian
+ file alias.
+ :param private: True if the build is for a private archive.
+ :return: A Deferred that calls back with a librarian file alias.
+ """
+ out_file_fd, out_file_name = tempfile.mkstemp(suffix=".buildlog")
+ out_file = os.fdopen(out_file_fd, "r+")
+
+ def got_file(ignored, filename, out_file, out_file_name):
+ try:
+ # If the requested file is the 'buildlog' compress it
+ # using gzip before storing in Librarian.
+ if file_sha1 == 'buildlog':
+ out_file = open(out_file_name)
+ filename += '.gz'
+ out_file_name += '.gz'
+ gz_file = gzip.GzipFile(out_file_name, mode='wb')
+ copy_and_close(out_file, gz_file)
+ os.remove(out_file_name.replace('.gz', ''))
+
+ # Reopen the file, seek to its end position, count and seek
+ # to beginning, ready for adding to the Librarian.
+ out_file = open(out_file_name)
+ out_file.seek(0, 2)
+ bytes_written = out_file.tell()
+ out_file.seek(0)
+
+ library_file = getUtility(ILibraryFileAliasSet).create(
+ filename, bytes_written, out_file,
+ contentType=filenameToContentType(filename),
+ restricted=private)
+ finally:
+ # Remove the temporary file. getFile() closes the file
+ # object.
+ os.remove(out_file_name)
+
+ return library_file.id
+
+ d = self._interactor.slave.getFile(file_sha1, out_file)
+ d.addCallback(got_file, filename, out_file, out_file_name)
+ return d
+
def getLogFromSlave(self, queue_item):
"""See `IPackageBuild`."""
- d = self._interactor.transferSlaveFileToLibrarian(
+ d = self.transferSlaveFileToLibrarian(
SLAVE_LOG_FILENAME, queue_item.getLogFileName(),
self.build.is_private)
return d
@@ -92,11 +147,11 @@
# in this list.
ALLOWED_STATUS_NOTIFICATIONS = ['OK', 'PACKAGEFAIL', 'CHROOTFAIL']
- def handleStatus(self, status, librarian, slave_status):
+ def handleStatus(self, status, slave_status):
"""See `IBuildFarmJobBehavior`."""
from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
- send_notification = status in self.ALLOWED_STATUS_NOTIFICATIONS
+ notify = status in self.ALLOWED_STATUS_NOTIFICATIONS
method = getattr(self, '_handleStatus_' + status, None)
if method is None:
logger.critical(
@@ -108,12 +163,11 @@
% (status, self.getBuildCookie(),
self.build.buildqueue_record.specific_job.build.title,
self.build.buildqueue_record.builder.name))
- d = method(librarian, slave_status, logger, send_notification)
+ d = method(slave_status, logger, notify)
return d
@defer.inlineCallbacks
- def _handleStatus_OK(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_OK(self, slave_status, logger, notify):
"""Handle a package that built successfully.
Once built successfully, we pull the files, store them in a
@@ -196,7 +250,7 @@
else:
logger.warning(
"Copy from slave for build %s was unsuccessful.", build.id)
- if send_notification:
+ if notify:
build.notify(
extra_info='Copy from slave was unsuccessful.')
target_dir = os.path.join(root, "failed")
@@ -214,8 +268,7 @@
os.rename(grab_dir, os.path.join(target_dir, upload_leaf))
@defer.inlineCallbacks
- def _handleStatus_generic_failure(self, status, librarian, slave_status,
- logger, send_notification):
+ def _handleStatus_generic_fail(self, status, slave_status, logger, notify):
"""Handle a generic build failure.
The build, not the builder, has failed. Set its status, store
@@ -228,35 +281,28 @@
slave_status=slave_status)
transaction.commit()
yield self.storeLogFromSlave()
- if send_notification:
+ if notify:
self.build.notify()
yield self._interactor.cleanSlave()
self.build.buildqueue_record.destroySelf()
transaction.commit()
- def _handleStatus_PACKAGEFAIL(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_PACKAGEFAIL(self, slave_status, logger, notify):
"""Handle a package that had failed to build."""
- return self._handleStatus_generic_failure(
- BuildStatus.FAILEDTOBUILD, librarian, slave_status, logger,
- send_notification)
+ return self._handleStatus_generic_fail(
+ BuildStatus.FAILEDTOBUILD, slave_status, logger, notify)
- def _handleStatus_DEPFAIL(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_DEPFAIL(self, slave_status, logger, notify):
"""Handle a package that had missing dependencies."""
- return self._handleStatus_generic_failure(
- BuildStatus.MANUALDEPWAIT, librarian, slave_status, logger,
- send_notification)
+ return self._handleStatus_generic_fail(
+ BuildStatus.MANUALDEPWAIT, slave_status, logger, notify)
- def _handleStatus_CHROOTFAIL(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_CHROOTFAIL(self, slave_status, logger, notify):
"""Handle a package that had failed when unpacking the CHROOT."""
- return self._handleStatus_generic_failure(
- BuildStatus.CHROOTWAIT, librarian, slave_status, logger,
- send_notification)
+ return self._handleStatus_generic_fail(
+ BuildStatus.CHROOTWAIT, slave_status, logger, notify)
- def _handleStatus_BUILDERFAIL(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_BUILDERFAIL(self, slave_status, logger, notify):
"""Handle builder failures.
Fail the builder, and reset the job.
@@ -267,8 +313,7 @@
transaction.commit()
@defer.inlineCallbacks
- def _handleStatus_ABORTED(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_ABORTED(self, slave_status, logger, notify):
"""Handle aborted builds.
If the build was explicitly cancelled, then mark it as such.
@@ -291,8 +336,7 @@
transaction.commit()
@defer.inlineCallbacks
- def _handleStatus_GIVENBACK(self, librarian, slave_status, logger,
- send_notification):
+ def _handleStatus_GIVENBACK(self, slave_status, logger, notify):
"""Handle automatic retry requested by builder.
GIVENBACK pseudo-state represents a request for automatic retry
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-08-29 03:14:22 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-08-29 05:08:28 +0000
@@ -206,7 +206,7 @@
self.assertEqual(BuildStatus.UPLOADING, self.build.status)
self.assertResultCount(1, "incoming")
- d = self.behavior.handleStatus('OK', None, {
+ d = self.behavior.handleStatus('OK', {
'filemap': {'myfile.py': 'test_file_hash'},
})
return d.addCallback(got_status)
@@ -219,7 +219,7 @@
self.assertResultCount(0, "failed")
self.assertIdentical(None, self.build.buildqueue_record)
- d = self.behavior.handleStatus('OK', None, {
+ d = self.behavior.handleStatus('OK', {
'filemap': {'/tmp/myfile.py': 'test_file_hash'},
})
return d.addCallback(got_status)
@@ -231,7 +231,7 @@
self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
self.assertResultCount(0, "failed")
- d = self.behavior.handleStatus('OK', None, {
+ d = self.behavior.handleStatus('OK', {
'filemap': {'../myfile.py': 'test_file_hash'},
})
return d.addCallback(got_status)
@@ -239,7 +239,7 @@
def test_handleStatus_OK_sets_build_log(self):
# The build log is set during handleStatus.
self.assertEqual(None, self.build.log)
- d = self.behavior.handleStatus('OK', None, {
+ d = self.behavior.handleStatus('OK', {
'filemap': {'myfile.py': 'test_file_hash'},
})
@@ -265,7 +265,7 @@
len(pop_notifications()) > 0,
"Notifications received")
- d = self.behavior.handleStatus(status, None, {})
+ d = self.behavior.handleStatus(status, {})
return d.addCallback(got_status)
def test_handleStatus_DEPFAIL_notifies(self):
@@ -285,7 +285,7 @@
0, len(pop_notifications()), "Notifications received")
self.assertEqual(BuildStatus.CANCELLED, self.build.status)
- d = self.behavior.handleStatus("ABORTED", None, {})
+ d = self.behavior.handleStatus("ABORTED", {})
return d.addCallback(got_status)
def test_handleStatus_ABORTED_recovers_building(self):
@@ -299,13 +299,13 @@
self.assertEqual(1, self.builder.failure_count)
self.assertIn("resume", self.slave.call_log)
- d = self.behavior.handleStatus("ABORTED", None, {})
+ d = self.behavior.handleStatus("ABORTED", {})
return d.addCallback(got_status)
def test_date_finished_set(self):
# The date finished is updated during handleStatus_OK.
self.assertEqual(None, self.build.date_finished)
- d = self.behavior.handleStatus('OK', None, {
+ d = self.behavior.handleStatus('OK', {
'filemap': {'myfile.py': 'test_file_hash'},
})
=== modified file 'lib/lp/code/model/tests/test_recipebuilder.py'
--- lib/lp/code/model/tests/test_recipebuilder.py 2013-08-27 10:23:43 +0000
+++ lib/lp/code/model/tests/test_recipebuilder.py 2013-08-29 05:08:28 +0000
@@ -375,7 +375,7 @@
return removeSecurityProxy(interactor._current_build_behavior)
def assertDeferredNotifyCount(self, status, behavior, expected_count):
- d = behavior.handleStatus(status, None, {'filemap': {}})
+ d = behavior.handleStatus(status, {'filemap': {}})
def cb(result):
self.assertEqual(expected_count, len(pop_notifications()))
=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py 2013-08-28 09:21:19 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py 2013-08-29 05:08:28 +0000
@@ -119,7 +119,7 @@
status['filemap'] = raw_slave_status[3]
@defer.inlineCallbacks
- def handleStatus(self, status, librarian, slave_status, queue_item=None):
+ def handleStatus(self, status, slave_status, queue_item=None):
"""Deal with a finished build job.
Retrieves tarball and logs from the slave, then cleans up the
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2013-08-29 03:45:03 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2013-08-29 05:08:28 +0000
@@ -204,7 +204,7 @@
return (
behavior.handleStatus(
behavior._interactor.extractBuildStatus(slave_status),
- None, slave_status, queue_item=queue_item),
+ slave_status, queue_item=queue_item),
slave_call_log)
def build_updated(ignored):
@@ -246,8 +246,8 @@
behavior.updateSlaveStatus(raw_status, slave_status)
self.assertNotIn('filemap', slave_status)
return behavior.handleStatus(
- behavior._interactor.extractBuildStatus(slave_status),
- None, slave_status, queue_item=queue_item),
+ behavior._interactor.extractBuildStatus(slave_status),
+ slave_status, queue_item=queue_item),
def build_updated(ignored):
self.assertEqual(BuildStatus.FAILEDTOBUILD, behavior.build.status)
@@ -287,8 +287,8 @@
behavior.updateSlaveStatus(raw_status, slave_status)
self.assertFalse('filemap' in slave_status)
return behavior.handleStatus(
- behavior._interactor.extractBuildStatus(slave_status),
- None, slave_status, queue_item=queue_item),
+ behavior._interactor.extractBuildStatus(slave_status),
+ slave_status, queue_item=queue_item),
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behavior.build.status)
@@ -330,8 +330,8 @@
}
behavior.updateSlaveStatus(status, slave_status)
return behavior.handleStatus(
- behavior._interactor.extractBuildStatus(slave_status),
- None, slave_status, queue_item=queue_item),
+ behavior._interactor.extractBuildStatus(slave_status),
+ slave_status, queue_item=queue_item),
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behavior.build.status)
Follow ups
