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

[Martin Albisetti]

Add a view to see the newest oopses.

Add an option to see the newest oopses by project.

[Benji York (benji)]

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)

[lp:~beuno/python-oops-tools/show-newest-oopses]

52. By Martin Albisetti on 2012-10-15

Review comments

[Martin Albisetti (beuno)]

> 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!

[lp:~beuno/python-oops-tools/show-newest-oopses]

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

=== modified file 'src/oopstools/oops/dboopsloader.py'
--- src/oopstools/oops/dboopsloader.py	2011-10-21 13:54:29 +0000
+++ src/oopstools/oops/dboopsloader.py	2012-10-17 22:06:22 +0000
@@ -87,7 +87,7 @@
     # XXX matsubara: remove the if clause when refactoring this code
     # to use a single oops directory.
     if type(oopsdir) == str:
-        oopsdir = [oopsdir,]
+        oopsdir = [oopsdir, ]
 
     # Walk through all subdirectories containing OOPSes and ensure there
     # is a matching database structure for them.
@@ -141,7 +141,7 @@
                     before_date = date - datetime.timedelta(days=1)
                     after_date = date + datetime.timedelta(days=2)
                     query = Oops.objects.filter(
-                        date__range=(before_date,after_date),
+                        date__range=(before_date, after_date),
                         pathname__startswith=prefix).values_list(
                         'pathname', flat=True)
                     oops_reports = set(
@@ -151,7 +151,7 @@
                         oops = self._load_oops(datedir, filename)
                         if oops is not None:
                             yield oops
-                        # We update the last_date only when oops 
+                        # We update the last_date only when oops
                         # has the date different to what we already have
                         # This speeds up the loading process
                         if entry.last_date != date:
@@ -177,7 +177,7 @@
                 # condition.
                 logger.error(
                     "OOPS %s/%s already in the DB.", datedir, filename)
-                transaction.rollback() # Continue to next one.
+                transaction.rollback()  # Continue to next one.
             else:
                 logger.error(
                     "%s raised with datedir: %s filename: %s ",
 
=== modified file 'src/oopstools/oops/dbsummaries.py'
--- src/oopstools/oops/dbsummaries.py	2012-09-11 02:08:56 +0000
+++ src/oopstools/oops/dbsummaries.py	2012-10-17 22:06:22 +0000
@@ -32,6 +32,7 @@
 #############################################################################
 # Groups
 
+
 def _format_http_method_count(data):
     tmp = []
     for method in TRACKED_HTTP_METHODS + ('Other',):
@@ -40,6 +41,7 @@
             tmp.append("%s: %s" % (method, count))
     return ' '.join(tmp)
 
+
 def _escape(value):
     if value is not None:
         if not isinstance(value, unicode):
@@ -333,7 +335,7 @@
                 'oopsinfestation__exception_value')
             )
         super(ErrorSection, self).__init__(
-            title, errors, max_count, _all_groups = all_groups)
+            title, errors, max_count, _all_groups=all_groups)
 
 
 class NotFoundSection(ErrorSection):
@@ -362,7 +364,7 @@
                 '-count', 'most_expensive_statement')
             )
         super(TimeOutSection, self).__init__(
-            title, errors, max_count, _all_groups = all_groups)
+            title, errors, max_count, _all_groups=all_groups)
 
 # We perform the top value queries outside of the ORM for performance.
 #
@@ -772,6 +774,7 @@
             ErrorSection, dict(title='Exceptions', max_count=50), section_set)
         self.addSections(*section_set)
 
+
 class ISDErrorSummary(GenericErrorSummary):
     """Summarize ISD error reports (placeholder)"""
 
 
=== modified file 'src/oopstools/oops/helpers.py'
--- src/oopstools/oops/helpers.py	2011-10-13 20:18:51 +0000
+++ src/oopstools/oops/helpers.py	2012-10-17 22:06:22 +0000
@@ -101,7 +101,7 @@
 def parsedate(date_string):
     """Return a naive date time object for the given string.
 
-    This function ignores subsecond accuracy and the timezone. 
+    This function ignores subsecond accuracy and the timezone.
     May be used to parse a ISO 8601 date string.
 
     >>> ds = '2009-01-02'
@@ -132,7 +132,7 @@
     """Generate a summary with the given prefix and dates."""
     script_cmd = '%s/analyse_error_reports' % settings.BIN_DIR
 
-    args = [script_cmd,]
+    args = [script_cmd]
     for prefix in prefixes:
         args.append("-p%s" % prefix)
     for d in (from_date, to_date):
@@ -161,7 +161,7 @@
     pos = text.find(pattern)
     if pos < 0:
         return "Pattern: '%s' not found in %s" % (pattern, text)
-    return text[pos+len(pattern):].split("===")[0].strip()
+    return text[pos + len(pattern):].split("===")[0].strip()
 
 
 def reset_database():
@@ -209,6 +209,7 @@
 
 _cached_launchpadlib = None
 
+
 def get_launchpadlib():
     """Return a launchpadlib instance from the cache.
 
@@ -225,6 +226,7 @@
 
 _today = None
 
+
 def today():
     """Return a date time object from the cache.
 
@@ -238,6 +240,6 @@
 
 def load_prefixes(report_name):
     """Return a list of Prefix objects grouped by Report."""
-    from oopstools.oops.models import Report 
+    from oopstools.oops.models import Report
     report = Report.objects.get(name=report_name)
     return [prefix.value for prefix in report.prefixes.all()]
 
=== modified file 'src/oopstools/oops/oopsstore.py'
--- src/oopstools/oops/oopsstore.py	2011-10-13 20:18:51 +0000
+++ src/oopstools/oops/oopsstore.py	2012-10-17 22:06:22 +0000
@@ -21,7 +21,7 @@
 import datetime
 import itertools
 
-from oopstools.oops.models import Oops, OopsReadError, oops_re
+from oopstools.oops.models import Oops, OopsReadError, oops_re, Prefix
 
 
 # the section of the OOPS ID before the instance identifier is the
@@ -77,7 +77,7 @@
         # XXX matsubara: remove the if clause when refactoring this code
         # to use a single oops directory.
         if type(oopsdir) == str:
-            oopsdir = [oopsdir,]
+            oopsdir = [oopsdir]
         # XXX matsubara: find_dirs changes directories to do its job and
         # before calling it, record the original directory the script is
         # in and after it finishes building
@@ -123,7 +123,7 @@
         dse = match.group('dse')
         oops = match.group('oopsprefix') + match.group('id')
         if date:
-            pass # got enough data
+            pass  # got enough data
         elif dse:
             # dse is the number of days since the epoch
             day = epoch + datetime.timedelta(days=int(dse) - 1)
@@ -145,7 +145,7 @@
             # between the OOPS filename and the OOPS id, that's why
             # there're two different regex to identify those.
             oops_prefix = filename.split(".")[1]
-            oops_prefix = re.sub("\d+$", "",  oops_prefix)
+            oops_prefix = re.sub("\d+$", "", oops_prefix)
         # Normalize oops prefix
         return oops_prefix.upper()
 
@@ -183,3 +183,14 @@
             return itertools.ifilter(
                 lambda oops: startdatetime <= oops.date <= enddatetime,
                 oops_results)
+
+    def newest(self, prefix, limit=50):
+        """Get the newest oopses."""
+        oops_list = list()
+
+        prefix = Prefix.objects.get(value__exact=prefix)
+        query = Oops.objects.filter(
+            prefix__exact=prefix).order_by('date')[:limit]
+        for oops in query:
+            oops_list.append(oops)
+        return oops_list
 
=== added file 'src/oopstools/oops/templates/newest.html'
--- src/oopstools/oops/templates/newest.html	1970-01-01 00:00:00 +0000
+++ src/oopstools/oops/templates/newest.html	2012-10-17 22:06:22 +0000
@@ -0,0 +1,12 @@
+{% extends "base.html" %}
+
+{% block title %}Newest OOPS Reports{% endblock %}
+
+{% block summaries %}
+
+<ul>
+{% for oops in latest_oopses %}
+        <li><a href="/oops/?oopsid={{ oops }}">{{ oops }}</a></li>
+{% endfor %}
+</ul>
+{% endblock %}
 
=== modified file 'src/oopstools/oops/test/oopsstore.txt'
--- src/oopstools/oops/test/oopsstore.txt	2011-10-13 20:18:51 +0000
+++ src/oopstools/oops/test/oopsstore.txt	2012-10-17 22:06:22 +0000
@@ -119,3 +119,35 @@
     >>> [oops.oopsid for oops in
     ...     store.search(start_date, end_date, prefixes=['CCW'])]
     []
+
+Show the newest oopses for each directory.
+
+    >>> newest_oopses = store.newest()
+    >>> len(newest_oopses)
+    2
+
+    >>> sorted(newest_oopses.values())
+    [['01149-1925canistelubuntu0.oops'], ['61295.SMPM25']]
+
+newest() can optionally limit the amount of oopses it returns.
+    
+    >>> from datetime import date
+    >>> from django.conf import settings
+    >>> from oopstools.oops.helpers import create_fake_oops
+    >>> oops_dir_one = os.path.join(
+    ...     settings.OOPSDIR[0], 'dir1', '2008-03-18')
+    >>> fake_oops_one = open(oops_dir_one + "/" + "68350.X1", "w")
+    >>> contents = create_fake_oops(date(2008, 3, 18))
+    >>> fake_oops_one.write(contents)
+    >>> fake_oops_one.close()
+    >>> newest_oopses = store.newest()
+    >>> sorted(newest_oopses.values())
+    [['01149-1925canistelubuntu0.oops'], ['68350.X1', '61295.SMPM25']]
+
+When limiting to 1, we only get 1 oops from dir1
+
+    >>> newest_oopses = store.newest(limit=1)
+    >>> sorted(newest_oopses.values())
+    [['01149-1925canistelubuntu0.oops'], ['68350.X1']]
+
+    >>> os.remove(fake_oops_one.name)
 
=== modified file 'src/oopstools/oops/test/test_runner.py'
--- src/oopstools/oops/test/test_runner.py	2011-10-13 20:18:51 +0000
+++ src/oopstools/oops/test/test_runner.py	2012-10-17 22:06:22 +0000
@@ -24,7 +24,8 @@
 from django.test.simple import DjangoTestSuiteRunner
 
 DEFAULT_OPTIONS = (
-    doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS |doctest.REPORT_NDIFF)
+    doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS | doctest.REPORT_NDIFF)
+
 
 class CustomTestRunner(DjangoTestSuiteRunner):
 
 
=== modified file 'src/oopstools/oops/views.py'
--- src/oopstools/oops/views.py	2011-11-03 00:08:18 +0000
+++ src/oopstools/oops/views.py	2012-10-17 22:06:22 +0000
@@ -44,7 +44,7 @@
     oopsids.add(query_str.upper())
     oopsids.add(query_str.lower())
     if not query_str.upper().startswith('OOPS-'):
-        oopsids.update(map(lambda x:'OOPS-'+x, oopsids))
+        oopsids.update(map(lambda x: 'OOPS-' + x, oopsids))
 
     # Check each ID in both the database and filesystem
     # (should maybe have an API option for this, instead of doing it manually?)
@@ -151,4 +151,9 @@
             return render_to_response("summary.html", dictionary=data)
 
 
-
+def newest(request):
+    """Show the newest oopses for all projects."""
+    store = OopsStore(settings.OOPSDIR)
+    latest_oopses = store.newest(prefix=request.GET.get('prefix'))
+    data = {'latest_oopses': latest_oopses}
+    return render_to_response("newest.html", dictionary=data)
 
=== modified file 'src/oopstools/urls.py'
--- src/oopstools/urls.py	2011-10-13 20:18:51 +0000
+++ src/oopstools/urls.py	2012-10-17 22:06:22 +0000
@@ -24,7 +24,7 @@
     # Example:
     # (r'^oopstools/', include('oopstools.foo.urls')),
 
-    # Uncomment the admin/doc line below and add 'django.contrib.admindocs' 
+    # Uncomment the admin/doc line below and add 'django.contrib.admindocs'
     # to INSTALLED_APPS to enable admin documentation:
     # (r'^admin/doc/', include('django.contrib.admindocs.urls')),
 
@@ -39,4 +39,5 @@
     (r'^oops[.py]*/meta$', 'oopstools.oops.views.meta'),
     (r'^oops/static/(?P<path>.*)$', 'django.views.static.serve',
         {'document_root': settings.STATIC_DOC_ROOT}),
+    (r'^newest/$', 'oopstools.oops.views.newest')
 )