launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04635
[Merge] lp:~lifeless/python-oops/extraction into lp:python-oops
Robert Collins has proposed merging lp:~lifeless/python-oops/extraction into lp:python-oops.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lifeless/python-oops/extraction/+merge/71794
Now we start getting interesting functionality - this sets default hooks to handle exceptions, the reporting program, the topic (a more generic pageid), and datestamping.
--
https://code.launchpad.net/~lifeless/python-oops/extraction/+merge/71794
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops/extraction into lp:python-oops.
=== modified file 'oops/__init__.py'
--- oops/__init__.py 2011-08-16 06:12:49 +0000
+++ oops/__init__.py 2011-08-17 01:15: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, 5, 'beta', 0)
+__version__ = (0, 0, 6, 'beta', 0)
__all__ = [
'Config'
=== modified file 'oops/config.py'
--- oops/config.py 2011-08-16 06:24:29 +0000
+++ oops/config.py 2011-08-17 01:15:24 +0000
@@ -43,6 +43,9 @@
the inclusion of binary data in the report, and provides cross-language
compatibility.
+A minimal report can be empty, but this is fairly useless and may even be
+rejected by some repositories.
+
Some well known keys used by Launchpad in its OOPS reports::
* id: The name of this error report.
@@ -50,6 +53,7 @@
* 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.
+ [deprecated: This maps to the new topic key instead.]
* branch_nick: The branch nickname.
* revno: The revision number of the branch.
* tb_text: A text version of the traceback.
@@ -59,8 +63,14 @@
* 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.
-* Informational: A flag, True if the error wasn't fatal- if it was
- 'informational'.
+* reporter: Describes the program creating the report. For instance you might
+ put the name of the program, or its website - as long as its distint from
+ other reporters.
+* topic: The subject or context for the report. With a command line tool you
+ might put the subcommand here, with a web site you might put the template (as
+ opposed to the url). This is used as a weak correlation hint: reports from the
+ same topic are more likely to have the same cause than reports from different
+ topics.
"""
@@ -72,6 +82,8 @@
from copy import deepcopy
+from createhooks import default_hooks
+
class Config:
"""The configuration for the OOPS system.
@@ -94,7 +106,7 @@
def __init__(self):
self.filters = []
- self.on_create = []
+ self.on_create = list(default_hooks)
self.template = {}
self.publishers = []
=== added file 'oops/createhooks.py'
--- oops/createhooks.py 1970-01-01 00:00:00 +0000
+++ oops/createhooks.py 2011-08-17 01:15:24 +0000
@@ -0,0 +1,123 @@
+# Copyright (c) 2010, 2011, Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Various hooks that can be used to populate OOPS reports.
+
+The default_hooks list contains some innocuous hooks which most reporters will
+want.
+"""
+
+__all__ = [
+ 'attach_exc_info',
+ 'attach_date',
+ 'copy_reporter',
+ 'copy_topic',
+ 'copy_url',
+ 'default_hooks',
+ 'safe_unicode',
+ ]
+
+__metaclass__ = type
+
+import datetime
+import traceback
+
+from pytz import utc
+
+# Used to detect missing keys.
+_sentinel = object()
+
+# (Prep for python3 - should be set conditional on version
+strtypes = (basestring,)
+
+
+def _simple_copy(key):
+ """Curry a simple hook that copies a key from context to report."""
+ def copy_key(report, context):
+ value = context.get(key, _sentinel)
+ if value is not _sentinel:
+ report[key] = value
+ copy_key.__doc__ = (
+ "Copy the %s field from context to report, if present." % key)
+ return copy_key
+
+copy_reporter = _simple_copy('reporter')
+copy_topic = _simple_copy('topic')
+copy_url = _simple_copy('url')
+
+
+def safe_unicode(obj):
+ """Used to reliably get *a* string for an object.
+
+ This is called on objects like exceptions, where bson won't be able to
+ serialize it, but a representation is needed for the report. It is
+ exposed a convenience for other on_create hook authors.
+ """
+ if isinstance(obj, unicode):
+ return obj
+ # A call to str(obj) could raise anything at all.
+ # We'll ignore these errors, and print something
+ # useful instead, but also log the error.
+ # We disable the pylint warning for the blank except.
+ try:
+ value = unicode(obj)
+ except:
+ value = u'<unprintable %s object>' % (
+ unicode(type(obj).__name__))
+ # Some objects give back bytestrings to __unicode__...
+ if isinstance(value, str):
+ value = value.decode('latin-1')
+ return value
+
+
+def attach_date(report, context):
+ """Set the time key in report to a datetime of now."""
+ report['time'] = datetime.datetime.now(utc)
+
+
+def attach_exc_info(report, context):
+ """Attach exception info to the report.
+
+ This reads the 'exc_info' key from the context and sets the:
+ * type
+ * value
+ * tb_text
+ keys in the report.
+
+ exc_info must be a tuple, but it can contain either live exception
+ information or simple strings (allowing exceptions that have been
+ serialised and received over the network to be reported).
+ """
+ info = context.get('exc_info')
+ if info is None:
+ return
+ report['type'] = getattr(info[0], '__name__', info[0])
+ report['value'] = safe_unicode(info[1])
+ if isinstance(info[2], strtypes):
+ tb_text = info[2]
+ else:
+ tb_text = u''.join(map(safe_unicode, traceback.format_tb(info[2])))
+ report['tb_text'] = tb_text
+
+
+# hooks that are installed into Config objects by default.
+default_hooks = [
+ attach_exc_info,
+ attach_date,
+ copy_reporter,
+ copy_topic,
+ copy_url,
+ ]
=== modified file 'oops/tests/__init__.py'
--- oops/tests/__init__.py 2011-08-15 06:42:14 +0000
+++ oops/tests/__init__.py 2011-08-17 01:15:24 +0000
@@ -22,6 +22,7 @@
def test_suite():
test_mod_names = [
'config',
+ 'createhooks',
]
return TestLoader().loadTestsFromNames(
['oops.tests.test_' + name for name in test_mod_names])
=== modified file 'oops/tests/test_config.py'
--- oops/tests/test_config.py 2011-08-16 06:24:29 +0000
+++ oops/tests/test_config.py 2011-08-17 01:15:24 +0000
@@ -24,13 +24,14 @@
from testtools.matchers import Is
from oops.config import Config
+from oops.createhooks import default_hooks
class TestConfig(testtools.TestCase):
def test_init(self):
config = Config()
- self.assertEqual([], config.on_create)
+ self.assertEqual(default_hooks, config.on_create)
self.assertEqual([], config.publishers)
self.assertEqual({}, config.template)
@@ -39,24 +40,28 @@
def capture(id, report, context):
calls.append((id, report, context))
config = Config()
+ config.on_create = []
config.on_create.append(partial(capture, '1'))
config.on_create.append(partial(capture, '2'))
- report = config.create('2')
+ report = config.create(dict(foo='2'))
self.assertThat(report, Is(calls[0][1]))
self.assertThat(report, Is(calls[1][1]))
- self.assertEqual([('1', {}, '2'), ('2', {}, '2')], calls)
+ self.assertEqual(
+ [('1', {}, {'foo':'2'}), ('2', {}, {'foo':'2'})], calls)
def test_on_create_no_context(self):
calls = []
def capture(report, context):
calls.append((report, context))
config = Config()
+ config.on_create = []
config.on_create.append(capture)
report = config.create()
self.assertEqual([({}, {})], calls)
def test_create_template(self):
config = Config()
+ config.on_create = []
config.template['base'] = True
config.template['list'] = []
report = config.create()
=== added file 'oops/tests/test_createhooks.py'
--- oops/tests/test_createhooks.py 1970-01-01 00:00:00 +0000
+++ oops/tests/test_createhooks.py 2011-08-17 01:15:24 +0000
@@ -0,0 +1,105 @@
+# Copyright (c) 2010, 2011, Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the oops config module."""
+
+__metaclass__ = type
+
+import datetime
+from functools import partial
+import sys
+
+import testtools
+from testtools.matchers import Is
+
+from oops.createhooks import (
+ attach_exc_info,
+ attach_date,
+ default_hooks,
+ copy_reporter,
+ copy_topic,
+ copy_url,
+ )
+
+
+class TestCreateHooks(testtools.TestCase):
+
+ def test_attach_exc_info_missing(self):
+ report = {}
+ attach_exc_info(report, {})
+ self.assertEqual({}, report)
+
+ def test_attach_exc_info_actual_exception(self):
+ report = {}
+ try:
+ raise ValueError('foo bar')
+ except ValueError:
+ attach_exc_info(report, {'exc_info':sys.exc_info()})
+ self.assertEqual('ValueError', report['type'])
+ self.assertEqual('foo bar', report['value'])
+ self.assertIsInstance(report['tb_text'], basestring)
+
+ def test_attach_date(self):
+ report = {}
+ attach_date(report, {})
+ self.assertIsInstance(report['time'], datetime.datetime)
+
+ def test_attach_exc_info_strings(self):
+ report = {}
+ attach_exc_info(report,
+ {'exc_info':('ValueError', 'foo bar', 'my traceback')})
+ self.assertEqual('ValueError', report['type'])
+ self.assertEqual('foo bar', report['value'])
+ self.assertEqual('my traceback', report['tb_text'])
+
+ def test_attach_exc_info_broken_exception(self):
+ class UnprintableException(Exception):
+ def __str__(self):
+ raise RuntimeError('arrgh')
+ __repr__ = __str__
+ report = {}
+ try:
+ raise UnprintableException('foo bar')
+ except UnprintableException:
+ attach_exc_info(report, {'exc_info':sys.exc_info()})
+ self.assertEqual(
+ '<unprintable UnprintableException object>', report['value'])
+ self.assertIsInstance(report['tb_text'], basestring)
+
+ def test_defaults(self):
+ self.assertEqual([attach_exc_info, attach_date, copy_reporter, copy_topic,
+ copy_url], default_hooks)
+
+ def test_reporter(self):
+ report = {}
+ copy_reporter(report, {})
+ self.assertEqual({}, report)
+ copy_reporter(report, {'reporter': 'foo'})
+ self.assertEqual({'reporter':'foo'}, report)
+
+ def test_topic(self):
+ report = {}
+ copy_topic(report, {})
+ self.assertEqual({}, report)
+ copy_topic(report, {'topic': 'foo'})
+ self.assertEqual({'topic':'foo'}, report)
+
+ def test_url(self):
+ report = {}
+ copy_url(report, {})
+ self.assertEqual({}, report)
+ copy_url(report, {'url': 'foo'})
+ self.assertEqual({'url':'foo'}, report)
=== modified file 'setup.py'
--- setup.py 2011-08-16 06:12:49 +0000
+++ setup.py 2011-08-17 01:15:24 +0000
@@ -23,7 +23,7 @@
os.path.join(os.path.dirname(__file__), 'README'), 'rb').read()
setup(name="oops",
- version="0.0.5",
+ version="0.0.6",
description=\
"OOPS report model and default allocation/[de]serialization.",
long_description=description,