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
1=== modified file 'cronscripts/request_daily_builds.py'
2--- cronscripts/request_daily_builds.py 2010-06-12 06:32:01 +0000
3+++ cronscripts/request_daily_builds.py 2010-12-20 01:48:17 +0000
4@@ -34,7 +34,7 @@
5 def main(self):
6 globalErrorUtility.configure(self.name)
7 source = getUtility(ISourcePackageRecipeBuildSource)
8- builds = source.makeDailyBuilds()
9+ builds = source.makeDailyBuilds(self.logger)
10 self.logger.info('Requested %d daily builds.' % len(builds))
11 transaction.commit()
12
13
14=== modified file 'lib/canonical/launchpad/scripts/logger.py'
15--- lib/canonical/launchpad/scripts/logger.py 2010-10-22 10:23:17 +0000
16+++ lib/canonical/launchpad/scripts/logger.py 2010-12-20 01:48:17 +0000
17@@ -143,6 +143,10 @@
18 for line in thing.splitlines():
19 self.log(line)
20
21+ def getLogOutput(self):
22+ self.buffer.seek(0)
23+ return self.buffer.readlines()
24+
25
26 class OopsHandler(logging.Handler):
27 """Handler to log to the OOPS system."""
28
29=== modified file 'lib/lp/code/interfaces/sourcepackagerecipebuild.py'
30--- lib/lp/code/interfaces/sourcepackagerecipebuild.py 2010-11-11 20:56:21 +0000
31+++ lib/lp/code/interfaces/sourcepackagerecipebuild.py 2010-12-20 01:48:17 +0000
32@@ -98,8 +98,11 @@
33 :return: `ISourcePackageRecipeBuild`.
34 """
35
36- def makeDailyBuilds():
37- """Create and return builds for stale ISourcePackageRecipes."""
38+ def makeDailyBuilds(logger=None):
39+ """Create and return builds for stale ISourcePackageRecipes.
40+
41+ :param logger: An optional logger to write debug info to.
42+ """
43
44 def getById(build_id):
45 """Return the `ISourcePackageRecipeBuild` for the given build id.
46
47=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
48--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-11-30 12:23:01 +0000
49+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-12-20 01:48:17 +0000
50@@ -182,7 +182,7 @@
51 return spbuild
52
53 @staticmethod
54- def makeDailyBuilds():
55+ def makeDailyBuilds(logger=None):
56 from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
57 recipes = SourcePackageRecipe.findStaleDailyBuilds()
58 builds = []
59@@ -200,6 +200,10 @@
60 info = sys.exc_info()
61 errorlog.globalErrorUtility.raising(info)
62 else:
63+ if logger is not None:
64+ logger.debug(
65+ 'Build for %s/%s requested',
66+ recipe.owner.name, recipe.name)
67 builds.append(build)
68 recipe.is_stale = False
69 return builds
70
71=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
72--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-11-19 12:22:15 +0000
73+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-12-20 01:48:17 +0000
74@@ -17,6 +17,7 @@
75 from twisted.trial.unittest import TestCase as TrialTestCase
76
77 from canonical.launchpad.interfaces.lpstorm import IStore
78+from canonical.launchpad.scripts.logger import BufferLogger
79 from canonical.launchpad.webapp.authorization import check_permission
80 from canonical.launchpad.webapp.testing import verifyObject
81 from canonical.testing.layers import (
82@@ -235,6 +236,18 @@
83 self.assertEqual(recipe, build.recipe)
84 self.assertEqual(list(recipe.distroseries), [build.distroseries])
85
86+ def test_makeDailyBuilds_logs_builds(self):
87+ # If a logger is passed into the makeDailyBuilds method, each recipe
88+ # that a build is requested for gets logged.
89+ owner = self.factory.makePerson(name='eric')
90+ self.factory.makeSourcePackageRecipe(
91+ owner=owner, name=u'funky-recipe', build_daily=True)
92+ logger = BufferLogger()
93+ SourcePackageRecipeBuild.makeDailyBuilds(logger)
94+ self.assertEqual(
95+ ['DEBUG: Build for eric/funky-recipe requested\n'],
96+ logger.getLogOutput())
97+
98 def test_makeDailyBuilds_clears_is_stale(self):
99 recipe = self.factory.makeSourcePackageRecipe(
100 build_daily=True, is_stale=True)
101
102=== modified file 'lib/lp/testing/logger.py'
103--- lib/lp/testing/logger.py 2010-07-12 13:06:41 +0000
104+++ lib/lp/testing/logger.py 2010-12-20 01:48:17 +0000
105@@ -4,7 +4,9 @@
106 """Frequently-used logging utilities for test suite."""
107
108 __metaclass__ = type
109-__all__ = ['MockLogger']
110+__all__ = [
111+ 'MockLogger',
112+ ]
113
114 import logging
115 import sys