Merge lp:~james-w/python-oops-dictconfig/check-new-only into lp:python-oops-dictconfig

[James Westby]

Check the value of 'new_only' before deciding whether to wrap the publisher.

Hi,

Currently if the 'new_only' key is in the dict definition then the
resulting publisher will be wrapped. Sometimes it's easier for the
creator to include the key but set it to False, so we check the value
as well before wrapping the publisher.

Thanks,

James

[Sidnei da Silva (sidnei)]

review: Approve

[James Westby (james-w)]

review: Approve

=== modified file 'oops_dictconfig/dictconfig.py'
--- oops_dictconfig/dictconfig.py	2013-02-06 17:00:45 +0000
+++ oops_dictconfig/dictconfig.py	2013-02-21 17:48:19 +0000
@@ -96,7 +96,7 @@
                 raise AssertionError(
                         "Unknown publisher type: %s" % str(publisher_type))
             publish_method = publisher_factory(publisher_defn)
-            if 'new_only' in publisher_defn:
+            if 'new_only' in publisher_defn and publisher_defn['new_only']:
                 publish_method = oops.publish_new_only(publish_method)
             oops_config.publishers.append(publish_method)
     oops_config.template.update(dict_config.get('template', {}))

=== modified file 'oops_dictconfig/tests/test_dictconfig.py'
--- oops_dictconfig/tests/test_dictconfig.py	2013-02-06 17:00:45 +0000
+++ oops_dictconfig/tests/test_dictconfig.py	2013-02-21 17:48:19 +0000
@@ -93,6 +93,15 @@
         #   publish method was wrapped by oops.publish_new_only
         self.assertEquals('result', config.publishers[0].func_name)
 
+    def test_datedir_doesnt_wrap_if_new_only_is_False(self):
+        publisher_defn = dict(type='datedir', error_dir=self.getUniqueString(),
+                new_only=False)
+        config = self.get_config_from_dict([publisher_defn])
+        # FIXME: brittle test, dependent on implementation details of
+        #   oops.publish_new_only. Find a better way of testing that the
+        #   publish method was not wrapped by oops.publish_new_only
+        self.assertEquals('publish', config.publishers[0].func_name)
+
     def test_amqp_missing_exchange_name_gives_error(self):
         publisher_defn = dict(type='amqp')
         e = self.assertRaises(AssertionError,
@@ -218,6 +227,15 @@
         #   publish method was wrapped by oops.publish_new_only
         self.assertEquals('result', config.publishers[0].func_name)
 
+    def test_amqp_doesnt_wrap_if_new_only_is_False(self):
+        publisher_defn = self.get_basic_ampq_definition()
+        publisher_defn['new_only'] = False
+        config = self.get_config_from_dict([publisher_defn])
+        # FIXME: brittle test, dependent on implementation details of
+        #   oops.publish_new_only. Find a better way of testing that the
+        #   publish method was not wrapped by oops.publish_new_only
+        self.assertIsInstance(config.publishers[0], oops_amqp.Publisher)
+
     def test_no_template_gives_blank_template(self):
         config = config_from_dict(dict())
         self.assertEqual({}, config.template)