Merge lp:~abentley/launchpad/twisted-oopsdir into lp:launchpad

Proposed by Aaron Bentley
Status: Merged
Approved by: Aaron Bentley
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/twisted-oopsdir
Merge into: lp:launchpad
Diff against target: 109 lines (+47/-8)
2 files modified
lib/lp/services/job/runner.py (+5/-6)
lib/lp/services/job/tests/test_runner.py (+42/-2)
To merge this branch: bzr merge lp:~abentley/launchpad/twisted-oopsdir
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+19431@code.launchpad.net

Commit message

Use correct OOPS dir for twisted job runner

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

= Summary =
Use the correct oops directory for twisted job runner

== Proposed fix ==
Set the global error utility to the values from self.config_name.

== Pre-implementation notes ==
None

== Implementation details ==
We must set the global settings because scripts may not have write access to
the default error directory.

== Tests ==
bin/test test_runner TestJobCronScript

== Demo and Q/A ==
Get a LOSA to add --twisted to the cron entry for update_preview_diffs.
Cause the script to be run. Ensure that oopses go to the correct directory.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/services/job/tests/test_runner.py
  lib/lp/services/job/runner.py

== Pylint notices ==

lib/lp/services/job/runner.py
    35: [F0401] Unable to import 'lazr.delegates' (No module named delegates)

Revision history for this message
Brad Crittenden (bac) wrote :

Aaron this change looks good. Please alphabetize the imports from lp.services.job.runner.

Thanks,
Brad

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/job/runner.py'
2--- lib/lp/services/job/runner.py 2010-02-04 19:56:36 +0000
3+++ lib/lp/services/job/runner.py 2010-02-18 14:16:24 +0000
4@@ -23,23 +23,21 @@
5 import sys
6
7 from ampoule import child, pool, main
8+import transaction
9 from twisted.internet import defer, reactor
10 from twisted.protocols import amp
11-
12 from zope.component import getUtility
13 from zope.security.proxy import removeSecurityProxy
14+from lazr.delegates import delegates
15
16 from canonical.config import config
17+from canonical.launchpad import scripts
18+from canonical.launchpad.webapp import errorlog
19 from canonical.twistedsupport.task import (
20 ParallelLimitedTaskConsumer, PollingTaskSource)
21-from lazr.delegates import delegates
22-import transaction
23-
24 from lp.services.scripts.base import LaunchpadCronScript
25 from lp.services.job.interfaces.job import LeaseHeld, IRunnableJob, IJob
26 from lp.services.mail.sendmail import MailController
27-from canonical.launchpad import scripts
28-from canonical.launchpad.webapp import errorlog
29
30
31 class BaseRunnableJob:
32@@ -388,6 +386,7 @@
33 self.runner_class = runner_class
34
35 def main(self):
36+ errorlog.globalErrorUtility.configure(self.config_name)
37 job_source = getUtility(self.source_interface)
38 runner = self.runner_class.runFromSource(job_source, self.logger)
39 self.logger.info(
40
41=== modified file 'lib/lp/services/job/tests/test_runner.py'
42--- lib/lp/services/job/tests/test_runner.py 2010-02-01 20:49:35 +0000
43+++ lib/lp/services/job/tests/test_runner.py 2010-02-18 14:16:24 +0000
44@@ -16,9 +16,11 @@
45 from zope.error.interfaces import IErrorReportingUtility
46 from zope.interface import implements
47
48+from lp.code.interfaces.branchmergeproposal import (
49+ IUpdatePreviewDiffJobSource,)
50 from lp.testing.mail_helpers import pop_notifications
51 from lp.services.job.runner import (
52- JobRunner, BaseRunnableJob, TwistedJobRunner
53+ JobCronScript, JobRunner, BaseRunnableJob, TwistedJobRunner
54 )
55 from lp.services.job.interfaces.job import JobStatus, IRunnableJob
56 from lp.services.job.model.job import Job
57@@ -288,7 +290,7 @@
58 def __init__(self):
59 self.entries = []
60
61- def info(self, input):
62+ def info(self, input, *args):
63 self.entries.append(input)
64
65
66@@ -315,5 +317,43 @@
67 self.assertIn('Job ran too long.', oops.value)
68
69
70+class TestJobCronScript(TestCaseWithFactory):
71+
72+ layer = LaunchpadZopelessLayer
73+
74+ def test_configures_oops_handler(self):
75+ """JobCronScript.main should configure the global error utility."""
76+
77+ class DummyRunner:
78+
79+ @classmethod
80+ def runFromSource(cls, source, logger):
81+ expected_config = errorlog.ErrorReportingUtility()
82+ expected_config.configure('update_preview_diffs')
83+ self.assertEqual(
84+ errorlog.globalErrorUtility.oops_prefix,
85+ expected_config.oops_prefix)
86+ return cls()
87+
88+ completed_jobs = []
89+ incomplete_jobs = []
90+
91+ class JobCronScriptSubclass(JobCronScript):
92+ config_name = 'update_preview_diffs'
93+ source_interface = IUpdatePreviewDiffJobSource
94+
95+ def __init__(self):
96+ super(JobCronScriptSubclass, self).__init__(DummyRunner)
97+ self.logger = ListLogger()
98+
99+ old_errorlog = errorlog.globalErrorUtility
100+ try:
101+ errorlog.globalErrorUtility = errorlog.ErrorReportingUtility()
102+ cronscript = JobCronScriptSubclass()
103+ cronscript.main()
104+ finally:
105+ errorlog.globalErrorUtility = old_errorlog
106+
107+
108 def test_suite():
109 return TestLoader().loadTestsFromName(__name__)