Launchpad itself

Merge lp:~thumper/launchpad/bugjam-619555-request-builds-logging into lp:launchpad

  1. bugjam-619555-request-builds-logging
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 12116
Proposed branch: lp:~thumper/launchpad/bugjam-619555-request-builds-logging
Merge into: lp:launchpad
Diff against target: 115 lines (+31/-5)
6 files modified
cronscripts/request_daily_builds.py (+1/-1)
lib/canonical/launchpad/scripts/logger.py (+4/-0)
lib/lp/code/interfaces/sourcepackagerecipebuild.py (+5/-2)
lib/lp/code/model/sourcepackagerecipebuild.py (+5/-1)
lib/lp/code/model/tests/test_sourcepackagerecipebuild.py (+13/-0)
lib/lp/testing/logger.py (+3/-1)
To merge this branch: bzr merge lp:~thumper/launchpad/bugjam-619555-request-builds-logging
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Benji York (community) code* Approve
Review via email: mp+43731@code.launchpad.net

Commit message

[r=benji,edwin-grubbs][ui=none][bug=619555] Log extra detail in the request_daily_builds cron script.

Description of the change

LOSAs have requested that we add extra debugging to record which recipes are having their daily builds requested. This is a pretty trivial change.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Nice, straight-forward branch.

The only thing I can suggest is that the conditional in TestLogger.log() is unnecessary. If args is an empty tuple, then "msg %= args" will leave msg as it was.

review: Approve (code*)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> Nice, straight-forward branch.
>
> The only thing I can suggest is that the conditional in TestLogger.log() is
> unnecessary. If args is an empty tuple, then "msg %= args" will leave msg as
> it was.

The MockLogger has a comment saying that they check whether args is empty, so that msg will not blow up if it contains formatting strings. It looks like logging.LogRecord.getMessage() has the same type of logic, so it would be worthwhile to keep.

I think that TestLogger should either subclass from FakeLogger, or TestLogger should be removed, and BufferLogger should be used in its place. We're beginning to get as many logger classes as Linux has text editors.

review: Approve (code)
Revision history for this message
Tim Penhey (thumper) wrote :

Updated to use the BufferLogger.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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-20 01:48:17 +0000
@@ -34,7 +34,7 @@
34 def main(self):34 def main(self):
35 globalErrorUtility.configure(self.name)35 globalErrorUtility.configure(self.name)
36 source = getUtility(ISourcePackageRecipeBuildSource)36 source = getUtility(ISourcePackageRecipeBuildSource)
37 builds = source.makeDailyBuilds()37 builds = source.makeDailyBuilds(self.logger)
38 self.logger.info('Requested %d daily builds.' % len(builds))38 self.logger.info('Requested %d daily builds.' % len(builds))
39 transaction.commit()39 transaction.commit()
4040
4141
=== modified file 'lib/canonical/launchpad/scripts/logger.py'
--- lib/canonical/launchpad/scripts/logger.py 2010-10-22 10:23:17 +0000
+++ lib/canonical/launchpad/scripts/logger.py 2010-12-20 01:48:17 +0000
@@ -143,6 +143,10 @@
143 for line in thing.splitlines():143 for line in thing.splitlines():
144 self.log(line)144 self.log(line)
145145
146 def getLogOutput(self):
147 self.buffer.seek(0)
148 return self.buffer.readlines()
149
146150
147class OopsHandler(logging.Handler):151class OopsHandler(logging.Handler):
148 """Handler to log to the OOPS system."""152 """Handler to log to the OOPS system."""
149153
=== 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-20 01:48:17 +0000
@@ -98,8 +98,11 @@
98 :return: `ISourcePackageRecipeBuild`.98 :return: `ISourcePackageRecipeBuild`.
99 """99 """
100100
101 def makeDailyBuilds():101 def makeDailyBuilds(logger=None):
102 """Create and return builds for stale ISourcePackageRecipes."""102 """Create and return builds for stale ISourcePackageRecipes.
103
104 :param logger: An optional logger to write debug info to.
105 """
103106
104 def getById(build_id):107 def getById(build_id):
105 """Return the `ISourcePackageRecipeBuild` for the given build id.108 """Return the `ISourcePackageRecipeBuild` for the given build id.
106109
=== 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-20 01:48:17 +0000
@@ -182,7 +182,7 @@
182 return spbuild182 return spbuild
183183
184 @staticmethod184 @staticmethod
185 def makeDailyBuilds():185 def makeDailyBuilds(logger=None):
186 from lp.code.model.sourcepackagerecipe import SourcePackageRecipe186 from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
187 recipes = SourcePackageRecipe.findStaleDailyBuilds()187 recipes = SourcePackageRecipe.findStaleDailyBuilds()
188 builds = []188 builds = []
@@ -200,6 +200,10 @@
200 info = sys.exc_info()200 info = sys.exc_info()
201 errorlog.globalErrorUtility.raising(info)201 errorlog.globalErrorUtility.raising(info)
202 else:202 else:
203 if logger is not None:
204 logger.debug(
205 'Build for %s/%s requested',
206 recipe.owner.name, recipe.name)
203 builds.append(build)207 builds.append(build)
204 recipe.is_stale = False208 recipe.is_stale = False
205 return builds209 return builds
206210
=== 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-20 01:48:17 +0000
@@ -17,6 +17,7 @@
17from twisted.trial.unittest import TestCase as TrialTestCase17from twisted.trial.unittest import TestCase as TrialTestCase
1818
19from canonical.launchpad.interfaces.lpstorm import IStore19from canonical.launchpad.interfaces.lpstorm import IStore
20from canonical.launchpad.scripts.logger import BufferLogger
20from canonical.launchpad.webapp.authorization import check_permission21from canonical.launchpad.webapp.authorization import check_permission
21from canonical.launchpad.webapp.testing import verifyObject22from canonical.launchpad.webapp.testing import verifyObject
22from canonical.testing.layers import (23from canonical.testing.layers import (
@@ -235,6 +236,18 @@
235 self.assertEqual(recipe, build.recipe)236 self.assertEqual(recipe, build.recipe)
236 self.assertEqual(list(recipe.distroseries), [build.distroseries])237 self.assertEqual(list(recipe.distroseries), [build.distroseries])
237238
239 def test_makeDailyBuilds_logs_builds(self):
240 # If a logger is passed into the makeDailyBuilds method, each recipe
241 # that a build is requested for gets logged.
242 owner = self.factory.makePerson(name='eric')
243 self.factory.makeSourcePackageRecipe(
244 owner=owner, name=u'funky-recipe', build_daily=True)
245 logger = BufferLogger()
246 SourcePackageRecipeBuild.makeDailyBuilds(logger)
247 self.assertEqual(
248 ['DEBUG: Build for eric/funky-recipe requested\n'],
249 logger.getLogOutput())
250
238 def test_makeDailyBuilds_clears_is_stale(self):251 def test_makeDailyBuilds_clears_is_stale(self):
239 recipe = self.factory.makeSourcePackageRecipe(252 recipe = self.factory.makeSourcePackageRecipe(
240 build_daily=True, is_stale=True)253 build_daily=True, is_stale=True)
241254
=== 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-20 01:48:17 +0000
@@ -4,7 +4,9 @@
4"""Frequently-used logging utilities for test suite."""4"""Frequently-used logging utilities for test suite."""
55
6__metaclass__ = type6__metaclass__ = type
7__all__ = ['MockLogger']7__all__ = [
8 'MockLogger',
9 ]
810
9import logging11import logging
10import sys12import sys