Reducing NotificationCentre log levels. Fixing rest_communication req…

…uest building: now using params instead of url, fixing bug where spaces were inappropriately removed.

egcg_core/__init__.py

@@ -1 +1 @@
-__version__ = '0.6.2'
+__version__ = '0.6.4'

egcg_core/notifications/__init__.py

@@ -1,8 +1,8 @@
 from egcg_core.app_logging import AppLogger
 from egcg_core.config import cfg
-from .asana_notification import AsanaNotification
-from .email_notification import EmailNotification
-from .log_notification import LogNotification
+from .asana import AsanaNotification
+from .email import EmailNotification
+from .log import LogNotification
 
 
 class NotificationCentre(AppLogger):
@@ -18,7 +18,7 @@ def __init__(self, name):
 
         for s in cfg.get('notifications', {}):
             if s in self.ntf_aliases:
-                self.info('Configuring notification for: ' + s)
+                self.debug('Configuring notification for: ' + s)
                 config = cfg['notifications'][s]
                 self.subscribers[s] = self.ntf_aliases[s](name=self.name, **config)
             else:
@@ -29,7 +29,7 @@ def notify(self, msg, subs):
             if s in self.subscribers:
                 self.subscribers[s].notify(msg)
             else:
-                self.warning('Tried to notify by %s, but no configuration present', s)
+                self.debug('Tried to notify by %s, but no configuration present', s)
 
     def notify_all(self, msg):
         for name, s in self.subscribers.items():

egcg_core/rest_communication.py

@@ -1,18 +1,26 @@
 import requests
+import json
 from urllib.parse import urljoin
 from egcg_core.config import cfg
 from egcg_core.app_logging import AppLogger
 from egcg_core.exceptions import RestCommunicationError
 
 
 class Communicator(AppLogger):
-    table = {' ': '', '\'': '"', 'None': 'null'}
+    table = {'None': 'null'}
     successful_statuses = (200, 201, 202)
 
     def __init__(self, auth=None, baseurl=None):
         self._baseurl = baseurl
         self._auth = auth
 
+    @staticmethod
+    def serialise(queries):
+        serialised_queries = {}
+        for k, v in queries.items():
+            serialised_queries[k] = json.dumps(v) if type(v) is dict else v
+        return serialised_queries
+
     @property
     def baseurl(self):
         if self._baseurl is None:
@@ -31,14 +39,8 @@ def _translate(cls, s):
             s = s.replace(k, v)
         return s
 
-    def api_url(self, endpoint, **query_args):
-        url = '{base_url}/{endpoint}/'.format(
-            base_url=self.baseurl, endpoint=endpoint
-        )
-        if query_args:
-            query = '?' + '&'.join('%s=%s' % (k, v) for k, v in query_args.items())
-            url += self._translate(query)
-        return url
+    def api_url(self, endpoint):
+        return '{base_url}/{endpoint}/'.format(base_url=self.baseurl, endpoint=endpoint)
 
     @staticmethod
     def _parse_query_string(query_string, requires=None):
@@ -83,8 +85,8 @@ def get_content(self, endpoint, paginate=True, quiet=False, **query_args):
                 max_results=query_args.pop('max_results', 100),  # default to page size of 100
                 page=query_args.pop('page', 1)
             )
-        url = self.api_url(endpoint, **query_args)
-        return self._req('GET', url, quiet=quiet).json()
+        url = self.api_url(endpoint)
+        return self._req('GET', url, quiet=quiet, params=self.serialise(query_args)).json()
 
     def get_documents(self, endpoint, paginate=True, all_pages=False, quiet=False, **query_args):
         content = self.get_content(endpoint, paginate, quiet, **query_args)

tests/test_notifications.py

@@ -63,7 +63,7 @@ def setUp(self):
         )
 
     def test_retries(self):
-        with patch('smtplib.SMTP', new=FakeSMTP), patch('egcg_core.notifications.email_notification.sleep'):
+        with patch('smtplib.SMTP', new=FakeSMTP), patch('egcg_core.notifications.email.sleep'):
             assert self.email_ntf._try_send(self.email_ntf.preprocess('this is a test')) is True
             assert self.email_ntf._try_send(self.email_ntf.preprocess('dodgy')) is False
 

tests/test_rest_communication.py

@@ -45,37 +45,28 @@ def setUp(self):
         self.comm = rest_communication.Communicator()
 
     def test_translate(self):
-        assert self.comm._translate("  '' None") == '""null'
+        assert self.comm._translate('None') == 'null'
 
     def test_api_url(self):
         assert self.comm.api_url('an_endpoint') == rest_url('an_endpoint')
-        exp = '?where={"this":"that"}&embedded={"things":1}&aggregate=True&sort=-_created'
-        obs = self.comm.api_url(
-            'an_endpoint',
-            where={'this': 'that'},
-            embedded={'things': 1},
-            aggregate=True,
-            sort='-_created'
-        ).replace(rest_url('an_endpoint'), '')
-        assert sorted(obs.lstrip('?').split('&')) == sorted(exp.lstrip('?').split('&'))
 
     def test_parse_query_string(self):
-        query_string = 'http://a_url?this=that&other={"another":"more"}'
+        query_string = 'http://a_url?this=that&other={"another":"more"}&things=1'
         no_query_string = 'http://a_url'
         dodgy_query_string = 'http://a_url?this=that?other=another'
 
         p = self.comm._parse_query_string
 
-        assert p(query_string) == {'this': 'that', 'other': '{"another":"more"}'}
+        assert p(query_string) == {'this': 'that', 'other': '{"another":"more"}', 'things': '1'}
         assert p(no_query_string) == {}
 
         with pytest.raises(RestCommunicationError) as e:
             p(dodgy_query_string)
             assert str(e) == 'Bad query string: ' + dodgy_query_string
 
         with pytest.raises(RestCommunicationError) as e2:
-            p(query_string, requires=['things'])
-            assert str(e2) == query_string + ' did not contain all required fields: ' + str(['things'])
+            p(query_string, requires=['thangs'])
+            assert str(e2) == query_string + " did not contain all required fields: ['thangs']"
 
     @patched_response
     def test_req(self, mocked_response):
@@ -98,19 +89,26 @@ def test_get_documents_depaginate(self):
                 'this', 'that', 'other', 'another', 'more', 'things'
             ]
             assert all([a[0][1].startswith(rest_url('an_endpoint')) for a in mocked_req.call_args_list])
-            assert [query_args_from_url(a[0][1]) for a in mocked_req.call_args_list] == [
-                {'page': '1', 'max_results': '101'},
-                {'page': '2', 'max_results': '101'},
-                {'page': '3', 'max_results': '101'}
+            assert [a[1] for a in mocked_req.call_args_list] == [
+                # Communicator.get_content passes ints
+                {'params': {'page': 1, 'max_results': 101}, 'quiet': False},
+                # url parsing passes strings, but requests removes the quotes anyway
+                {'params': {'page': '2', 'max_results': '101'}, 'quiet': False},
+                {'params': {'page': '3', 'max_results': '101'}, 'quiet': False}
             ]
 
     @patched_response
     def test_get_content(self, mocked_response):
         data = self.comm.get_content(test_endpoint, max_results=100, where={'a_field': 'thing'})
         assert data == test_request_content
         assert mocked_response.call_args[0][1].startswith(rest_url(test_endpoint))
-        assert query_args_from_url(mocked_response.call_args[0][1]) == {
-            'max_results': '100', 'where': {'a_field': 'thing'}, 'page': '1'
+        assert mocked_response.call_args[1] == {
+            'auth': ('a_user', 'a_password'),
+            'params': {
+                'max_results': 100,
+                'where': '{"a_field": "thing"}',
+                'page': 1
+            }
         }
 
     def test_get_documents(self):