launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/more-buildd-manager-logging into lp:launchpad.

Commit message:
Add a bit more logging to buildd-manager when fetching files.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/more-buildd-manager-logging/+merge/369031

Logging something more specific than Twisted's "Stopping factory" entries makes it easier to see which files have been grabbed and which are still in progress.

I suspect this won't actually help us debug current odd network slowness very much, but it will make me feel better when reading logs. :-)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/more-buildd-manager-logging into lp:launchpad.

=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py	2018-02-08 09:39:46 +0000
+++ lib/lp/buildmaster/interactor.py	2019-06-19 10:06:23 +0000
@@ -238,13 +238,14 @@
         """Get the URL for a file on the builder with a given SHA-1."""
         return urlappend(self._file_cache_url, sha1).encode('utf8')
 
-    def getFile(self, sha_sum, file_to_write):
+    def getFile(self, sha_sum, file_to_write, logger=None):
         """Fetch a file from the builder.
 
         :param sha_sum: The sha of the file (which is also its name on the
             builder)
         :param file_to_write: A file name or file-like object to write
             the file to
+        :param logger: An optional logger.
         :return: A Deferred that calls back when the download is done, or
             errback with the error string.
         """
@@ -256,19 +257,31 @@
             response.deliverBody(FileWritingProtocol(finished, file_to_write))
             return finished
 
+        def log_success(result):
+            logger.info("Grabbed %s" % file_url)
+            return result
+
+        def log_failure(failure):
+            logger.info("Failed to grab %s: %s\n%s" % (
+                file_url, failure.getErrorMessage(), failure.getTraceback()))
+            return failure
+
         d.addCallback(got_response)
+        if logger is not None:
+            d.addCallbacks(log_success, log_failure)
         return d
 
-    def getFiles(self, files):
+    def getFiles(self, files, logger=None):
         """Fetch many files from the builder.
 
         :param files: A sequence of pairs of the builder file name to
             retrieve and the file name or file object to write the file to.
+        :param logger: An optional logger.
 
         :return: A DeferredList that calls back when the download is done.
         """
         dl = defer.gatherResults([
-            self.getFile(builder_file, local_file)
+            self.getFile(builder_file, local_file, logger=logger)
             for builder_file, local_file in files])
         return dl
 

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/model/buildfarmjobbehaviour.py	2019-03-13 17:29:11 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehaviour.py	2019-06-19 10:06:23 +0000
@@ -349,7 +349,7 @@
                 raise BuildDaemonError(
                     "Build returned a file named %r." % filename)
             filenames_to_download.append((sha1, out_file_name))
-        yield self._slave.getFiles(filenames_to_download)
+        yield self._slave.getFiles(filenames_to_download, logger=logger)
 
         transaction.commit()