Merge lp:~gnuoy/charm-helpers/peerstorage-updates into lp:charm-helpers

[Liam Young]

Add helper to perform relation set and store the dict in the peer db. This function will be used address Bug #1355848

[lp:~gnuoy/charm-helpers/peerstorage-updates]

199. By Liam Young on 2014-08-19

Added peer_retrieve_by_prefix function to return all k/v pairs from the peer db given a prefix and filter using {inc,exc}_list.

[Chris Glass (tribaal)]

A couple of comments inline. The default to mutable argument is a showstopper as far as I'm concerned, but is an easy fix :)

review: Needs Fixing

[Liam Young (gnuoy)]

Thanks for the feedback and the link to the article, very interesting! I'll update the mp accordingly

[lp:~gnuoy/charm-helpers/peerstorage-updates]

200. By Liam Young on 2014-08-19

* Remove mutable defaults from new peer functions and from core.hookenv.relation_set
* Improve peer_store_and_set doc string

[Liam Young (gnuoy)]

review: Needs Resubmitting

[Chris Glass (tribaal)]

Looks good! +1, with a small inline nitpick :)

review: Approve

=== modified file 'charmhelpers/contrib/peerstorage/__init__.py'
--- charmhelpers/contrib/peerstorage/__init__.py	2014-03-07 11:57:28 +0000
+++ charmhelpers/contrib/peerstorage/__init__.py	2014-08-19 14:05:21 +0000
@@ -1,4 +1,6 @@
+from charmhelpers.core.hookenv import relation_id as current_relation_id
 from charmhelpers.core.hookenv import (
+    is_relation_made,
     relation_ids,
     relation_get,
     local_unit,
@@ -49,6 +51,24 @@
                          'peer relation {}'.format(relation_name))
 
 
+def peer_retrieve_by_prefix(prefix, relation_name='cluster', delimiter='_',
+                            inc_list=None, exc_list=None):
+    """ Retrieve k/v pairs given a prefix and filter using {inc,exc}_list """
+    inc_list = inc_list if inc_list else []
+    exc_list = exc_list if exc_list else []
+    peerdb_settings = peer_retrieve('-', relation_name=relation_name)
+    matched = {}
+    for k, v in peerdb_settings.items():
+        full_prefix = prefix + delimiter
+        if k.startswith(full_prefix):
+            new_key = k.replace(full_prefix, '')
+            if new_key in exc_list:
+                continue
+            if new_key in inc_list or len(inc_list) == 0:
+                matched[new_key] = v
+    return matched
+
+
 def peer_store(key, value, relation_name='cluster'):
     """ Store the key/value pair on the named peer relation relation_name """
     cluster_rels = relation_ids(relation_name)
@@ -81,3 +101,30 @@
                     echo_data[attribute] = value
     if len(echo_data) > 0:
         relation_set(relation_settings=echo_data)
+
+
+def peer_store_and_set(relation_id=None, peer_relation_name='cluster',
+                       peer_store_fatal=False, relation_settings=None,
+                       delimiter='_', **kwargs):
+    """Store the passed-in arguments both in the argument relation and in peer storage.
+
+    It functions like doing relation_set() and peer_store() at the same time, with the same data.
+
+    @param relation_id: the id of the relation to store the data on. Defaults to the current relation.
+    @param peer_store_fatal: Set to True, the function will raise an exception should the peer sotrage not be avialable."""
+
+    relation_settings = relation_settings if relation_settings else {}
+    relation_set(relation_id=relation_id,
+                 relation_settings=relation_settings,
+                 **kwargs)
+    if is_relation_made(peer_relation_name):
+        for key, value in dict(kwargs.items() +
+                               relation_settings.items()).iteritems():
+            key_prefix = relation_id or current_relation_id()
+            peer_store(key_prefix + delimiter + key,
+                       value,
+                       relation_name=peer_relation_name)
+    else:
+        if peer_store_fatal:
+            raise ValueError('Unable to detect '
+                             'peer relation {}'.format(peer_relation_name))

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2014-06-09 17:10:38 +0000
+++ charmhelpers/core/hookenv.py	2014-08-19 14:05:21 +0000
@@ -285,8 +285,9 @@
         raise
 
 
-def relation_set(relation_id=None, relation_settings={}, **kwargs):
+def relation_set(relation_id=None, relation_settings=None, **kwargs):
     """Set relation information for the current unit"""
+    relation_settings = relation_settings if relation_settings else {}
     relation_cmd_line = ['relation-set']
     if relation_id is not None:
         relation_cmd_line.extend(('-r', relation_id))

=== modified file 'tests/contrib/peerstorage/test_peerstorage.py'
--- tests/contrib/peerstorage/test_peerstorage.py	2014-03-05 10:32:49 +0000
+++ tests/contrib/peerstorage/test_peerstorage.py	2014-08-19 14:05:21 +0000
@@ -1,10 +1,17 @@
 from tests.helpers import FakeRelation
 from testtools import TestCase
-from mock import patch
+from mock import patch, call
 from charmhelpers.contrib import peerstorage
 
 
-TO_PATCH = ['relation_ids', 'relation_set', 'relation_get', 'local_unit']
+TO_PATCH = [
+    'current_relation_id',
+    'is_relation_made',
+    'local_unit',
+    'relation_get',
+    'relation_ids',
+    'relation_set',
+]
 FAKE_RELATION_NAME = 'cluster'
 FAKE_RELATION = {
     'cluster:0': {
@@ -67,3 +74,102 @@
     def test_peer_echo_includes(self):
         peerstorage.peer_echo(['key1'])
         self.relation_set.assert_called_with(relation_settings={'key1': 'value1'})
+
+    @patch.object(peerstorage, 'peer_store')
+    def test_peer_store_and_set_no_relation(self, peer_store):
+        self.is_relation_made.return_value = False
+        peerstorage.peer_store_and_set(relation_id='db', kwarg1='kwarg1_v')
+        self.relation_set.assert_called_with(relation_id='db',
+                                             relation_settings={},
+                                             kwarg1='kwarg1_v')
+        peer_store.assert_not_called()
+
+    @patch.object(peerstorage, 'peer_store')
+    def test_peer_store_and_set_no_relation_fatal(self, peer_store):
+        self.is_relation_made.return_value = False
+        self.assertRaises(ValueError,
+                          peerstorage.peer_store_and_set,
+                          relation_id='db',
+                          kwarg1='kwarg1_v',
+                          peer_store_fatal=True)
+
+    @patch.object(peerstorage, 'peer_store')
+    def test_peer_store_and_set_kwargs(self, peer_store):
+        self.is_relation_made.return_value = True
+        peerstorage.peer_store_and_set(relation_id='db', kwarg1='kwarg1_v')
+        self.relation_set.assert_called_with(relation_id='db',
+                                             relation_settings={},
+                                             kwarg1='kwarg1_v')
+        calls = [call('db_kwarg1', 'kwarg1_v', relation_name='cluster')]
+        peer_store.assert_has_calls(calls, any_order=True)
+
+    @patch.object(peerstorage, 'peer_store')
+    def test_peer_store_and_rel_settings(self, peer_store):
+        self.is_relation_made.return_value = True
+        rel_setting = {
+            'rel_set1': 'relset1_v'
+        }
+        peerstorage.peer_store_and_set(relation_id='db',
+                                       relation_settings=rel_setting)
+        self.relation_set.assert_called_with(relation_id='db',
+                                             relation_settings=rel_setting)
+        calls = [call('db_rel_set1', 'relset1_v', relation_name='cluster')]
+        peer_store.assert_has_calls(calls, any_order=True)
+
+    @patch.object(peerstorage, 'peer_store')
+    def test_peer_store_and_set(self, peer_store):
+        self.is_relation_made.return_value = True
+        rel_setting = {
+            'rel_set1': 'relset1_v'
+        }
+        peerstorage.peer_store_and_set(relation_id='db',
+                                       relation_settings=rel_setting,
+                                       kwarg1='kwarg1_v',
+                                       delimiter='+')
+        self.relation_set.assert_called_with(relation_id='db',
+                                             relation_settings=rel_setting,
+                                             kwarg1='kwarg1_v')
+        calls = [call('db+rel_set1', 'relset1_v', relation_name='cluster'),
+                 call('db+kwarg1', 'kwarg1_v', relation_name='cluster')]
+        peer_store.assert_has_calls(calls, any_order=True)
+
+    @patch.object(peerstorage, 'peer_retrieve')
+    def test_peer_retrieve_by_prefix(self, peer_retrieve):
+        rel_id = 'db:2'
+        settings = {
+            'user': 'bob',
+            'pass': 'reallyhardpassword',
+            'host': 'myhost',
+        }
+        peer_settings = {rel_id + '_' + k: v for k, v in settings.items()}
+        peer_retrieve.return_value = peer_settings
+        self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id), settings)
+
+    @patch.object(peerstorage, 'peer_retrieve')
+    def test_peer_retrieve_by_prefix_exc_list(self, peer_retrieve):
+        rel_id = 'db:2'
+        settings = {
+            'user': 'bob',
+            'pass': 'reallyhardpassword',
+            'host': 'myhost',
+        }
+        peer_settings = {rel_id + '_' + k: v for k, v in settings.items()}
+        del settings['host']
+        peer_retrieve.return_value = peer_settings
+        self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id,
+                                                              exc_list=['host']),
+                          settings)
+
+    @patch.object(peerstorage, 'peer_retrieve')
+    def test_peer_retrieve_by_prefix_inc_list(self, peer_retrieve):
+        rel_id = 'db:2'
+        settings = {
+            'user': 'bob',
+            'pass': 'reallyhardpassword',
+            'host': 'myhost',
+        }
+        peer_settings = {rel_id + '_' + k: v for k, v in settings.items()}
+        peer_retrieve.return_value = peer_settings
+        self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id,
+                                                              inc_list=['host']),
+                          {'host': 'myhost'})