← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~lifeless/python-oops-datedir-repo/bug-888866 into lp:python-oops-datedir-repo

 

Robert Collins has proposed merging lp:~lifeless/python-oops-datedir-repo/bug-888866 into lp:python-oops-datedir-repo.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #888866 in Python OOPS Date-dir repository: "req_vars is documented as a list of tuples"
  https://bugs.launchpad.net/python-oops-datedir-repo/+bug/888866

For more details, see:
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/bug-888866/+merge/81937

Handle req_vars that are dicts, and also add .testr.conf support - its darn useful.

I'll do a release when landing this, so that the LGPL release can take place.
-- 
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/bug-888866/+merge/81937
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-datedir-repo/bug-888866 into lp:python-oops-datedir-repo.
=== modified file '.bzrignore'
--- .bzrignore	2011-08-15 05:30:39 +0000
+++ .bzrignore	2011-11-11 04:51:25 +0000
@@ -8,3 +8,4 @@
 ./download-cache
 ./dist
 ./MANIFEST
+.testrepository

=== added file '.testr.conf'
--- .testr.conf	1970-01-01 00:00:00 +0000
+++ .testr.conf	2011-11-11 04:51:25 +0000
@@ -0,0 +1,4 @@
+[DEFAULT]
+test_command=PYTHONPATH=. bin/py -m subunit.run $LISTOPT $IDOPTION oops_datedir_repo.tests.test_suite
+test_id_option=--load-list $IDFILE
+test_list_option=--list

=== modified file 'NEWS'
--- NEWS	2011-11-11 04:21:05 +0000
+++ NEWS	2011-11-11 04:51:25 +0000
@@ -8,6 +8,11 @@
 
 * Now licensed under LGPL-3. (Robert Collins)
 
+* The req_vars variable in reports is accepted if it is a dict (with simple
+  string keys) and the rfc822 parser will parse into a dict. The bson parser
+  retains the bson structure - new OOPS will serialize differently in bson
+  where the would not in the rfc822 structure. (Robert Collins, #888866)
+
 0.0.10
 ------
 

=== modified file 'README'
--- README	2011-11-11 04:21:05 +0000
+++ README	2011-11-11 04:51:25 +0000
@@ -87,3 +87,7 @@
 For instance::
 
   $ bin/py -m testtools.run oops_datedir_repo.tests.test_suite
+
+If you have testrepository you can run the tests with that::
+
+  $ testr run

=== modified file 'oops_datedir_repo/serializer_bson.py'
--- oops_datedir_repo/serializer_bson.py	2011-11-11 04:21:05 +0000
+++ oops_datedir_repo/serializer_bson.py	2011-11-11 04:51:25 +0000
@@ -34,7 +34,7 @@
 * tb_text: A text version of the traceback.
 * username: The user associated with the request.
 * url: The URL for the failed request.
-* req_vars: The request variables.
+* req_vars: The request variables. Either a list of 2-tuples or a dict.
 * branch_nick: A name for the branch of code that was running when the report
   was triggered.
 * revno: The revision that the branch was at.
@@ -67,7 +67,7 @@
             'username', 'url'):
         report.setdefault(key, None)
     report.setdefault('duration', -1)
-    report.setdefault('req_vars', [])
+    report.setdefault('req_vars', {})
     report.setdefault('tb_text', '')
     report.setdefault('timeline', [])
     return report

=== modified file 'oops_datedir_repo/serializer_rfc822.py'
--- oops_datedir_repo/serializer_rfc822.py	2011-11-11 04:21:05 +0000
+++ oops_datedir_repo/serializer_rfc822.py	2011-11-11 04:51:25 +0000
@@ -33,7 +33,7 @@
 * tb_text: A text version of the traceback.
 * username: The user associated with the request.
 * url: The URL for the failed request.
-* req_vars: The request variables.
+* req_vars: The request variables. Either a list of 2-tuples or a dict.
 * branch_nick: A name for the branch of code that was running when the report
   was triggered.
 * revno: The revision that the branch was at.
@@ -122,6 +122,7 @@
                 req_vars.append([urllib.unquote(key), urllib.unquote(value)])
             elif is_traceback(line):
                 break
+    req_vars = dict(req_vars)
 
     # The rest is traceback.
     tb_text = ''.join([first_tb_line] + list(lines))
@@ -195,7 +196,11 @@
     chunks.append('\n')
     safe_chars = ';/\\?:@&+$, ()*!'
     if 'req_vars' in report:
-        for key, value in report['req_vars']:
+        try:
+            items = sorted(report['req_vars'].items())
+        except AttributeError:
+            items = report['req_vars']
+        for key, value in items:
             chunks.append('%s=%s\n' % (
                     urllib.quote(_safestr(key), safe_chars),
                     urllib.quote(_safestr(value), safe_chars)))

=== modified file 'oops_datedir_repo/tests/test_serializer.py'
--- oops_datedir_repo/tests/test_serializer.py	2011-11-11 04:21:05 +0000
+++ oops_datedir_repo/tests/test_serializer.py	2011-11-11 04:51:25 +0000
@@ -41,11 +41,11 @@
         'username': 'Sample User',
         'url': 'http://localhost:9000/foo',
         'duration': 42.0,
-        'req_vars': [
-            ['HTTP_USER_AGENT', 'Mozilla/5.0'],
-            ['HTTP_REFERER', 'http://localhost:9000/'],
-            ['name=foo', 'hello\nworld'],
-            ],
+        'req_vars': {
+            'HTTP_USER_AGENT': 'Mozilla/5.0',
+            'HTTP_REFERER': 'http://localhost:9000/',
+            'name=foo': 'hello\nworld',
+            },
         'timeline': [
             [1, 5, 'store_a', 'SELECT 1'],
             [5, 10, 'store_b', 'SELECT 2'],

=== modified file 'oops_datedir_repo/tests/test_serializer_rfc822.py'
--- oops_datedir_repo/tests/test_serializer_rfc822.py	2011-11-11 04:21:05 +0000
+++ oops_datedir_repo/tests/test_serializer_rfc822.py	2011-11-11 04:51:25 +0000
@@ -64,12 +64,11 @@
         self.assertEqual(report['username'], 'Sample User')
         self.assertEqual(report['url'], 'http://localhost:9000/foo')
         self.assertEqual(report['duration'], 42)
-        self.assertEqual(len(report['req_vars']), 3)
-        self.assertEqual(report['req_vars'][0],
-            ['HTTP_USER_AGENT', 'Mozilla/5.0'])
-        self.assertEqual(report['req_vars'][1],
-            ['HTTP_REFERER', 'http://localhost:9000/'])
-        self.assertEqual(report['req_vars'][2], ['name=foo', 'hello\nworld'])
+        self.assertEqual({
+            'HTTP_USER_AGENT': 'Mozilla/5.0',
+            'HTTP_REFERER': 'http://localhost:9000/',
+            'name=foo': 'hello\nworld'},
+            report['req_vars'])
         self.assertEqual(len(report['timeline']), 2)
         self.assertEqual(report['timeline'][0], [1, 5, 'store_a', 'SELECT 1'])
         self.assertEqual(report['timeline'][1], [5, 10, 'store_b', 'SELECT 2'])
@@ -99,12 +98,11 @@
                 foo/bar"""))
         report = read(fp)
         self.assertEqual(report['id'], 'OOPS-A0001')
-        self.assertEqual(len(report['req_vars']), 3)
-        self.assertEqual(report['req_vars'][0],
-            ['HTTP_USER_AGENT', 'Mozilla/5.0'])
-        self.assertEqual(report['req_vars'][1],
-            ['HTTP_REFERER', 'http://localhost:9000/'])
-        self.assertEqual(report['req_vars'][2], ['name=foo', 'hello\nworld'])
+        self.assertEqual({
+            'HTTP_USER_AGENT': 'Mozilla/5.0',
+            'HTTP_REFERER': 'http://localhost:9000/',
+            'name=foo': 'hello\nworld'},
+            report['req_vars'])
         self.assertEqual(len(report['timeline']), 2)
         self.assertEqual(
             report['timeline'][0],
@@ -143,12 +141,11 @@
         self.assertEqual(report['username'], 'Sample User')
         self.assertEqual(report['url'], 'http://localhost:9000/foo')
         self.assertEqual(report['duration'], 42)
-        self.assertEqual(len(report['req_vars']), 3)
-        self.assertEqual(report['req_vars'][0],
-            ['HTTP_USER_AGENT', 'Mozilla/5.0'])
-        self.assertEqual(report['req_vars'][1],
-            ['HTTP_REFERER', 'http://localhost:9000/'])
-        self.assertEqual(report['req_vars'][2], ['name=foo', 'hello\nworld'])
+        self.assertEqual({
+            'HTTP_USER_AGENT': 'Mozilla/5.0',
+            'HTTP_REFERER': 'http://localhost:9000/',
+            'name=foo': 'hello\nworld'},
+            report['req_vars'])
         self.assertEqual(len(report['timeline']), 2)
         self.assertEqual(report['timeline'][0], [1, 5, None, 'SELECT 1'])
         self.assertEqual(report['timeline'][1], [5, 10, None, 'SELECT 2'])
@@ -229,8 +226,8 @@
 
     def test_minimal_oops(self):
         # If we get a crazy-small oops, we can read it sensibly.  Because there
-        # is existing legacy code, all keys are filled in with None, [] rather
-        # than being empty.
+        # is existing legacy code, all keys are filled in with None, [] or {}
+        # rather than being empty.
         fp = StringIO.StringIO(dedent("""\
             Oops-Id: OOPS-A0001
             """))
@@ -371,3 +368,19 @@
             "Oops-Id: \\xeafoo\n",
             "\n",
             ], to_chunks(report))
+
+    def test_write_reqvars_dict(self):
+        report = {
+            'req_vars': {'HTTP_USER_AGENT': 'Mozilla/5.0',
+                'HTTP_REFERER': 'http://localhost:9000/',
+                'name=foo': 'hello\nworld'},
+            'id': 'OOPS-1234',
+            }
+        self.assertEqual([
+            "Oops-Id: OOPS-1234\n",
+            "\n",
+            "HTTP_REFERER=http://localhost:9000/\n";,
+            "HTTP_USER_AGENT=Mozilla/5.0\n",
+            "name%3Dfoo=hello%0Aworld\n",
+            "\n",
+            ], to_chunks(report))