launchpad-reviewers team mailing list archive

[Diogo Matsubara <matsubara@xxxxxxxxxxxxx>]

Diogo Matsubara has proposed merging lp:~matsubara/python-oops-datedir-repo/improve-db-statement-matches into lp:python-oops-datedir-repo.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~matsubara/python-oops-datedir-repo/improve-db-statement-matches/+merge/73059

This branch fixes a couple of small things:

- improves how request variables and db statements are identified by the parser. Before, it'd allow db statements to be read as if they were request variables. The check for request variables is now a bit more precise.

- Duration got from oops report file is now parsed as a float rather than as an int.

- The first traceback line is preserved avoiding a traceback without a newline character in its first line.

-- 
https://code.launchpad.net/~matsubara/python-oops-datedir-repo/improve-db-statement-matches/+merge/73059
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~matsubara/python-oops-datedir-repo/improve-db-statement-matches into lp:python-oops-datedir-repo.

=== modified file 'oops_datedir_repo/serializer_rfc822.py'
--- oops_datedir_repo/serializer_rfc822.py	2011-08-25 20:53:24 +0000
+++ oops_datedir_repo/serializer_rfc822.py	2011-08-26 14:17:14 +0000
@@ -70,7 +70,7 @@
     exc_value = msg.getheader('exception-value')
     datestr = msg.getheader('date')
     if datestr is not None:
-        date =iso8601.parse_date(msg.getheader('date'))
+        date = iso8601.parse_date(msg.getheader('date'))
     else:
         date = None
     topic = msg.getheader('topic')
@@ -79,9 +79,9 @@
     username = msg.getheader('user')
     url = msg.getheader('url')
     try:
-        duration = int(float(msg.getheader('duration', '-1')))
+        duration = float(msg.getheader('duration', '-1'))
     except ValueError:
-        duration = -1
+        duration = float(-1)
     informational = msg.getheader('informational')
     branch_nick = msg.getheader('branch')
     revno = msg.getheader('revision')
@@ -94,28 +94,27 @@
     lines = iter(msg.fp)
 
     def is_req_var(line):
-        return "=" in line
+        return "=" in line and not is_statement(line)
 
+    statement_pat = re.compile(r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)')
     def is_statement(line):
-        return re.match(r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
+        return statement_pat.match(line)
 
     def is_traceback(line):
-        return 'traceback' in line.lower() or '== EXTRA DATA ==' in line
+        return line.lower().startswith('traceback') or line.startswith(
+            '== EXTRA DATA ==')
 
     req_vars = []
     statements = []
     first_tb_line = ''
     for line in lines:
+        first_tb_line = line
         line = line.strip()
         if line == '':
             continue
         else:
-            if is_req_var(line):
-                key, value = line.split('=', 1)
-                req_vars.append((urllib.unquote(key), urllib.unquote(value)))
-            elif is_statement(line):
-                match = re.match(
-                    r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
+            if is_statement(line):
+                match = statement_pat.match(line)
                 assert match is not None, (
                     "Unable to interpret oops line: %s" % line)
                 start, end, db_id, statement = match.groups()
@@ -123,8 +122,10 @@
                     db_id = intern(db_id)  # This string is repeated lots.
                 statements.append(
                     (int(start), int(end), db_id, statement))
+            elif is_req_var(line):
+                key, value = line.split('=', 1)
+                req_vars.append((urllib.unquote(key), urllib.unquote(value)))
             elif is_traceback(line):
-                first_tb_line = line
                 break
 
     # The rest is traceback.

=== modified file 'oops_datedir_repo/tests/test_serializer_rfc822.py'
--- oops_datedir_repo/tests/test_serializer_rfc822.py	2011-08-25 20:53:24 +0000
+++ oops_datedir_repo/tests/test_serializer_rfc822.py	2011-08-26 14:17:14 +0000
@@ -97,10 +97,11 @@
             HTTP_REFERER=http://localhost:9000/
             name%3Dfoo=hello%0Aworld
 
-            00001-00005@store_a SELECT 1
+            00001-00005@store_a SELECT 1 = 2
             00005-00010@store_b SELECT 2
 
-            traceback-text"""))
+            traceback-text
+                foo/bar"""))
         report = read(fp)
         self.assertEqual(report['id'], 'OOPS-A0001')
         self.assertEqual(report['req_vars'][0], ('HTTP_USER_AGENT',
@@ -109,6 +110,7 @@
                                              'http://localhost:9000/'))
         self.assertEqual(report['req_vars'][2], ('name=foo', 'hello\nworld'))
         self.assertEqual(len(report['db_statements']), 2)
+        self.assertEqual(report['tb_text'], 'traceback-text\n    foo/bar')
 
     def test_read_no_store_id(self):
         """Test ErrorReport.read() for old logs with no store_id."""