launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05487
[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))