Merge lp:~beuno/python-oops-tools/show-newest-oopses into lp:python-oops-tools

Proposed by Martin Albisetti on 2012-10-15
Status: Needs review
Proposed branch: lp:~beuno/python-oops-tools/show-newest-oopses
Merge into: lp:python-oops-tools
Diff against target: 309 lines (+85/-18)
9 files modified
src/oopstools/oops/dboopsloader.py (+4/-4)
src/oopstools/oops/dbsummaries.py (+5/-2)
src/oopstools/oops/helpers.py (+6/-4)
src/oopstools/oops/oopsstore.py (+15/-4)
src/oopstools/oops/templates/newest.html (+12/-0)
src/oopstools/oops/test/oopsstore.txt (+32/-0)
src/oopstools/oops/test/test_runner.py (+2/-1)
src/oopstools/oops/views.py (+7/-2)
src/oopstools/urls.py (+2/-1)
To merge this branch: bzr merge lp:~beuno/python-oops-tools/show-newest-oopses
Reviewer Review Type Date Requested Status
Benji York (community) code 2012-10-15 Approve on 2012-10-15
Review via email: mp+129752@code.launchpad.net

Commit message

Add a view to see the newest oopses.

Description of the change

Add an option to see the newest oopses by project.

To post a comment you must log in.
Benji York (benji) wrote :

The whitespace fixes are appreciated. One class of whitespace change
looks like a search/replace error though: lines 8 and 103 of the diff
have one-element lists that included a trailing comma. That was
somewhat odd, but the added space after the comma is even oddder. I
suggest removing both the comma and the space.

I don't know much about the on-disk layout for OOPS storage, but I would
be a little concerned that non-oops files and directories would confuse
the "newest" method. If this trait is common to the rest of the
OOPS-tools code, then my concern can be ignored.

The test for the new "newest" function (line 239 of the diff) contains a
dependency on the order of elements returned from a call the "values"
method of a dict. That order can vary, so the returned list needs to be
sorted in the doctest in order to avoid spurious test failures.

The above applies to the test on line 246 of the diff as well.

Once these small issues are fixed, the code is good to land.

review: Approve (code)
52. By Martin Albisetti on 2012-10-15

Review comments

Martin Albisetti (beuno) wrote :

> The whitespace fixes are appreciated. One class of whitespace change
> looks like a search/replace error though: lines 8 and 103 of the diff
> have one-element lists that included a trailing comma. That was
> somewhat odd, but the added space after the comma is even oddder. I
> suggest removing both the comma and the space.

Done.

> I don't know much about the on-disk layout for OOPS storage, but I would
> be a little concerned that non-oops files and directories would confuse
> the "newest" method. If this trait is common to the rest of the
> OOPS-tools code, then my concern can be ignored.

It is. In this case, the worst that can happen is linking to something that isn't an oops.

> The test for the new "newest" function (line 239 of the diff) contains a
> dependency on the order of elements returned from a call the "values"
> method of a dict. That order can vary, so the returned list needs to be
> sorted in the doctest in order to avoid spurious test failures.
>
> The above applies to the test on line 246 of the diff as well.

Done!

53. By Martin Albisetti on 2012-10-17

Start talking to the DB instead

Unmerged revisions

53. By Martin Albisetti on 2012-10-17

Start talking to the DB instead

52. By Martin Albisetti on 2012-10-15

Review comments

51. By Martin Albisetti on 2012-10-15

Add test

50. By Martin Albisetti on 2012-10-15

Clean up things a bit

49. By Martin Albisetti on 2012-10-15

Working, quick and dirty

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/oopstools/oops/dboopsloader.py'
2--- src/oopstools/oops/dboopsloader.py 2011-10-21 13:54:29 +0000
3+++ src/oopstools/oops/dboopsloader.py 2012-10-17 22:06:22 +0000
4@@ -87,7 +87,7 @@
5 # XXX matsubara: remove the if clause when refactoring this code
6 # to use a single oops directory.
7 if type(oopsdir) == str:
8- oopsdir = [oopsdir,]
9+ oopsdir = [oopsdir, ]
10
11 # Walk through all subdirectories containing OOPSes and ensure there
12 # is a matching database structure for them.
13@@ -141,7 +141,7 @@
14 before_date = date - datetime.timedelta(days=1)
15 after_date = date + datetime.timedelta(days=2)
16 query = Oops.objects.filter(
17- date__range=(before_date,after_date),
18+ date__range=(before_date, after_date),
19 pathname__startswith=prefix).values_list(
20 'pathname', flat=True)
21 oops_reports = set(
22@@ -151,7 +151,7 @@
23 oops = self._load_oops(datedir, filename)
24 if oops is not None:
25 yield oops
26- # We update the last_date only when oops
27+ # We update the last_date only when oops
28 # has the date different to what we already have
29 # This speeds up the loading process
30 if entry.last_date != date:
31@@ -177,7 +177,7 @@
32 # condition.
33 logger.error(
34 "OOPS %s/%s already in the DB.", datedir, filename)
35- transaction.rollback() # Continue to next one.
36+ transaction.rollback() # Continue to next one.
37 else:
38 logger.error(
39 "%s raised with datedir: %s filename: %s ",
40
41=== modified file 'src/oopstools/oops/dbsummaries.py'
42--- src/oopstools/oops/dbsummaries.py 2012-09-11 02:08:56 +0000
43+++ src/oopstools/oops/dbsummaries.py 2012-10-17 22:06:22 +0000
44@@ -32,6 +32,7 @@
45 #############################################################################
46 # Groups
47
48+
49 def _format_http_method_count(data):
50 tmp = []
51 for method in TRACKED_HTTP_METHODS + ('Other',):
52@@ -40,6 +41,7 @@
53 tmp.append("%s: %s" % (method, count))
54 return ' '.join(tmp)
55
56+
57 def _escape(value):
58 if value is not None:
59 if not isinstance(value, unicode):
60@@ -333,7 +335,7 @@
61 'oopsinfestation__exception_value')
62 )
63 super(ErrorSection, self).__init__(
64- title, errors, max_count, _all_groups = all_groups)
65+ title, errors, max_count, _all_groups=all_groups)
66
67
68 class NotFoundSection(ErrorSection):
69@@ -362,7 +364,7 @@
70 '-count', 'most_expensive_statement')
71 )
72 super(TimeOutSection, self).__init__(
73- title, errors, max_count, _all_groups = all_groups)
74+ title, errors, max_count, _all_groups=all_groups)
75
76 # We perform the top value queries outside of the ORM for performance.
77 #
78@@ -772,6 +774,7 @@
79 ErrorSection, dict(title='Exceptions', max_count=50), section_set)
80 self.addSections(*section_set)
81
82+
83 class ISDErrorSummary(GenericErrorSummary):
84 """Summarize ISD error reports (placeholder)"""
85
86
87=== modified file 'src/oopstools/oops/helpers.py'
88--- src/oopstools/oops/helpers.py 2011-10-13 20:18:51 +0000
89+++ src/oopstools/oops/helpers.py 2012-10-17 22:06:22 +0000
90@@ -101,7 +101,7 @@
91 def parsedate(date_string):
92 """Return a naive date time object for the given string.
93
94- This function ignores subsecond accuracy and the timezone.
95+ This function ignores subsecond accuracy and the timezone.
96 May be used to parse a ISO 8601 date string.
97
98 >>> ds = '2009-01-02'
99@@ -132,7 +132,7 @@
100 """Generate a summary with the given prefix and dates."""
101 script_cmd = '%s/analyse_error_reports' % settings.BIN_DIR
102
103- args = [script_cmd,]
104+ args = [script_cmd]
105 for prefix in prefixes:
106 args.append("-p%s" % prefix)
107 for d in (from_date, to_date):
108@@ -161,7 +161,7 @@
109 pos = text.find(pattern)
110 if pos < 0:
111 return "Pattern: '%s' not found in %s" % (pattern, text)
112- return text[pos+len(pattern):].split("===")[0].strip()
113+ return text[pos + len(pattern):].split("===")[0].strip()
114
115
116 def reset_database():
117@@ -209,6 +209,7 @@
118
119 _cached_launchpadlib = None
120
121+
122 def get_launchpadlib():
123 """Return a launchpadlib instance from the cache.
124
125@@ -225,6 +226,7 @@
126
127 _today = None
128
129+
130 def today():
131 """Return a date time object from the cache.
132
133@@ -238,6 +240,6 @@
134
135 def load_prefixes(report_name):
136 """Return a list of Prefix objects grouped by Report."""
137- from oopstools.oops.models import Report
138+ from oopstools.oops.models import Report
139 report = Report.objects.get(name=report_name)
140 return [prefix.value for prefix in report.prefixes.all()]
141
142=== modified file 'src/oopstools/oops/oopsstore.py'
143--- src/oopstools/oops/oopsstore.py 2011-10-13 20:18:51 +0000
144+++ src/oopstools/oops/oopsstore.py 2012-10-17 22:06:22 +0000
145@@ -21,7 +21,7 @@
146 import datetime
147 import itertools
148
149-from oopstools.oops.models import Oops, OopsReadError, oops_re
150+from oopstools.oops.models import Oops, OopsReadError, oops_re, Prefix
151
152
153 # the section of the OOPS ID before the instance identifier is the
154@@ -77,7 +77,7 @@
155 # XXX matsubara: remove the if clause when refactoring this code
156 # to use a single oops directory.
157 if type(oopsdir) == str:
158- oopsdir = [oopsdir,]
159+ oopsdir = [oopsdir]
160 # XXX matsubara: find_dirs changes directories to do its job and
161 # before calling it, record the original directory the script is
162 # in and after it finishes building
163@@ -123,7 +123,7 @@
164 dse = match.group('dse')
165 oops = match.group('oopsprefix') + match.group('id')
166 if date:
167- pass # got enough data
168+ pass # got enough data
169 elif dse:
170 # dse is the number of days since the epoch
171 day = epoch + datetime.timedelta(days=int(dse) - 1)
172@@ -145,7 +145,7 @@
173 # between the OOPS filename and the OOPS id, that's why
174 # there're two different regex to identify those.
175 oops_prefix = filename.split(".")[1]
176- oops_prefix = re.sub("\d+$", "", oops_prefix)
177+ oops_prefix = re.sub("\d+$", "", oops_prefix)
178 # Normalize oops prefix
179 return oops_prefix.upper()
180
181@@ -183,3 +183,14 @@
182 return itertools.ifilter(
183 lambda oops: startdatetime <= oops.date <= enddatetime,
184 oops_results)
185+
186+ def newest(self, prefix, limit=50):
187+ """Get the newest oopses."""
188+ oops_list = list()
189+
190+ prefix = Prefix.objects.get(value__exact=prefix)
191+ query = Oops.objects.filter(
192+ prefix__exact=prefix).order_by('date')[:limit]
193+ for oops in query:
194+ oops_list.append(oops)
195+ return oops_list
196
197=== added file 'src/oopstools/oops/templates/newest.html'
198--- src/oopstools/oops/templates/newest.html 1970-01-01 00:00:00 +0000
199+++ src/oopstools/oops/templates/newest.html 2012-10-17 22:06:22 +0000
200@@ -0,0 +1,12 @@
201+{% extends "base.html" %}
202+
203+{% block title %}Newest OOPS Reports{% endblock %}
204+
205+{% block summaries %}
206+
207+<ul>
208+{% for oops in latest_oopses %}
209+ <li><a href="/oops/?oopsid={{ oops }}">{{ oops }}</a></li>
210+{% endfor %}
211+</ul>
212+{% endblock %}
213
214=== modified file 'src/oopstools/oops/test/oopsstore.txt'
215--- src/oopstools/oops/test/oopsstore.txt 2011-10-13 20:18:51 +0000
216+++ src/oopstools/oops/test/oopsstore.txt 2012-10-17 22:06:22 +0000
217@@ -119,3 +119,35 @@
218 >>> [oops.oopsid for oops in
219 ... store.search(start_date, end_date, prefixes=['CCW'])]
220 []
221+
222+Show the newest oopses for each directory.
223+
224+ >>> newest_oopses = store.newest()
225+ >>> len(newest_oopses)
226+ 2
227+
228+ >>> sorted(newest_oopses.values())
229+ [['01149-1925canistelubuntu0.oops'], ['61295.SMPM25']]
230+
231+newest() can optionally limit the amount of oopses it returns.
232+
233+ >>> from datetime import date
234+ >>> from django.conf import settings
235+ >>> from oopstools.oops.helpers import create_fake_oops
236+ >>> oops_dir_one = os.path.join(
237+ ... settings.OOPSDIR[0], 'dir1', '2008-03-18')
238+ >>> fake_oops_one = open(oops_dir_one + "/" + "68350.X1", "w")
239+ >>> contents = create_fake_oops(date(2008, 3, 18))
240+ >>> fake_oops_one.write(contents)
241+ >>> fake_oops_one.close()
242+ >>> newest_oopses = store.newest()
243+ >>> sorted(newest_oopses.values())
244+ [['01149-1925canistelubuntu0.oops'], ['68350.X1', '61295.SMPM25']]
245+
246+When limiting to 1, we only get 1 oops from dir1
247+
248+ >>> newest_oopses = store.newest(limit=1)
249+ >>> sorted(newest_oopses.values())
250+ [['01149-1925canistelubuntu0.oops'], ['68350.X1']]
251+
252+ >>> os.remove(fake_oops_one.name)
253
254=== modified file 'src/oopstools/oops/test/test_runner.py'
255--- src/oopstools/oops/test/test_runner.py 2011-10-13 20:18:51 +0000
256+++ src/oopstools/oops/test/test_runner.py 2012-10-17 22:06:22 +0000
257@@ -24,7 +24,8 @@
258 from django.test.simple import DjangoTestSuiteRunner
259
260 DEFAULT_OPTIONS = (
261- doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS |doctest.REPORT_NDIFF)
262+ doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS | doctest.REPORT_NDIFF)
263+
264
265 class CustomTestRunner(DjangoTestSuiteRunner):
266
267
268=== modified file 'src/oopstools/oops/views.py'
269--- src/oopstools/oops/views.py 2011-11-03 00:08:18 +0000
270+++ src/oopstools/oops/views.py 2012-10-17 22:06:22 +0000
271@@ -44,7 +44,7 @@
272 oopsids.add(query_str.upper())
273 oopsids.add(query_str.lower())
274 if not query_str.upper().startswith('OOPS-'):
275- oopsids.update(map(lambda x:'OOPS-'+x, oopsids))
276+ oopsids.update(map(lambda x: 'OOPS-' + x, oopsids))
277
278 # Check each ID in both the database and filesystem
279 # (should maybe have an API option for this, instead of doing it manually?)
280@@ -151,4 +151,9 @@
281 return render_to_response("summary.html", dictionary=data)
282
283
284-
285+def newest(request):
286+ """Show the newest oopses for all projects."""
287+ store = OopsStore(settings.OOPSDIR)
288+ latest_oopses = store.newest(prefix=request.GET.get('prefix'))
289+ data = {'latest_oopses': latest_oopses}
290+ return render_to_response("newest.html", dictionary=data)
291
292=== modified file 'src/oopstools/urls.py'
293--- src/oopstools/urls.py 2011-10-13 20:18:51 +0000
294+++ src/oopstools/urls.py 2012-10-17 22:06:22 +0000
295@@ -24,7 +24,7 @@
296 # Example:
297 # (r'^oopstools/', include('oopstools.foo.urls')),
298
299- # Uncomment the admin/doc line below and add 'django.contrib.admindocs'
300+ # Uncomment the admin/doc line below and add 'django.contrib.admindocs'
301 # to INSTALLED_APPS to enable admin documentation:
302 # (r'^admin/doc/', include('django.contrib.admindocs.urls')),
303
304@@ -39,4 +39,5 @@
305 (r'^oops[.py]*/meta$', 'oopstools.oops.views.meta'),
306 (r'^oops/static/(?P<path>.*)$', 'django.views.static.serve',
307 {'document_root': settings.STATIC_DOC_ROOT}),
308+ (r'^newest/$', 'oopstools.oops.views.newest')
309 )

Subscribers

People subscribed via source and target branches

to all changes: