launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/use-oops-timeline into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/use-oops-timeline/+merge/75935

Another step in the extraction of non-core-code, use the oops-timeline module rather than our own homebrew.
-- 
https://code.launchpad.net/~lifeless/launchpad/use-oops-timeline/+merge/75935
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/use-oops-timeline into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py	2011-09-13 05:23:16 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py	2011-09-18 23:59:29 +0000
@@ -26,6 +26,7 @@
 import oops.createhooks
 from oops_datedir_repo import DateDirRepo
 import oops_datedir_repo.serializer_rfc822
+import oops_timeline
 import pytz
 from zope.component.interfaces import ObjectEvent
 from zope.error.interfaces import IErrorReportingUtility
@@ -96,7 +97,7 @@
     implements(IErrorReport)
 
     def __init__(self, id, type, value, time, tb_text, username,
-                 url, duration, req_vars, db_statements, informational=None,
+                 url, duration, req_vars, timeline, informational=None,
                  branch_nick=None, revno=None, topic=None, reporter=None):
         self.id = id
         self.type = type
@@ -111,7 +112,7 @@
         self.duration = duration
         # informational is ignored - will be going from the oops module soon too.
         self.req_vars = req_vars
-        self.db_statements = db_statements
+        self.db_statements = timeline
         self.branch_nick = branch_nick or versioninfo.branch_nick
         self.revno = revno or versioninfo.revno
 
@@ -244,23 +245,17 @@
         return statement
 
 
-def attach_timeline(report, context):
-    """Attach the timeline of actions to the report.
-
-    Looks for the timeline in content['timeline'] and sets it in
-    report['db_statements'].
-    """
-    timeline = context.get('timeline')
+def filter_sessions_timeline(report, context):
+    """Filter timeline session data in the report."""
+    timeline = report.get('timeline')
     if timeline is None:
         return
     statements = []
-    for action in timeline.actions:
-        start, end, category, detail = action.logTuple()
+    for event in timeline:
+        start, end, category, detail = event[:4]
         detail = _filter_session_statement(category, detail)
-        statements.append(
-            (start, end, category, detail))
-    report['db_statements'] = statements
-    return report
+        statements.append((start, end, category, detail) + event[4:])
+    report['timeline'] = statements
 
 
 class ErrorReportingUtility:
@@ -287,6 +282,8 @@
         if section_name is None:
             section_name = self._default_config_section
         self._oops_config = oops.Config()
+        # We use the timeline module
+        oops_timeline.install_hooks(self._oops_config)
         #
         # What do we want in our reports?
         # Constants:
@@ -300,10 +297,9 @@
         self._oops_config.on_create.append(attach_ignore_from_exception)
         # Zope HTTP requests have lots of goodies.
         self._oops_config.on_create.append(attach_http_request)
-        # Timelines are really useful. raising() gets one from
-        # get_request_timeline, other daemons can make one and use the oops
-        # config directly.
-        self._oops_config.on_create.append(attach_timeline)
+        # We don't want session cookie values in the report - they contain
+        # authentication keys.
+        self._oops_config.on_create.append(filter_sessions_timeline)
         # We permit adding messages during the execution of a script (not
         # threadsafe - so only scripts) - a todo item is to only add this
         # for scripts (or to make it threadsafe)

=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py	2011-08-17 01:06:02 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py	2011-09-18 23:59:29 +0000
@@ -572,7 +572,7 @@
             info = sys.exc_info()
             oops = utility._oops_config.create(
                     dict(exc_info=info, timeline=timeline))
-        self.assertEqual("SELECT '%s'", oops['db_statements'][0][3])
+        self.assertEqual("SELECT '%s'", oops['timeline'][0][3])
 
 
 
@@ -632,7 +632,7 @@
         self.assertEqual(None, report.get('username'))
         self.assertEqual(None, report.get('url'))
         self.assertEqual([], report['req_vars'])
-        self.assertEqual([], report['db_statements'])
+        self.assertEqual([], report['timeline'])
 
     def setUp(self):
         super(TestOopsLoggingHandler, self).setUp()

=== modified file 'setup.py'
--- setup.py	2011-09-05 15:42:27 +0000
+++ setup.py	2011-09-18 23:59:29 +0000
@@ -58,6 +58,7 @@
         'oauth',
         'oops',
         'oops_datedir_repo',
+        'oops_timeline',
         'oops_wsgi',
         'paramiko',
         'pgbouncer',

=== modified file 'versions.cfg'
--- versions.cfg	2011-09-16 09:07:33 +0000
+++ versions.cfg	2011-09-18 23:59:29 +0000
@@ -48,9 +48,10 @@
 mocker = 0.10.1
 mozrunner = 1.3.4
 oauth = 1.0
-oops = 0.0.6
-oops-datedir-repo = 0.0.3
+oops = 0.0.7
+oops-datedir-repo = 0.0.6
 oops-wsgi = 0.0.2
+oops-timeline = 0.0.1
 ordereddict = 1.1
 paramiko = 1.7.4
 Paste = 1.7.2