Merge lp:~stub/charm-helpers/py3 into lp:charm-helpers

[Stuart Bishop]

A small grabbag of fixes I have needed putting together a new charm.

The main fix is host.write() for Python3, where Python3 is picky about byte strings vs. text strings. Since we use write for all sorts of files, it needs to accept byte strings. Py2 users are unaffected.

Includes fixes for Bug #1402192 , as well as https://code.launchpad.net/~stub/charm-helpers/improve-peerstorage/+merge/243519 (if that hasn't landed already).

[lp:~stub/charm-helpers/py3]

214. By Stuart Bishop on 2014-12-13

Merge trunk

[Jorge Niedbalski (niedbalski)]

LGTM, minor observations.

review: Needs Fixing

[lp:~stub/charm-helpers/py3]

215. By Stuart Bishop on 2014-12-16

Tidy a test

[Stuart Bishop (stub)]

I discussed the .get() method with Jorge, and why the current approach is necessary.

I have tidied the test I made whitespace changes to further per request.

[Jay R. Wren (evarlast)]

The behavior of the in operator is not consistent.

In an attempt to workaround the get bug, one may use in to prevent a KeyError

    if 'foo' not in config:
        return

but this returns even if 'foo' is in _prev_dict.

a workaround is

    if 'foo' not in config.keys():
        return

but expected behavior is that the .keys() call is not necessary.

[lp:~stub/charm-helpers/py3]

216. By Stuart Bishop on 2015-04-29

Merge trunk

217. By Stuart Bishop on 2015-04-29

Add test demonstrating the behavior of the in operator

[Stuart Bishop (stub)]

@evarlast - I've added a test for the behaviour of the in operator, and it seems to be fine.

=== modified file 'charmhelpers/contrib/peerstorage/__init__.py'
--- charmhelpers/contrib/peerstorage/__init__.py	2015-01-22 06:06:03 +0000
+++ charmhelpers/contrib/peerstorage/__init__.py	2015-04-29 13:06:12 +0000
@@ -73,6 +73,8 @@
     exc_list = exc_list if exc_list else []
     peerdb_settings = peer_retrieve('-', relation_name=relation_name)
     matched = {}
+    if peerdb_settings is None:
+        return matched
     for k, v in peerdb_settings.items():
         full_prefix = prefix + delimiter
         if k.startswith(full_prefix):

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-04-07 16:12:51 +0000
+++ charmhelpers/core/hookenv.py	2015-04-29 13:06:12 +0000
@@ -250,6 +250,12 @@
         except KeyError:
             return (self._prev_dict or {})[key]
 
+    def get(self, key, default=None):
+        try:
+            return self[key]
+        except KeyError:
+            return default
+
     def keys(self):
         prev_keys = []
         if self._prev_dict is not None:

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2015-03-12 10:56:57 +0000
+++ charmhelpers/core/host.py	2015-04-29 13:06:12 +0000
@@ -90,7 +90,7 @@
             ['service', service_name, 'status'],
             stderr=subprocess.STDOUT).decode('UTF-8')
     except subprocess.CalledProcessError as e:
-        return 'unrecognized service' not in e.output
+        return b'unrecognized service' not in e.output
     else:
         return True
 

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2015-01-22 06:11:15 +0000
+++ charmhelpers/fetch/__init__.py	2015-04-29 13:06:12 +0000
@@ -158,7 +158,7 @@
 
 def apt_cache(in_memory=True):
     """Build and return an apt cache"""
-    import apt_pkg
+    from apt import apt_pkg
     apt_pkg.init()
     if in_memory:
         apt_pkg.config.set("Dir::Cache::pkgcache", "")

=== modified file 'tests/contrib/peerstorage/test_peerstorage.py'
--- tests/contrib/peerstorage/test_peerstorage.py	2014-08-19 09:38:07 +0000
+++ tests/contrib/peerstorage/test_peerstorage.py	2015-04-29 13:06:12 +0000
@@ -146,6 +146,14 @@
         self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id), settings)
 
     @patch.object(peerstorage, 'peer_retrieve')
+    def test_peer_retrieve_by_prefix_empty_relation(self, peer_retrieve):
+        # If relation-get returns None, peer_retrieve_by_prefix returns
+        # an empty dictionary.
+        peer_retrieve.return_value = None
+        rel_id = 'db:2'
+        self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id), {})
+
+    @patch.object(peerstorage, 'peer_retrieve')
     def test_peer_retrieve_by_prefix_exc_list(self, peer_retrieve):
         rel_id = 'db:2'
         settings = {

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2015-03-12 10:31:22 +0000
+++ tests/core/test_hookenv.py	2015-04-29 13:06:12 +0000
@@ -3,7 +3,7 @@
 from subprocess import CalledProcessError
 import shutil
 import tempfile
-from mock import patch, call, mock_open
+from mock import patch, call, mock_open, sentinel
 from mock import MagicMock
 from testtools import TestCase
 import yaml
@@ -128,11 +128,47 @@
         self.assertEqual(c['foo'], 'bar')
         self.assertEqual(c['baz'], 'bam')
 
+    def test_get(self):
+        c = hookenv.Config(dict(foo='bar'))
+        c.save()
+        c = hookenv.Config(dict(baz='bam'))
+
+        self.assertIsNone(c.get('missing'))
+        self.assertIs(c.get('missing', sentinel.missing), sentinel.missing)
+        self.assertEqual(c.get('foo'), 'bar')
+        self.assertEqual(c.get('baz'), 'bam')
+
     def test_keys(self):
         c = hookenv.Config(dict(foo='bar'))
         c["baz"] = "bar"
         self.assertEqual(sorted([six.u("foo"), "baz"]), sorted(c.keys()))
 
+    def test_in(self):
+        # Test behavior of the in operator.
+
+        # Items that exist in the dict exist. Items that don't don't.
+        c = hookenv.Config(dict(foo='one'))
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' not in c)
+        c.save()
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' not in c)
+
+        # Adding items works as expected.
+        c['foo'] = 'two'
+        c['bar'] = 'two'
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' in c)
+        c.save()
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' in c)
+
+        # Removing items works as expected.
+        del c['foo']
+        self.assertTrue('foo' not in c)
+        c.save()
+        self.assertTrue('foo' not in c)
+
 
 class SerializableTest(TestCase):
     def test_serializes_object_to_json(self):

=== modified file 'tests/core/test_templating.py'
--- tests/core/test_templating.py	2015-02-03 21:52:36 +0000
+++ tests/core/test_templating.py	2015-04-29 13:06:12 +0000
@@ -1,9 +1,9 @@
-import os
 import pkg_resources
 import shutil
 import tempfile
 import unittest
 import jinja2
+import os.path
 import pwd
 import grp
 
@@ -17,7 +17,8 @@
 class TestTemplating(unittest.TestCase):
     def setUp(self):
         self.charm_dir = pkg_resources.resource_filename(__name__, '')
-        self._charm_dir_patch = mock.patch.object(templating.hookenv, 'charm_dir')
+        self._charm_dir_patch = mock.patch.object(templating.hookenv,
+                                                  'charm_dir')
         self._charm_dir_mock = self._charm_dir_patch.start()
         self._charm_dir_mock.side_effect = lambda: self.charm_dir
 
@@ -28,9 +29,8 @@
     @mock.patch.object(templating.host, 'mkdir')
     @mock.patch.object(templating.host, 'log')
     def test_render(self, log, mkdir, fchown):
-        _, fn1 = tempfile.mkstemp()
-        _, fn2 = tempfile.mkstemp()
-        try:
+        with tempfile.NamedTemporaryFile() as fn1, \
+                tempfile.NamedTemporaryFile() as fn2:
             context = {
                 'nats': {
                     'port': '1234',
@@ -41,23 +41,19 @@
                 },
                 'nginx_port': 80,
             }
-            templating.render('fake_cc.yml', fn1,
+            templating.render('fake_cc.yml', fn1.name,
                               context, templates_dir=TEMPLATES_DIR)
-            contents = open(fn1).read()
+            contents = open(fn1.name).read()
             self.assertRegexpMatches(contents, 'port: 1234')
             self.assertRegexpMatches(contents, 'host: example.com')
             self.assertRegexpMatches(contents, 'domain: api.foo.com')
 
-            templating.render('test.conf', fn2, context,
+            templating.render('test.conf', fn2.name, context,
                               templates_dir=TEMPLATES_DIR)
-            contents = open(fn2).read()
+            contents = open(fn2.name).read()
             self.assertRegexpMatches(contents, 'listen 80')
             self.assertEqual(fchown.call_count, 2)
             self.assertEqual(mkdir.call_count, 2)
-        finally:
-            for fn in (fn1, fn2):
-                if os.path.exists(fn):
-                    os.remove(fn)
 
     @mock.patch.object(templating.host.os, 'fchown')
     @mock.patch.object(templating.host, 'log')
@@ -80,8 +76,10 @@
     @mock.patch.object(templating, 'hookenv')
     @mock.patch('jinja2.Environment')
     def test_load_error(self, Env, hookenv):
-        Env().get_template.side_effect = jinja2.exceptions.TemplateNotFound('fake_cc.yml')
+        Env().get_template.side_effect = jinja2.exceptions.TemplateNotFound(
+            'fake_cc.yml')
         self.assertRaises(
             jinja2.exceptions.TemplateNotFound, templating.render,
             'fake.src', 'fake.tgt', {}, templates_dir='tmpl')
-        hookenv.log.assert_called_once_with('Could not load template fake.src from tmpl.', level=hookenv.ERROR)
+        hookenv.log.assert_called_once_with(
+            'Could not load template fake.src from tmpl.', level=hookenv.ERROR)