Merge lp:~james-w/python-oops-dictconfig/new-publisher-api into lp:python-oops-dictconfig

[James Westby]

Add a better way to specify fallback publishers now that oops allows it.

Hi,

Now that oops supports the better publish_with_fallback option, allow
that to be specified in the config, and deprecate the 'new_only' method
of doing fallback (as it is deprecated in oops itself).

Thanks,

James

[Diogo Baeder (diogobaeder)]

review: Approve

[James Westby (james-w)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~james-w/python-oops-dictconfig/new-publisher-api into lp:python-oops-dictconfig failed. Below is the output from the failed tests.

Downloading/unpacking testtools
  Running setup.py egg_info for package testtools

    no previously-included directories found matching 'doc/_build'
Downloading/unpacking oops-amqp
  Downloading oops_amqp-0.0.7.tar.gz
  Running setup.py egg_info for package oops-amqp

Downloading/unpacking oops-datedir-repo
  Downloading oops_datedir_repo-0.0.21.tar.gz
  Running setup.py egg_info for package oops-datedir-repo

Downloading/unpacking extras (from testtools)
  Downloading extras-0.0.3.tar.gz
  Running setup.py egg_info for package extras

Downloading/unpacking python-mimeparse (from testtools)
  Downloading python-mimeparse-0.1.4.tar.gz
  Running setup.py egg_info for package python-mimeparse

Downloading/unpacking pymongo (from oops-amqp)
  Running setup.py egg_info for package pymongo

Requirement already satisfied (use --upgrade to upgrade): oops>=0.0.11 in ./virtualenv/lib/python2.7/site-packages/oops-0.0.13-py2.7.egg (from oops-amqp)
Downloading/unpacking amqplib (from oops-amqp)
  Running setup.py egg_info for package amqplib

Downloading/unpacking bson (from oops-datedir-repo)
  Downloading bson-0.3.3.tar.gz
  Running setup.py egg_info for package bson

Downloading/unpacking iso8601 (from oops-datedir-repo)
  Downloading iso8601-0.1.4.tar.gz
  Running setup.py egg_info for package iso8601

Downloading/unpacking launchpadlib (from oops-datedir-repo)
  Running setup.py egg_info for package launchpadlib

Requirement already satisfied (use --upgrade to upgrade): pytz in ./virtualenv/lib/python2.7/site-packages/pytz-2013b-py2.7.egg (from oops-datedir-repo)
Downloading/unpacking httplib2 (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package httplib2

Downloading/unpacking keyring (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package keyring
    zip_safe flag not set; analyzing archive contents...

    Installed /tmp/easy_install-bwAGVc/pytest-runner-1.2/hgtools-3.0.2-py2.7.egg

    Installed /mnt/tarmac/cache/python-oops-dictconfig/virtualenv/build/keyring/pytest_runner-1.2-py2.7.egg

    warning: no previously-included files found matching '.hg/last-message.txt'
Downloading/unpacking lazr.restfulclient>=0.9.19 (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package lazr.restfulclient

Downloading/unpacking lazr.uri (from launchpadlib->oops-datedir-repo)
  Downloading lazr.uri-1.0.3.tar.gz
  Running setup.py egg_info for package lazr.uri

Downloading/unpacking oauth (from launchpadlib->oops-datedir-repo)
  Downloading oauth-1.0.1.tar.gz
  Running setup.py egg_info for package oauth

Requirement already satisfied (use --upgrade to upgrade): distribute in ./virtualenv/lib/python2.7/site-packages/distribute-0.6.24-py2.7.egg (from launchpadlib->oops-datedir-repo)
Downloading/unpacking simplejson (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package simplejson

Downloading/unpacking testresources (from launchpadlib->oops-datedi...

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~james-w/python-oops-dictconfig/new-publisher-api into lp:python-oops-dictconfig failed. Below is the output from the failed tests.

Downloading/unpacking testtools
  Running setup.py egg_info for package testtools

    no previously-included directories found matching 'doc/_build'
Downloading/unpacking oops-amqp
  Downloading oops_amqp-0.0.7.tar.gz
  Running setup.py egg_info for package oops-amqp

Downloading/unpacking oops-datedir-repo
  Downloading oops_datedir_repo-0.0.21.tar.gz
  Running setup.py egg_info for package oops-datedir-repo

Downloading/unpacking configglue
  Downloading configglue-1.0.3.tar.gz
  Running setup.py egg_info for package configglue

Downloading/unpacking extras (from testtools)
  Downloading extras-0.0.3.tar.gz
  Running setup.py egg_info for package extras

Downloading/unpacking python-mimeparse (from testtools)
  Downloading python-mimeparse-0.1.4.tar.gz
  Running setup.py egg_info for package python-mimeparse

Downloading/unpacking pymongo (from oops-amqp)
  Running setup.py egg_info for package pymongo

Requirement already satisfied (use --upgrade to upgrade): oops>=0.0.11 in ./virtualenv/lib/python2.7/site-packages/oops-0.0.13-py2.7.egg (from oops-amqp)
Downloading/unpacking amqplib (from oops-amqp)
  Running setup.py egg_info for package amqplib

Downloading/unpacking bson (from oops-datedir-repo)
  Downloading bson-0.3.3.tar.gz
  Running setup.py egg_info for package bson

Downloading/unpacking iso8601 (from oops-datedir-repo)
  Downloading iso8601-0.1.4.tar.gz
  Running setup.py egg_info for package iso8601

Downloading/unpacking launchpadlib (from oops-datedir-repo)
  Running setup.py egg_info for package launchpadlib

Requirement already satisfied (use --upgrade to upgrade): pytz in ./virtualenv/lib/python2.7/site-packages/pytz-2013b-py2.7.egg (from oops-datedir-repo)
Downloading/unpacking pyxdg (from configglue)
  Running setup.py egg_info for package pyxdg

Downloading/unpacking httplib2 (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package httplib2

Downloading/unpacking keyring (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package keyring
    zip_safe flag not set; analyzing archive contents...

    Installed /tmp/easy_install-SF0734/pytest-runner-1.2/hgtools-3.0.2-py2.7.egg

    Installed /mnt/tarmac/cache/python-oops-dictconfig/virtualenv/build/keyring/pytest_runner-1.2-py2.7.egg

    warning: no previously-included files found matching '.hg/last-message.txt'
Downloading/unpacking lazr.restfulclient>=0.9.19 (from launchpadlib->oops-datedir-repo)
  Running setup.py egg_info for package lazr.restfulclient

Downloading/unpacking lazr.uri (from launchpadlib->oops-datedir-repo)
  Downloading lazr.uri-1.0.3.tar.gz
  Running setup.py egg_info for package lazr.uri

Downloading/unpacking oauth (from launchpadlib->oops-datedir-repo)
  Error <urlopen error timed out> while getting http://pypi.python.org/packages/source/o/oauth/oauth-1.0.1.tar.gz#md5=30ed3cc8c11d7841a89feab437aabf81 (from http://pypi.python.org/simple/oauth/)
Exception:
Traceback (most recent call l...

[Diogo Baeder (diogobaeder)]

Em 01-05-2013 13:30, Ubuntu One Auto Pilot escreveu:
> The proposal to merge lp:~james-w/python-oops-dictconfig/new-publisher-api into lp:python-oops-dictconfig has been updated.
>
> Status: Approved => Needs review
>
> For more details, see:
> https://code.launchpad.net/~james-w/python-oops-dictconfig/new-publisher-api/+merge/159677
http://2guystalkingmetsbaseball.com/wp-content/uploads/2013/04/cultures-escape-bulgaria-gandalf-you-shall-not-pass.jpg

=== modified file 'README'
--- README	2013-02-06 17:00:45 +0000
+++ README	2013-05-01 15:40:31 +0000
@@ -41,7 +41,8 @@
   * type: the type of publisher, currently `"datedir"` and `"amqp"` are
           supported.
 
-All publishers support the following optional key:
+All publishers support the following (deprecated) optional key. Instead of
+using this, you should see the section on fallback publishers:
 
   * new_only: if this is `True` then the publisher will be wrapped in
               `oops.publish_only_new`, meaning that the oops will only
@@ -93,6 +94,32 @@
                to a user is being published (but uniqueness cannot be
                guaranteed).
 
+Fallback publishers
+-------------------
+
+Sometimes it is desirable to have a chain of publishers, where the next in
+the chain is used if the current attempt reported failure. For example, you
+can have an amqp publisher with a datedir fallback, so that if the amqp
+broker is not reachable the oops are saved to a datedir, where they can
+be forwarded later.
+
+To do this create a publisher definition with a 'fallback_chain' key,
+whose value is a list of publishers to chain, e.g.
+
+  {
+     'fallback_chain': [
+         {'type': 'amqp',
+          ...
+         },
+         {'type': 'datedir',
+          ...
+         }
+     ]
+  }
+
+Note that these chains can't be nested, so 'fallback_chain' can only appear
+at the top level of the publishers list.
+
 Examples
 --------
 
@@ -129,18 +156,21 @@
     {
         'publishers': [
             {
-                'type': 'amqp',
-                'host': 'amqp.example.com:5302',
-                'user': 'oopsuser',
-                'password': 'oopspassword',
-                'vhost': 'oopses',
-                'exchange_name': 'oopses',
-                'routing_key': 'oopses',
-            },
-            {
-                'type': 'datedir',
-                'error_dir': '/var/log/oopses/',
-                'new_only': True,
+                'fallback_chain': [
+                    {
+                        'type': 'amqp',
+                        'host': 'amqp.example.com:5302',
+                        'user': 'oopsuser',
+                        'password': 'oopspassword',
+                        'vhost': 'oopses',
+                        'exchange_name': 'oopses',
+                        'routing_key': 'oopses',
+                    },
+                    {
+                        'type': 'datedir',
+                        'error_dir': '/var/log/oopses/',
+                    },
+                ],
             },
         ],
     }
@@ -183,8 +213,11 @@
     oopses = oopses_config
 
     [oopses_config]
-    publishers = amqp_publisher
-                 datedir_publisher
+    publishers = fallback_chain
+
+    [fallback_chain]
+    fallback_chain = amqp_publisher
+                     datedir_publisher
 
     [amqp_publisher]
     type = amqp
@@ -198,7 +231,6 @@
     [datedir_publisher]
     type = datedir
     error_dir = /var/log/oopses
-    only_new = True
 
 
 Running Tests

=== modified file 'oops_dictconfig/configglue_options.py'
--- oops_dictconfig/configglue_options.py	2013-02-26 16:15:03 +0000
+++ oops_dictconfig/configglue_options.py	2013-05-01 15:40:31 +0000
@@ -16,7 +16,7 @@
 from configglue import schema
 
 
-class PublisherDescriptionOption(schema.DictOption):
+class BasePublisherDescriptionOption(schema.DictOption):
 
     def __init__(self, **kwargs):
         if 'spec' not in kwargs:
@@ -55,7 +55,30 @@
                 )
         if 'help' not in kwargs:
             kwargs['help'] = "Config for an oops publisher."
+        super(BasePublisherDescriptionOption, self).__init__(**kwargs)
+
+
+class FallbackChainOption(schema.ListOption):
+
+    def __init__(self, **kwargs):
+        if 'item' not in kwargs:
+            kwargs['item'] = BasePublisherDescriptionOption()
+        if 'help' not in kwargs:
+            kwargs['help'] = ("List of oops publishers in order to publish to."
+                " Each publisher will only be used if all of the publishers"
+                " earlier in the list reported a failure to publish.")
+        super(FallbackChainOption, self).__init__(**kwargs)
+
+
+class PublisherDescriptionOption(BasePublisherDescriptionOption):
+
+    def __init__(self, **kwargs):
+        add_fallback_chain = False
+        if 'spec' not in kwargs:
+            add_fallback_chain = True
         super(PublisherDescriptionOption, self).__init__(**kwargs)
+        if add_fallback_chain:
+            self.spec['fallback_chain'] = FallbackChainOption()
 
 
 class PublishersOption(schema.ListOption):

=== modified file 'oops_dictconfig/dictconfig.py'
--- oops_dictconfig/dictconfig.py	2013-04-17 16:39:49 +0000
+++ oops_dictconfig/dictconfig.py	2013-05-01 15:40:31 +0000
@@ -60,6 +60,28 @@
     }
 
 
+def publisher_from_definition(publisher_defn):
+    publisher_type = publisher_defn.get('type')
+    if publisher_type is None:
+        raise AssertionError(
+                "Missing publisher type: %s" % str(publisher_defn))
+    publisher_factory = PUBLISHERS.get(publisher_type)
+    if publisher_factory is None:
+        raise AssertionError(
+                "Unknown publisher type: %s" % str(publisher_type))
+    publish_method = publisher_factory(publisher_defn)
+    if 'new_only' in publisher_defn and publisher_defn['new_only']:
+        publish_method = oops.publish_new_only(publish_method)
+    return publish_method
+
+
+def make_fallback_chain(publisher_defns):
+    publishers = []
+    for publisher_defn in publisher_defns:
+        publishers.append(publisher_from_definition(publisher_defn))
+    return oops.publish_with_fallback(*publishers)
+
+
 def config_from_dict(dict_config):
     """Create an `oops.Config` object from `dict_config`.
 
@@ -88,17 +110,11 @@
     publishers = []
     if 'publishers' in dict_config:
         for publisher_defn in dict_config['publishers']:
-            publisher_type = publisher_defn.get('type')
-            if publisher_type is None:
-                raise AssertionError(
-                        "Missing publisher type: %s" % str(publisher_defn))
-            publisher_factory = PUBLISHERS.get(publisher_type)
-            if publisher_factory is None:
-                raise AssertionError(
-                        "Unknown publisher type: %s" % str(publisher_type))
-            publish_method = publisher_factory(publisher_defn)
-            if 'new_only' in publisher_defn and publisher_defn['new_only']:
-                publish_method = oops.publish_new_only(publish_method)
+            if 'fallback_chain' in publisher_defn:
+                publish_method = make_fallback_chain(
+                        publisher_defn['fallback_chain'])
+            else:
+                publish_method = publisher_from_definition(publisher_defn)
             publishers.append(publish_method)
     oops_config.publisher = oops.publish_to_many(*publishers)
     oops_config.template.update(dict_config.get('template', {}))

=== modified file 'oops_dictconfig/tests/__init__.py'
--- oops_dictconfig/tests/__init__.py	2012-02-02 22:18:43 +0000
+++ oops_dictconfig/tests/__init__.py	2013-05-01 15:40:31 +0000
@@ -17,6 +17,7 @@
 
 def test_suite():
     module_names = [
+        'oops_dictconfig.tests.test_configglue_options',
         'oops_dictconfig.tests.test_dictconfig',
         ]
     loader = unittest.TestLoader()

=== added file 'oops_dictconfig/tests/test_configglue_options.py'
--- oops_dictconfig/tests/test_configglue_options.py	1970-01-01 00:00:00 +0000
+++ oops_dictconfig/tests/test_configglue_options.py	2013-05-01 15:40:31 +0000
@@ -0,0 +1,114 @@
+# Copyright (c) 2013, 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).
+
+from StringIO import StringIO
+
+from configglue.glue import schemaconfigglue
+from configglue.parser import SchemaConfigParser
+from configglue.schema import Schema, Section
+from testtools import TestCase
+
+from oops_dictconfig.configglue_options import OopsOption
+
+
+class BasicSchema(Schema):
+
+    class oops(Section):
+
+        oops = OopsOption()
+
+
+class ConfigglueOptionsTests(TestCase):
+
+    def get_parser(self, schema_class):
+        return SchemaConfigParser(schema_class())
+
+    def get_options_from_config(self, schema_class, config):
+        """`config` is a file-like object containing the config"""
+        parser = self.get_parser(schema_class)
+        parser.readfp(config)
+        return schemaconfigglue(parser, argv=[])[1]
+
+    def test_emtpy_gives_empty(self):
+        options = self.get_options_from_config(BasicSchema,
+            StringIO(""))
+        self.assertEqual([], options.oops_oops['publishers'])
+        self.assertEqual({}, options.oops_oops['template'])
+
+    def test_basic_amqp_definition(self):
+        options = self.get_options_from_config(BasicSchema,
+            StringIO("""
+[oops]
+oops = oops_config
+
+[oops_config]
+publishers = amqp_publisher
+
+[amqp_publisher]
+type = amqp
+host = example.com
+"""))
+        publishers = options.oops_oops['publishers']
+        self.assertEqual(1, len(publishers))
+        self.assertEqual('amqp', publishers[0]['type'])
+        self.assertEqual('example.com', publishers[0]['host'])
+
+    def test_basic_datedir_definition(self):
+        options = self.get_options_from_config(BasicSchema,
+            StringIO("""
+[oops]
+oops = oops_config
+
+[oops_config]
+publishers = datedir_publisher
+
+[datedir_publisher]
+type = datedir
+error_dir = some/dir
+"""))
+        publishers = options.oops_oops['publishers']
+        self.assertEqual(1, len(publishers))
+        self.assertEqual('datedir', publishers[0]['type'])
+        self.assertEqual('some/dir', publishers[0]['error_dir'])
+
+    def test_fallback_chain(self):
+        options = self.get_options_from_config(BasicSchema,
+            StringIO("""
+[oops]
+oops = oops_config
+
+[oops_config]
+publishers = fallback_chain
+
+[fallback_chain]
+fallback_chain = amqp_publisher
+                 datedir_publisher
+
+[amqp_publisher]
+type = amqp
+host = example.com
+
+[datedir_publisher]
+type = datedir
+error_dir = some/dir
+"""))
+        publishers = options.oops_oops['publishers']
+        self.assertEqual(1, len(publishers))
+        self.assertEqual(2, len(publishers[0]['fallback_chain']))
+        fallback_chain = publishers[0]['fallback_chain']
+        self.assertEqual('amqp', fallback_chain[0]['type'])
+        self.assertEqual('example.com', fallback_chain[0]['host'])
+        self.assertEqual('datedir', fallback_chain[1]['type'])
+        self.assertEqual('some/dir', fallback_chain[1]['error_dir'])

=== modified file 'oops_dictconfig/tests/test_dictconfig.py'
--- oops_dictconfig/tests/test_dictconfig.py	2013-04-17 16:39:49 +0000
+++ oops_dictconfig/tests/test_dictconfig.py	2013-05-01 15:40:31 +0000
@@ -271,3 +271,17 @@
     def test_non_empty_template_gives_non_empty_template(self):
         config = config_from_dict(dict(template=dict(foo='bar')))
         self.assertEqual(dict(foo='bar'), config.template)
+
+    def test_fallback_chain_creates_fallback_chain(self):
+        amqp_defn = self.get_basic_ampq_definition()
+        datedir_defn = dict(type='datedir', error_dir='error_dir')
+        config = self.get_config_from_dict(
+                [dict(fallback_chain=[amqp_defn, datedir_defn])])
+        fallback_fn = get_publishers_from_function(config.publisher)[0]
+        # FIXME: brittle test, dependent on implementation details of
+        #   oops.publish_with_fallback. Find a better way of testing
+        #   that the function is used.
+        self.assertEquals('result', fallback_fn.func_name)
+        chain = get_publishers_from_function(fallback_fn)
+        self.assertIsInstance(chain[0], oops_amqp.Publisher)
+        self.assertIsInstance(chain[1].im_self, DateDirRepo)

=== modified file 'tarmac_tests.sh'
--- tarmac_tests.sh	2012-02-24 21:22:56 +0000
+++ tarmac_tests.sh	2013-05-01 15:40:31 +0000
@@ -6,5 +6,5 @@
 virtualenv --no-site-packages virtualenv > log
 . virtualenv/bin/activate >> log
 python setup.py develop >> log
-pip install testtools oops_amqp oops_datedir_repo
+pip install testtools oops_amqp oops_datedir_repo configglue
 python -m testtools.run oops_dictconfig.tests.test_suite