launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21813
[Merge] lp:~cjwatson/python-oops-twisted/new-oops-publisher-api into lp:python-oops-twisted
Colin Watson has proposed merging lp:~cjwatson/python-oops-twisted/new-oops-publisher-api into lp:python-oops-twisted.
Commit message:
Support the new oops API for publishers.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/python-oops-twisted/new-oops-publisher-api/+merge/330117
And in today's "let's catch up with five-year-old API changes where none of the people involved work on the codebase any more", we have this!
Most of the code and tests are just modified versions of code from python-oops, and defer.inlineCallbacks has brought the structure more closely into line with the original versions. I've tested this with an appropriately-modified Launchpad branch and it seems to be behaving itself.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/python-oops-twisted/new-oops-publisher-api into lp:python-oops-twisted.
=== modified file 'NEWS'
--- NEWS 2011-12-09 06:15:18 +0000
+++ NEWS 2017-09-02 13:20:23 +0000
@@ -6,6 +6,10 @@
NEXT
----
+* Support the new oops API for publishers. Add a new
+ oops_twisted.publishers module with asynchronous versions of helpers from
+ oops.publishers.
+
0.0.6
-----
=== modified file 'oops_twisted/__init__.py'
--- oops_twisted/__init__.py 2011-12-09 06:15:18 +0000
+++ oops_twisted/__init__.py 2017-09-02 13:20:23 +0000
@@ -54,7 +54,7 @@
or similar, or use native Twisted publishers. For instance::
>>> from functools import partial
- >>> config.publishers.append(partial(deferToThread, blocking_publisher))
+ >>> config.publisher = partial(deferToThread, blocking_publisher)
A helper 'defer_publisher' is supplied to do this for your convenience.
@@ -109,7 +109,7 @@
useful with a customised Twisted WSGI resource that runs IBodyProducer
iterators in the IO loop, rather than using up a threadpool thread. To use this
pass tracker=oops_twisted.wsgi.body_producer_tracker when calling
-oops_wsgi.make_app. Note that a non-twisted OOPS Config is assumed because
+oops_wsgi.make_app. Note that a non-twisted OOPS Config is assumed because
the WSGI protocol is synchronous: be sure to provide the oops_wsgi make_app
with a non-twisted OOPS Config.
@@ -148,8 +148,11 @@
__all__ = [
'Config',
+ 'convert_result_to_list',
'defer_publisher',
'OOPSObserver',
+ 'publish_to_many',
+ 'publish_with_fallback',
]
from oops_twisted.config import (
@@ -157,3 +160,8 @@
defer_publisher,
)
from oops_twisted.log import OOPSObserver
+from oops_twisted.publishers import (
+ convert_result_to_list,
+ publish_to_many,
+ publish_with_fallback,
+ )
=== modified file 'oops_twisted/config.py'
--- oops_twisted/config.py 2011-12-09 06:06:25 +0000
+++ oops_twisted/config.py 2017-09-02 13:20:23 +0000
@@ -1,4 +1,4 @@
-# Copyright (c) 2010, 2011, Canonical Ltd
+# Copyright (C) 2010, 2011, 2017 Canonical Ltd.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Lesser General Public License as published by
@@ -25,12 +25,17 @@
"""
from functools import partial
+import warnings
+
+import oops
from twisted.internet import defer
from twisted.internet.threads import deferToThread
-import oops
-
-from createhooks import failure_to_context
+from oops_twisted.createhooks import failure_to_context
+from oops_twisted.publishers import (
+ convert_result_to_list,
+ publish_to_many,
+ )
__all__ = [
'Config',
@@ -51,11 +56,12 @@
oops.Config.__init__(self)
self.on_create.insert(0, failure_to_context)
+ @defer.inlineCallbacks
def publish(self, report):
"""Publish a report.
The report is first synchronously run past self.filters, then fired
- asynchronously at all of self.publishers.
+ asynchronously at self.publisher.
See `pydoc oops.Config.publish` for more documentation.
@@ -65,23 +71,25 @@
"""
for report_filter in self.filters:
if report_filter(report):
- return defer.succeed(None)
- if not self.publishers:
- return defer.succeed([])
- d = self.publishers[0](report)
- result = []
- def stash_id(id):
- report['id'] = id
- result.append(id)
- return report
- d.addCallback(stash_id)
- for publisher in self.publishers[1:]:
- d.addCallback(publisher)
- d.addCallback(stash_id)
- def return_result(_):
- return result
- d.addCallback(return_result)
- return d
+ defer.returnValue(None)
+ # XXX cjwatson 2017-09-02 bug=1015293: Remove this once users have
+ # had a chance to migrate.
+ if self.publishers:
+ warnings.warn(
+ "Using the oops_twisted.Config.publishers attribute is "
+ "deprecated. Use the oops_twisted.Config.publisher attribute "
+ "instead, with an oops_twisted.publishers.publish_to_many "
+ "object if multiple publishers are needed",
+ DeprecationWarning, stacklevel=2)
+ old_publishers = map(convert_result_to_list, self.publishers)
+ if self.publisher:
+ publisher = publish_to_many(self.publisher, *old_publishers)
+ else:
+ publisher = publish_to_many(*old_publishers)
+ ret = yield publisher(report)
+ if ret:
+ report['id'] = ret[-1]
+ defer.returnValue(ret)
def defer_publisher(publisher):
=== added file 'oops_twisted/publishers.py'
--- oops_twisted/publishers.py 1970-01-01 00:00:00 +0000
+++ oops_twisted/publishers.py 2017-09-02 13:20:23 +0000
@@ -0,0 +1,103 @@
+# Copyright (C) 2017 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation, version 3 only.
+#
+# 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 Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# GNU Lesser General Public License version 3 (see the file LICENSE).
+
+"""Asynchronous publisher utilities."""
+
+__all__ = [
+ 'convert_result_to_list',
+ 'publish_to_many',
+ 'publish_with_fallback',
+ ]
+
+from twisted.internet import defer
+
+
+def publish_with_fallback(*publishers):
+ """An asynchronous fallback publisher of oops reports.
+
+ This is a publisher; see Config.publish for the calling and return
+ conventions. This publisher delegates to the supplied publishers
+ by calling them all until one reports that it has published the
+ report, and aggregates the results.
+
+ This is a version of `oops.publishers.publish_with_fallback` suitable
+ for use with Twisted.
+
+ :param *publishers: a list of callables to publish oopses to.
+ :return: a callable that will return a Deferred; on success this will
+ have published a report to each of the publishers.
+ """
+ @defer.inlineCallbacks
+ def publish(report):
+ ret = []
+ for publisher in publishers:
+ published = yield publisher(report)
+ ret.extend(published)
+ if ret:
+ break
+ defer.returnValue(ret)
+ return publish
+
+
+def publish_to_many(*publishers):
+ """An asynchronous fan-out publisher of oops reports.
+
+ This is a publisher; see Config.publish for the calling and return
+ conventions. This publisher delegates to the supplied publishers
+ by calling them all, and aggregates the results.
+
+ If a publisher returns a non-emtpy list (indicating that the report was
+ published) then the last item of this list will be set as the 'id' key
+ in the report before the report is passed to the next publisher. This
+ makes it possible for publishers later in the chain to re-use the id.
+
+ This is a version of `oops.publishers.publish_to_many` suitable for use
+ with Twisted.
+
+ :param *publishers: a list of callables to publish oopses to.
+ :return: a callable that will return a Deferred; on success this will
+ have published a report to each of the publishers.
+ """
+ @defer.inlineCallbacks
+ def publish(report):
+ ret = []
+ for publisher in publishers:
+ if ret:
+ report['id'] = ret[-1]
+ published = yield publisher(report)
+ ret.extend(published)
+ defer.returnValue(ret)
+ return publish
+
+
+def convert_result_to_list(publisher):
+ """Ensure that an asynchronous publisher returns a list.
+
+ The old protocol for publisher callables was to return an id, or
+ a False value if the report was not published. The new protocol
+ is to return a list, which is empty if the report was not
+ published.
+
+ This function converts a publisher using the old protocol into one that
+ uses the new protocol, translating values as needed.
+ """
+ @defer.inlineCallbacks
+ def publish(report):
+ ret = yield publisher(report)
+ if ret:
+ defer.returnValue([ret])
+ else:
+ defer.returnValue([])
+ return publish
=== modified file 'oops_twisted/tests/test_config.py'
--- oops_twisted/tests/test_config.py 2011-12-09 06:06:25 +0000
+++ oops_twisted/tests/test_config.py 2017-09-02 13:20:23 +0000
@@ -15,16 +15,12 @@
"""Tests for the twisted oops config."""
-from functools import partial
-
from testtools import TestCase
from testtools.deferredruntest import AsynchronousDeferredRunTest
-from twisted.internet.threads import deferToThread
from oops_twisted import (
Config,
defer_publisher,
- OOPSObserver,
)
from oops_twisted.createhooks import (
failure_to_context,
@@ -56,7 +52,6 @@
return d
def test_publishers_called_and_results_aggregated(self):
- calls = []
def pub_1(report):
return '1'
def pub_2(report):
@@ -69,6 +64,53 @@
d = config.publish(report)
d.addCallback(lambda result: self.assertEqual(['1', '2'], result))
d.addCallback(lambda _: self.assertEqual({'id': '2'}, report))
+ return d
+
+ def test_publish_to_publisher(self):
+ calls = []
+ config = Config()
+ def succeed(report):
+ calls.append(report.copy())
+ return ['a']
+ config.publisher = defer_publisher(succeed)
+ report = dict(foo='bar')
+ d = config.publish(report)
+ d.addCallback(lambda _: self.assertEqual([dict(foo='bar')], calls))
+ return d
+
+ def test_returns_return_value_of_publisher(self):
+ ids = ['a', 'b']
+ def succeed(report):
+ return ids
+ config = Config()
+ config.publisher = defer_publisher(succeed)
+ report = dict(foo='bar')
+ d = config.publish(report)
+ d.addCallback(lambda result: self.assertEqual(ids, result))
+ return d
+
+ def test_publisher_and_publishers_used_together(self):
+ calls = []
+ def nopublish(report):
+ calls.append(report)
+ return []
+ config = Config()
+ config.publisher = defer_publisher(nopublish)
+ config.publishers.append(defer_publisher(nopublish))
+ d = config.publish({})
+ d.addCallback(lambda _: self.assertEqual(2, len(calls)))
+ return d
+
+ def test_puts_id_in_report(self):
+ the_id = 'b'
+ def succeed(report):
+ return ['a', the_id]
+ config = Config()
+ config.publisher = defer_publisher(succeed)
+ report = dict(foo='bar')
+ d = config.publish(report)
+ d.addCallback(lambda _: self.assertEqual(the_id, report['id']))
+ return d
class TestDeferPublisher(TestCase):
=== added file 'oops_twisted/tests/test_publishers.py'
--- oops_twisted/tests/test_publishers.py 1970-01-01 00:00:00 +0000
+++ oops_twisted/tests/test_publishers.py 2017-09-02 13:20:23 +0000
@@ -0,0 +1,182 @@
+# Copyright (C) 2017 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation, version 3 only.
+#
+# 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 Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# GNU Lesser General Public License version 3 (see the file LICENSE).
+
+"""Tests for the publishers module."""
+
+__metaclass__ = type
+
+import testtools
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from twisted.internet import defer
+
+from oops_twisted.publishers import (
+ convert_result_to_list,
+ publish_to_many,
+ publish_with_fallback,
+ )
+
+
+class TestPublishToMany(testtools.TestCase):
+
+ run_tests_with = AsynchronousDeferredRunTest
+
+ @defer.inlineCallbacks
+ def test_publishes_to_one(self):
+ calls = []
+ def capture(report):
+ calls.append(report)
+ return defer.succeed([])
+ publisher = publish_to_many(capture)
+ yield publisher(dict(foo='bar'))
+ self.assertEqual([dict(foo='bar')], calls)
+
+ @defer.inlineCallbacks
+ def test_publishes_to_two(self):
+ calls1 = []
+ calls2 = []
+ def capture1(report):
+ calls1.append(report)
+ return defer.succeed([])
+ def capture2(report):
+ calls2.append(report)
+ return defer.succeed([])
+ publisher = publish_to_many(capture1, capture2)
+ yield publisher(dict(foo='bar'))
+ self.assertEqual([dict(foo='bar')], calls1)
+ self.assertEqual([dict(foo='bar')], calls2)
+
+ @defer.inlineCallbacks
+ def test_returns_empty_list_with_no_publishers(self):
+ publisher = publish_to_many()
+ result = yield publisher({})
+ self.assertEqual([], result)
+
+ @defer.inlineCallbacks
+ def test_adds_ids_from_publishers(self):
+ first_ids = ['a', 'b']
+ def first_success(report):
+ return defer.succeed(first_ids)
+ second_ids = ['c', 'd']
+ def second_success(report):
+ return defer.succeed(second_ids)
+ publisher = publish_to_many(first_success, second_success)
+ result = yield publisher({})
+ self.assertEqual(first_ids + second_ids, result)
+
+ @defer.inlineCallbacks
+ def test_puts_id_in_report(self):
+ the_id = 'b'
+ def first(report):
+ return defer.succeed(['a', the_id])
+ def second(report):
+ self.assertEqual(the_id, report['id'])
+ return defer.succeed([])
+ publisher = publish_to_many(first, second)
+ yield publisher({})
+
+ @defer.inlineCallbacks
+ def test_puts_nothing_in_report_for_unpublished(self):
+ def first(report):
+ return defer.succeed([])
+ def second(report):
+ if 'id' in report:
+ self.fail("id set to %s when previous publisher "
+ "didn't publish" % report['id'])
+ return defer.succeed([])
+ publisher = publish_to_many(first, second)
+ yield publisher({})
+
+
+class TestPublishWithFallback(testtools.TestCase):
+
+ run_tests_with = AsynchronousDeferredRunTest
+
+ @defer.inlineCallbacks
+ def test_publishes_to_one(self):
+ calls = []
+ def capture(report):
+ calls.append(report)
+ return defer.succeed([])
+ publisher = publish_with_fallback(capture)
+ yield publisher(dict(foo='bar'))
+ self.assertEqual([dict(foo='bar')], calls)
+
+ @defer.inlineCallbacks
+ def test_publishes_to_two(self):
+ calls1 = []
+ calls2 = []
+ def capture1(report):
+ calls1.append(report)
+ return defer.succeed([])
+ def capture2(report):
+ calls2.append(report)
+ return defer.succeed([])
+ publisher = publish_with_fallback(capture1, capture2)
+ yield publisher(dict(foo='bar'))
+ self.assertEqual([dict(foo='bar')], calls1)
+ self.assertEqual([dict(foo='bar')], calls2)
+
+ @defer.inlineCallbacks
+ def test_returns_ids_from_publisher(self):
+ ids = ['a', 'b']
+ def success(report):
+ return defer.succeed(ids)
+ publisher = publish_with_fallback(success)
+ result = yield publisher({})
+ self.assertEqual(ids, result)
+
+ @defer.inlineCallbacks
+ def test_publishes_new(self):
+ def failure(report):
+ return defer.succeed([])
+ calls = []
+ def capture(report):
+ calls.append(report)
+ return defer.succeed([])
+ publisher = publish_with_fallback(failure, capture)
+ result = yield publisher({})
+ self.assertEqual([], result)
+ self.assertEqual(1, len(calls))
+
+ @defer.inlineCallbacks
+ def test_publish_stops_when_a_publisher_succeeds(self):
+ def success(report):
+ return defer.succeed(['the id'])
+ def fail(report):
+ self.fail("Called fallback when primary succeeded.")
+ publisher = publish_with_fallback(success, fail)
+ result = yield publisher({})
+ self.assertEqual(['the id'], result)
+
+
+class ConvertResultToListTests(testtools.TestCase):
+
+ run_tests_with = AsynchronousDeferredRunTest
+
+ @defer.inlineCallbacks
+ def test_converts_False_to_empty_list(self):
+ # A false-ish value gets turned in to an empty list
+ def falseish(report):
+ return defer.succeed(False)
+ result = yield convert_result_to_list(falseish)({})
+ self.assertEqual([], result)
+
+ @defer.inlineCallbacks
+ def test_converts_True_to_list(self):
+ # A true-ish value gets turned in to an empty list
+ def trueish(report):
+ return defer.succeed("aaa")
+ result = yield convert_result_to_list(trueish)({})
+ self.assertEqual(["aaa"], result)
=== modified file 'setup.py'
--- setup.py 2011-12-09 06:15:18 +0000
+++ setup.py 2017-09-02 13:20:23 +0000
@@ -14,7 +14,6 @@
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# GNU Lesser General Public License version 3 (see the file LICENSE).
-
from distutils.core import setup
import os.path
@@ -40,7 +39,7 @@
'Programming Language :: Python',
],
install_requires = [
- 'oops',
+ 'oops>=0.0.11',
'oops_wsgi',
'pytz',
'Twisted',
=== modified file 'versions.cfg'
--- versions.cfg 2011-11-07 03:06:22 +0000
+++ versions.cfg 2017-09-02 13:20:23 +0000
@@ -4,7 +4,7 @@
[versions]
fixtures = 0.3.6
iso8601 = 0.1.4
-oops = 0.0.7
+oops = 0.0.13
oops_wsgi = 0.0.6
paste = 1.7.2
pytz = 2010o
Follow ups
