Merge lp:~lifeless/launchpad/lessGetLastOops into lp:launchpad

Proposed by Robert Collins on 2010-09-15
Status: Merged
Approved by: Jelmer Vernooij on 2010-09-15
Approved revision: no longer in the source branch.
Merged at revision: 11554
Proposed branch: lp:~lifeless/launchpad/lessGetLastOops
Merge into: lp:launchpad
Diff against target: 81 lines (+7/-10)
2 files modified
lib/canonical/launchpad/webapp/errorlog.py (+2/-1)
lib/lp/services/job/tests/test_runner.py (+5/-9)
To merge this branch: bzr merge lp:~lifeless/launchpad/lessGetLastOops
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) 2010-09-15 Approve on 2010-09-15
Review via email: mp+35605@code.launchpad.net

Commit Message

Switch to self.oopses for some getLastOopsReport calls - its cheaper.

Description of the Change

Reduce some uses of getLastOopsReport which appears to be flaky in hard to reproduce situations, but nevertheless is doing to much work anyway.

To post a comment you must log in.
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
2--- lib/canonical/launchpad/webapp/errorlog.py 2010-09-12 11:43:36 +0000
3+++ lib/canonical/launchpad/webapp/errorlog.py 2010-09-15 22:18:52 +0000
4@@ -324,7 +324,7 @@
5 # Check today
6 oopsid, filename = self.log_namer._findHighestSerialFilename(time=now)
7 if filename is None:
8- # Check yesterday
9+ # Check yesterday, we may have just passed midnight.
10 yesterday = now - datetime.timedelta(days=1)
11 oopsid, filename = self.log_namer._findHighestSerialFilename(
12 time=yesterday)
13@@ -484,6 +484,7 @@
14 info.
15 :param now: The datetime to use as the current time. Will be
16 determined if not supplied. Useful for testing.
17+ :return: The ErrorReport created.
18 """
19 return self._raising(
20 info, request=request, now=now, informational=True)
21
22=== modified file 'lib/lp/services/job/tests/test_runner.py'
23--- lib/lp/services/job/tests/test_runner.py 2010-08-20 20:31:18 +0000
24+++ lib/lp/services/job/tests/test_runner.py 2010-09-15 22:18:52 +0000
25@@ -149,7 +149,6 @@
26
27 def test_runAll_skips_lease_failures(self):
28 """Ensure runAll skips jobs whose leases can't be acquired."""
29- last_oops = errorlog.globalErrorUtility.getLastOopsReport()
30 job_1, job_2 = self.makeTwoJobs()
31 job_2.job.acquireLease()
32 runner = JobRunner([job_1, job_2])
33@@ -158,8 +157,7 @@
34 self.assertEqual(JobStatus.WAITING, job_2.job.status)
35 self.assertEqual([job_1], runner.completed_jobs)
36 self.assertEqual([job_2], runner.incomplete_jobs)
37- new_last_oops = errorlog.globalErrorUtility.getLastOopsReport()
38- self.assertEqual(last_oops.id, new_last_oops.id)
39+ self.assertEqual([], self.oopses)
40
41 def test_runAll_reports_oopses(self):
42 """When an error is encountered, report an oops and continue."""
43@@ -177,7 +175,7 @@
44 self.assertEqual(JobStatus.FAILED, job_1.job.status)
45 self.assertEqual(JobStatus.COMPLETED, job_2.job.status)
46 reporter = errorlog.globalErrorUtility
47- oops = reporter.getLastOopsReport()
48+ oops = self.oopses[-1]
49 self.assertIn('Fake exception. Foobar, I say!', oops.tb_text)
50 self.assertEqual(1, len(oops.req_vars))
51 self.assertEqual("{'foo': 'bar'}", oops.req_vars[0][1])
52@@ -195,7 +193,7 @@
53 runner = JobRunner([job_1, job_2])
54 runner.runAll()
55 reporter = getUtility(IErrorReportingUtility)
56- oops = reporter.getLastOopsReport()
57+ oops = self.oopses[-1]
58 self.assertEqual(1, len(oops.req_vars))
59 self.assertEqual("{'foo': 'bar'}", oops.req_vars[0][1])
60
61@@ -231,7 +229,7 @@
62 runner.runAll()
63 (notification,) = pop_notifications()
64 reporter = errorlog.globalErrorUtility
65- oops = reporter.getLastOopsReport()
66+ oops = self.oopses[-1]
67 self.assertIn(
68 'Launchpad encountered an internal error during the following'
69 ' operation: appending a string to a list. It was logged with id'
70@@ -257,10 +255,8 @@
71 job_1.user_error_types = (ExampleError,)
72 job_1.error_recipients = ['jrandom@example.org']
73 runner = JobRunner([job_1, job_2])
74- reporter = errorlog.globalErrorUtility
75- old_oops = reporter.getLastOopsReport()
76 runner.runAll()
77- self.assertNoNewOops(old_oops)
78+ self.assertEqual([], self.oopses)
79 notifications = pop_notifications()
80 self.assertEqual(1, len(notifications))
81 body = notifications[0].get_payload(decode=True)