launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04636
[Merge] lp:~lifeless/python-oops-datedir-repo/extraction into lp:python-oops-datedir-repo
Robert Collins has proposed merging lp:~lifeless/python-oops-datedir-repo/extraction into lp:python-oops-datedir-repo.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/extraction/+merge/71795
This handles the new topic and reporter keys (mapping pageid to topic on in-out for compat with oops-tools (until oops-tools uses this library :P)), and takes more encoding responsibility so that callers can relax a little (given the definition we're aiming at of 'bson serializable is all we need).
--
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/extraction/+merge/71795
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-datedir-repo/extraction into lp:python-oops-datedir-repo.
=== modified file 'oops_datedir_repo/__init__.py'
--- oops_datedir_repo/__init__.py 2011-08-16 02:21:20 +0000
+++ oops_datedir_repo/__init__.py 2011-08-17 01:17:24 +0000
@@ -25,7 +25,7 @@
# established at this point, and setup.py will use a version of next-$(revno).
# If the releaselevel is 'final', then the tarball will be major.minor.micro.
# Otherwise it is major.minor.micro~$(revno).
-__version__ = (0, 0, 2, 'beta', 0)
+__version__ = (0, 0, 3, 'beta', 0)
__all__ = [
'DateDirRepo',
=== modified file 'oops_datedir_repo/serializer_rfc822.py'
--- oops_datedir_repo/serializer_rfc822.py 2011-08-15 05:30:39 +0000
+++ oops_datedir_repo/serializer_rfc822.py 2011-08-17 01:17:24 +0000
@@ -19,13 +19,16 @@
This style of OOPS format is very web server specific, not extensible - it
should be considered deprecated.
-The reports this serializer handles always have the following variables:
+The reports this serializer handles always have the following variables (See
+the python-oops api docs for more information about these variables):
* id: The name of this error report.
* type: The type of the exception that occurred.
* value: The value of the exception that occurred.
* time: The time at which the exception occurred.
-* pageid: The identifier for the template/script that oopsed.
+* reporter: The reporting program.
+* topic: The identifier for the template/script that oopsed.
+ [this is written as Page-Id for compatibility with as yet unported tools.]
* branch_nick: The branch nickname.
* revno: The revision number of the branch.
* tb_text: A text version of the traceback.
@@ -37,6 +40,9 @@
* revno: The revision that the branch was at.
* Informational: A flag, True if the error wasn't fatal- if it was
'informational'.
+ [Deprecated - this is no longer part of the oops report conventions. Existing
+ reports with it set are still read, but the key is only present if it was
+ truely in the report.]
"""
@@ -67,13 +73,16 @@
date =iso8601.parse_date(msg.getheader('date'))
else:
date = None
- pageid = msg.getheader('page-id')
+ topic = msg.getheader('topic')
+ if topic is None:
+ topic = msg.getheader('page-id')
username = msg.getheader('user')
url = msg.getheader('url')
duration = int(float(msg.getheader('duration', '-1')))
informational = msg.getheader('informational')
branch_nick = msg.getheader('branch')
revno = msg.getheader('revision')
+ reporter = msg.getheader('oops-reporter')
# Explicitly use an iterator so we can process the file
# sequentially. In most instances the iterator will actually
@@ -109,10 +118,15 @@
# The rest is traceback.
tb_text = ''.join(lines)
- return dict(id=id, type=exc_type, value=exc_value, time=date,
- pageid=pageid, tb_text=tb_text, username=username, url=url,
+ result = dict(id=id, type=exc_type, value=exc_value, time=date,
+ topic=topic, tb_text=tb_text, username=username, url=url,
duration=duration, req_vars=req_vars, db_statements=statements,
- informational=informational, branch_nick=branch_nick, revno=revno)
+ branch_nick=branch_nick, revno=revno)
+ if informational is not None:
+ result['informational'] = informational
+ if reporter is not None:
+ result['reporter'] = reporter
+ return result
def _normalise_whitespace(s):
@@ -151,45 +165,41 @@
def to_chunks(report):
"""Returns a list of bytestrings making up the serialized oops."""
chunks = []
- chunks.append('Oops-Id: %s\n' % _normalise_whitespace(report['id']))
- if 'type' in report:
- chunks.append(
- 'Exception-Type: %s\n' % _normalise_whitespace(report['type']))
- if 'value' in report:
- chunks.append(
- 'Exception-Value: %s\n' % _normalise_whitespace(report['value']))
+ def header(label, key, optional=True):
+ if optional and key not in report:
+ return
+ value = _safestr(report[key])
+ value = _normalise_whitespace(value)
+ chunks.append('%s: %s\n' % (label, value))
+ header('Oops-Id', 'id', optional=False)
+ header('Exception-Type', 'type')
+ header('Exception-Value', 'value')
if 'time' in report:
chunks.append('Date: %s\n' % report['time'].isoformat())
- if 'pageid' in report:
- chunks.append(
- 'Page-Id: %s\n' % _normalise_whitespace(report['pageid']))
- if 'branch_nick' in report:
- chunks.append('Branch: %s\n' % _safestr(report['branch_nick']))
- if 'revno' in report:
- chunks.append('Revision: %s\n' % report['revno'])
- if 'username' in report:
- chunks.append(
- 'User: %s\n' % _normalise_whitespace(report['username']))
- if 'url' in report:
- chunks.append('URL: %s\n' % _normalise_whitespace(report['url']))
- if 'duration' in report:
- chunks.append('Duration: %s\n' % report['duration'])
- if 'informational' in report:
- chunks.append('Informational: %s\n' % report['informational'])
+ header('Page-Id', 'topic')
+ header('Branch', 'branch_nick')
+ header('Revision', 'revno')
+ header('User', 'username')
+ header('URL', 'url')
+ header('Duration', 'duration')
+ header('Informational', 'informational')
+ header('Oops-Reporter', 'reporter')
chunks.append('\n')
safe_chars = ';/\\?:@&+$, ()*!'
if 'req_vars' in report:
for key, value in report['req_vars']:
- chunks.append('%s=%s\n' % (urllib.quote(key, safe_chars),
- urllib.quote(value, safe_chars)))
+ chunks.append('%s=%s\n' % (
+ urllib.quote(_safestr(key), safe_chars),
+ urllib.quote(_safestr(value), safe_chars)))
chunks.append('\n')
if 'db_statements' in report:
for (start, end, database_id, statement) in report['db_statements']:
chunks.append('%05d-%05d@%s %s\n' % (
- start, end, database_id, _normalise_whitespace(statement)))
+ start, end, _safestr(database_id),
+ _safestr(_normalise_whitespace(statement))))
chunks.append('\n')
if 'tb_text' in report:
- chunks.append(report['tb_text'])
+ chunks.append(_safestr(report['tb_text']))
return chunks
=== modified file 'oops_datedir_repo/tests/test_serializer_rfc822.py'
--- oops_datedir_repo/tests/test_serializer_rfc822.py 2011-08-15 05:30:39 +0000
+++ oops_datedir_repo/tests/test_serializer_rfc822.py 2011-08-17 01:17:24 +0000
@@ -41,7 +41,7 @@
Exception-Type: NotFound
Exception-Value: error message
Date: 2005-04-01T00:00:00+00:00
- Page-Id: IFoo:+foo-template
+ Topic: IFoo:+foo-template
User: Sample User
URL: http://localhost:9000/foo
Duration: 42
@@ -60,7 +60,7 @@
self.assertEqual(report['value'], 'error message')
self.assertEqual(
report['time'], datetime.datetime(2005, 4, 1, tzinfo=utc))
- self.assertEqual(report['pageid'], 'IFoo:+foo-template')
+ self.assertEqual(report['topic'], 'IFoo:+foo-template')
self.assertEqual(report['tb_text'], 'traceback-text')
self.assertEqual(report['username'], 'Sample User')
self.assertEqual(report['url'], 'http://localhost:9000/foo')
@@ -86,7 +86,7 @@
Exception-Type: NotFound
Exception-Value: error message
Date: 2005-04-01T00:00:00+00:00
- Page-Id: IFoo:+foo-template
+ Topic: IFoo:+foo-template
User: Sample User
URL: http://localhost:9000/foo
Duration: 42
@@ -105,7 +105,7 @@
self.assertEqual(report['value'], 'error message')
self.assertEqual(
report['time'], datetime.datetime(2005, 4, 1, tzinfo=utc))
- self.assertEqual(report['pageid'], 'IFoo:+foo-template')
+ self.assertEqual(report['topic'], 'IFoo:+foo-template')
self.assertEqual(report['tb_text'], 'traceback-text')
self.assertEqual(report['username'], 'Sample User')
self.assertEqual(report['url'], 'http://localhost:9000/foo')
@@ -119,7 +119,6 @@
self.assertEqual(len(report['db_statements']), 2)
self.assertEqual(report['db_statements'][0], (1, 5, None, 'SELECT 1'))
self.assertEqual(report['db_statements'][1], (5, 10, None, 'SELECT 2'))
- self.assertFalse(report['informational'])
def test_read_branch_nick_revno(self):
"""Test ErrorReport.read()."""
@@ -128,7 +127,6 @@
Exception-Type: NotFound
Exception-Value: error message
Date: 2005-04-01T00:00:00+00:00
- Page-Id: IFoo:+foo-template
User: Sample User
URL: http://localhost:9000/foo
Duration: 42
@@ -147,6 +145,45 @@
self.assertEqual(report['branch_nick'], 'mybranch')
self.assertEqual(report['revno'], '45')
+ def test_read_reporter(self):
+ """Test ErrorReport.read()."""
+ fp = StringIO.StringIO(dedent("""\
+ Oops-Id: OOPS-A0001
+ Oops-Reporter: foo/bar
+
+ """))
+ report = read(fp)
+ self.assertEqual(report['reporter'], 'foo/bar')
+
+ def test_read_pageid_to_topic(self):
+ """Test ErrorReport.read()."""
+ fp = StringIO.StringIO(dedent("""\
+ Oops-Id: OOPS-A0001
+ Page-Id: IFoo:+foo-template
+
+ """))
+ report = read(fp)
+ self.assertEqual(report['topic'], 'IFoo:+foo-template')
+
+ def test_read_informational_read(self):
+ """Test ErrorReport.read()."""
+ fp = StringIO.StringIO(dedent("""\
+ Oops-Id: OOPS-A0001
+ Informational: True
+
+ """))
+ report = read(fp)
+ self.assertEqual('True', report['informational'])
+
+ def test_read_no_informational_no_key(self):
+ """Test ErrorReport.read()."""
+ fp = StringIO.StringIO(dedent("""\
+ Oops-Id: OOPS-A0001
+
+ """))
+ report = read(fp)
+ self.assertFalse('informational' in report)
+
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
@@ -159,14 +196,13 @@
self.assertEqual(report['type'], None)
self.assertEqual(report['value'], None)
self.assertEqual(report['time'], None)
- self.assertEqual(report['pageid'], None)
+ self.assertEqual(report['topic'], None)
self.assertEqual(report['tb_text'], '')
self.assertEqual(report['username'], None)
self.assertEqual(report['url'], None)
self.assertEqual(report['duration'], -1)
self.assertEqual(len(report['req_vars']), 0)
self.assertEqual(len(report['db_statements']), 0)
- self.assertFalse(report['informational'])
self.assertEqual(report['branch_nick'], None)
self.assertEqual(report['revno'], None)
@@ -180,7 +216,7 @@
'type': 'NotFound',
'value': 'error message',
'time': datetime.datetime(2005, 04, 01, 00, 00, 00, tzinfo=utc),
- 'pageid': 'IFoo:+foo-template',
+ 'topic': 'IFoo:+foo-template',
'tb_text': 'traceback-text',
'username': 'Sample User',
'url': 'http://localhost:9000/foo',
@@ -223,7 +259,7 @@
'type': 'NotFound',
'value': 'error message',
'time': datetime.datetime(2005, 04, 01, 00, 00, 00, tzinfo=utc),
- 'pageid': 'IFoo:+foo-template',
+ 'topic': 'IFoo:+foo-template',
'tb_text': 'traceback-text',
'username': 'Sample User',
'url': 'http://localhost:9000/foo',
@@ -269,3 +305,26 @@
"Oops-Id: OOPS-1234\n",
"\n"
], to_chunks(report))
+
+ def test_reporter(self):
+ report = {'reporter': 'foo', 'id': 'bar'}
+ self.assertEqual([
+ "Oops-Id: bar\n",
+ "Oops-Reporter: foo\n",
+ "\n",
+ ], to_chunks(report))
+
+ def test_bad_strings(self):
+ # Because of the rfc822 limitations, not all strings can be supported
+ # in this format - particularly in headers... so all header strings are
+ # passed through an escape process.
+ report = {'id': u'\xeafoo'}
+ self.assertEqual([
+ "Oops-Id: \\xeafoo\n",
+ "\n",
+ ], to_chunks(report))
+ report = {'id': '\xeafoo'}
+ self.assertEqual([
+ "Oops-Id: \\xeafoo\n",
+ "\n",
+ ], to_chunks(report))
=== modified file 'setup.py'
--- setup.py 2011-08-16 02:21:20 +0000
+++ setup.py 2011-08-17 01:17:24 +0000
@@ -22,7 +22,7 @@
description = file(os.path.join(os.path.dirname(__file__), 'README'), 'rb').read()
setup(name="oops_datedir_repo",
- version="0.0.2",
+ version="0.0.3",
description="OOPS disk serialisation and repository management.",
long_description=description,
maintainer="Launchpad Developers",
