launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/bugjam-619555-request-builds-logging into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #619555 cronscripts/request_daily_builds.py is not verbose enough on default logging
  https://bugs.launchpad.net/bugs/619555


LOSAs have requested that we add extra debugging to record which recipes are having their daily builds requested.  This is a pretty trivial change.
-- 
https://code.launchpad.net/~thumper/launchpad/bugjam-619555-request-builds-logging/+merge/43731
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/bugjam-619555-request-builds-logging into lp:launchpad.

=== modified file 'cronscripts/request_daily_builds.py'
--- cronscripts/request_daily_builds.py	2010-06-12 06:32:01 +0000
+++ cronscripts/request_daily_builds.py	2010-12-15 03:01:18 +0000
@@ -34,7 +34,7 @@
     def main(self):
         globalErrorUtility.configure(self.name)
         source = getUtility(ISourcePackageRecipeBuildSource)
-        builds = source.makeDailyBuilds()
+        builds = source.makeDailyBuilds(self.logger)
         self.logger.info('Requested %d daily builds.' % len(builds))
         transaction.commit()
 

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipebuild.py'
--- lib/lp/code/interfaces/sourcepackagerecipebuild.py	2010-11-11 20:56:21 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipebuild.py	2010-12-15 03:01:18 +0000
@@ -98,8 +98,11 @@
         :return: `ISourcePackageRecipeBuild`.
         """
 
-    def makeDailyBuilds():
-        """Create and return builds for stale ISourcePackageRecipes."""
+    def makeDailyBuilds(logger=None):
+        """Create and return builds for stale ISourcePackageRecipes.
+
+        :param logger: An optional logger to write debug info to.
+        """
 
     def getById(build_id):
         """Return the `ISourcePackageRecipeBuild` for the given build id.

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-11-30 12:23:01 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-12-15 03:01:18 +0000
@@ -182,7 +182,7 @@
         return spbuild
 
     @staticmethod
-    def makeDailyBuilds():
+    def makeDailyBuilds(logger=None):
         from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
         recipes = SourcePackageRecipe.findStaleDailyBuilds()
         builds = []
@@ -200,6 +200,10 @@
                     info = sys.exc_info()
                     errorlog.globalErrorUtility.raising(info)
                 else:
+                    if logger is not None:
+                        logger.debug(
+                            'Build for %s/%s requested',
+                            recipe.owner.name, recipe.name)
                     builds.append(build)
             recipe.is_stale = False
         return builds

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-11-19 12:22:15 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-12-15 03:01:18 +0000
@@ -53,6 +53,7 @@
     TestCaseWithFactory,
     )
 from lp.testing.fakemethod import FakeMethod
+from lp.testing.logger import TestLogger
 from lp.testing.mail_helpers import pop_notifications
 
 
@@ -235,6 +236,18 @@
         self.assertEqual(recipe, build.recipe)
         self.assertEqual(list(recipe.distroseries), [build.distroseries])
 
+    def test_makeDailyBuilds_logs_builds(self):
+        # If a logger is passed into the makeDailyBuilds method, each recipe
+        # that a build is requested for gets logged.
+        owner = self.factory.makePerson(name='eric')
+        self.factory.makeSourcePackageRecipe(
+            owner=owner, name=u'funky-recipe', build_daily=True)
+        logger = TestLogger()
+        SourcePackageRecipeBuild.makeDailyBuilds(logger)
+        self.assertEqual(
+            ['DEBUG Build for eric/funky-recipe requested'],
+            logger.output)
+
     def test_makeDailyBuilds_clears_is_stale(self):
         recipe = self.factory.makeSourcePackageRecipe(
             build_daily=True, is_stale=True)

=== modified file 'lib/lp/testing/logger.py'
--- lib/lp/testing/logger.py	2010-07-12 13:06:41 +0000
+++ lib/lp/testing/logger.py	2010-12-15 03:01:18 +0000
@@ -4,7 +4,10 @@
 """Frequently-used logging utilities for test suite."""
 
 __metaclass__ = type
-__all__ = ['MockLogger']
+__all__ = [
+    'MockLogger',
+    'TestLogger',
+    ]
 
 import logging
 import sys
@@ -63,3 +66,22 @@
 
     def exception(self, *args):
         self.log(*args, **{'exc_info': True})
+
+
+class TestLogger:
+    """Imitates a logger, but records output."""
+
+    def __init__(self, outfile=None):
+        self.output = []
+
+    def log(self, prefix, msg, *args):
+        # The standard logger takes a template string as the first
+        # argument, but we must only attempt to use it as one if we have
+        # arguments. Otherwise logging of messages with string formatting
+        # sequences will die.
+        if len(args) > 0:
+            msg %= args
+        self.output.append('%s %s' % (prefix, msg))
+
+    def debug(self, *args):
+        self.log('DEBUG', *args)