Review fixes:

 * config params
 * string formats
 * lint fixes

moira_client/models/common.py

@@ -0,0 +1,18 @@
+DAYS_OF_WEEK = ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun']
+
+MINUTES_IN_HOUR = 60
+
+
+def get_schedule(start_hour, start_minute, end_hour, end_minute, disabled_days):
+    days = []
+    for day in DAYS_OF_WEEK:
+        day_info = {
+            'enabled': True if day not in disabled_days else False,
+            'name': day
+        }
+        days.append(day_info)
+    return {
+        'days': days,
+        'startOffset': start_hour * MINUTES_IN_HOUR + start_minute,
+        'endOffset': end_hour * MINUTES_IN_HOUR + end_minute,
+    }

moira_client/models/config.py

@@ -2,19 +2,19 @@
 
 
 class WebContact:
-    def __init__(self, _type, label, **kwargs):
+    def __init__(self, _type, label, validation=None, placeholder=None, _help=None):
         self.type = _type
         self.label = label
-        self.validation = kwargs.get('validation', None)
-        self.placeholder = kwargs.get('placeholder', None)
-        self.help = kwargs.get('help', None)
+        self.validation = validation
+        self.placeholder = placeholder
+        self.help = _help
 
 
 class Config:
-    def __init__(self, remote_allowed, contacts, **kwargs):
+    def __init__(self, remote_allowed, contacts, support_email=None):
         self.remoteAllowed = remote_allowed
         self.contacts = contacts
-        self.supportEmail = kwargs.get('supportEmail', None)
+        self.supportEmail = support_email
 
 
 class ConfigManager:
@@ -39,9 +39,13 @@ def fetch(self):
         contacts = []
         for contact in result['contacts']:
             if self._validate_contact(contact):
-                contacts.append(WebContact(_type=contact['type'], **contact))
-        result['contacts'] = contacts
-        return Config(result['remoteAllowed'], **result)
+                _help = contact['help'] if 'help' in contact else None
+                validation = contact['validation'] if 'validation' in contact else None
+                placeholder = contact['placeholder'] if 'placeholder' in contact else None
+                contacts.append(WebContact(_type=contact['type'], label=contact['label'], validation=validation,
+                                           placeholder=placeholder, _help=_help))
+        support = result['supportEmail'] if 'supportEmail' in result else None
+        return Config(remote_allowed=result['remoteAllowed'], contacts=contacts, support_email=support)
 
     def _validate_contact(self, contact):
         if 'type' not in contact:
@@ -52,5 +56,5 @@ def _validate_contact(self, contact):
 
     def _full_path(self, path=''):
         if path:
-            return 'config/' + path
+            return 'config/{}'.format(path)
         return 'config'

moira_client/models/contact.py

@@ -151,15 +151,15 @@ def test(self, contact_id):
         """
 
         try:
-            self._client.post(self._full_path(contact_id)+"/test")
+            self._client.post(self._full_path('{id}/test'.format(id=contact_id)))
             return False
         except InvalidJSONError as e:
-            if e.content == b'':  # successfully if response is blank
+            if len(e.content) == 0:  # successfully if response is blank
                 return True
             else:
                 return False
 
     def _full_path(self, path=''):
         if path:
-            return 'contact/' + path
+            return 'contact/{}'.format(path)
         return 'contact'

moira_client/models/event.py

@@ -38,7 +38,7 @@ def delete_all(self):
         :return: True on success, False otherwise
         """
         try:
-            result = self._client.delete(self._full_path("/all"))
+            result = self._client.delete(self._full_path("all"))
             return False
         except InvalidJSONError as e:
             if e.content == b'':  # successfully if response is blank
@@ -47,5 +47,5 @@ def delete_all(self):
 
     def _full_path(self, path=''):
         if path:
-            return 'event/' + path
+            return 'event/{}'.format(path)
         return 'event'

moira_client/models/health.py

@@ -58,5 +58,5 @@ def enable_notifications(self):
 
     def _full_path(self, path=''):
         if path:
-            return 'health/' + path
+            return 'health/{}'.format(path)
         return 'health'

moira_client/models/notification.py

@@ -71,5 +71,5 @@ def delete(self, notification_id):
 
     def _full_path(self, path=''):
         if path:
-            return 'notification/' + path
+            return 'notification/{}'.format(path)
         return 'notification'

moira_client/models/pattern.py

@@ -52,5 +52,5 @@ def delete(self, pattern):
 
     def _full_path(self, path=''):
         if path:
-            return 'pattern/' + path
+            return 'pattern/{}'.format(path)
         return 'pattern'

moira_client/models/subscription.py

@@ -1,10 +1,7 @@
 from ..client import InvalidJSONError
 from ..client import ResponseStructureError
 from .base import Base
-
-DAYS_OF_WEEK = ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun']
-
-MINUTES_IN_HOUR = 60
+from .common import MINUTES_IN_HOUR, get_schedule
 
 
 class Subscription(Base):
@@ -76,19 +73,11 @@ def _send_request(self, subscription_id=None):
         if subscription_id:
             data['id'] = subscription_id
 
-        data['sched']['days'] = []
-        for day in DAYS_OF_WEEK:
-            day_info = {
-                'enabled': True if day not in self.disabled_days else False,
-                'name': day
-            }
-            data['sched']['days'].append(day_info)
-
-        data['sched']['startOffset'] = self._start_hour * MINUTES_IN_HOUR + self._start_minute
-        data['sched']['endOffset'] = self._end_hour * MINUTES_IN_HOUR + self._end_minute
+        data['sched'] = get_schedule(self._start_hour, self._start_minute, self._end_hour, self._end_minute,
+                                     self.disabled_days)
 
         if subscription_id:
-            result = self._client.put('subscription/' + subscription_id, json=data)
+            result = self._client.put('subscription/{id}'.format(id=subscription_id), json=data)
         else:
             result = self._client.put('subscription', json=data)
         if 'id' not in result:
@@ -255,7 +244,7 @@ def is_exist(self, **kwargs):
                         equal = False
                         break
                 except Exception:
-                    raise ValueError('Wrong attibute "{}"'.format(attr))
+                    raise ValueError('Wrong attribute "{}"'.format(attr))
             if equal:
                 return True
         return False
@@ -313,7 +302,7 @@ def test(self, subscription_id):
         :return: True on success, False otherwise
         """
         try:
-            self._client.put(self._full_path(subscription_id) + "/test")
+            self._client.put(self._full_path('{id}/test'.format(id=subscription_id)))
             return False
         except InvalidJSONError as e:
             if e.content == b'':  # successfully if response is blank
@@ -322,5 +311,5 @@ def test(self, subscription_id):
 
     def _full_path(self, path=''):
         if path:
-            return 'subscription/' + path
+            return 'subscription/{}'.format(path)
         return 'subscription'

moira_client/models/tag.py

@@ -133,5 +133,5 @@ def fetch_assigned_subscriptions(self, tag):
 
     def _full_path(self, path=''):
         if path:
-            return 'tag/' + path
+            return 'tag/{}'.format(path)
         return 'tag'

moira_client/models/trigger.py

@@ -1,18 +1,14 @@
 from ..client import ResponseStructureError
 from ..client import InvalidJSONError
 from .base import Base
-
-
-DAYS_OF_WEEK = ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun']
+from .common import MINUTES_IN_HOUR, get_schedule
 
 STATE_OK = 'OK'
 STATE_WARN = 'WARN'
 STATE_ERROR = 'ERROR'
 STATE_NODATA = 'NODATA'
 STATE_EXCEPTION = 'EXCEPTION'
 
-MINUTES_IN_HOUR = 60
-
 RISING_TRIGGER = 'rising'
 FALLING_TRIGGER = 'falling'
 EXPRESSION_TRIGGER = 'expression'
@@ -172,19 +168,11 @@ def _send_request(self, trigger_id=None):
             api_response = TriggerManager(
                 self._client).fetch_by_id(trigger_id)
 
-        data['sched']['days'] = []
-        for day in DAYS_OF_WEEK:
-            day_info = {
-                'enabled': True if day not in self.disabled_days else False,
-                'name': day
-            }
-            data['sched']['days'].append(day_info)
-
-        data['sched']['startOffset'] = self._start_hour * MINUTES_IN_HOUR + self._start_minute
-        data['sched']['endOffset'] = self._end_hour * MINUTES_IN_HOUR + self._end_minute
+        data['sched'] = get_schedule(self._start_hour, self._start_minute, self._end_hour, self._end_minute,
+                                     self.disabled_days)
 
         if trigger_id and api_response:
-            res = self._client.put('trigger/' + trigger_id, json=data)
+            res = self._client.put('trigger/{id}'.format(id=trigger_id), json=data)
         else:
             res = self._client.put('trigger', json=data)
         if 'id' not in res:
@@ -263,24 +251,24 @@ def check_exists(self):
         for trigger in trigger_manager.fetch_all():
             if self.name == trigger.name and \
                 set(self.targets) == set(trigger.targets) and \
-                set(self.tags) == set(trigger.tags):
+                    set(self.tags) == set(trigger.tags):
                 return trigger
 
-    def get_metrics(self, _from, to):
+    def get_metrics(self, start, end):
         """
         Get metrics associated with certain trigger
 
-        :param _from: The start period of metrics to get. Example : -1hour
-        :param to: The end period of metrics to get. Example : now
+        :param start: The start period of metrics to get. Example : -1hour
+        :param end: The end period of metrics to get. Example : now
 
         :return: Metrics for trigger
         """
         try:
             params = {
-                'from': _from,
-                'to': to,
+                'from': start,
+                'to': end,
             }
-            result = self._client.get('trigger/' + self.id + '/metrics', params=params)
+            result = self._client.get('trigger/{id}/metrics'.format(id=self.id), params=params)
             return result
         except InvalidJSONError:
             return []
@@ -297,19 +285,20 @@ def delete_metric(self, metric_name):
             params = {
                 'name': metric_name,
             }
-            self._client.delete('trigger/' + self.id + '/metrics', params=params)
+            self._client.delete('trigger/{id}/metrics'.format(id=self.id), params=params)
             return True
         except InvalidJSONError:
             return False
 
     def delete_nodata_metrics(self):
         """
-        Deletes all metrics from last data which are in NODATA state. It also deletes all trigger patterns of those metrics.
+        Deletes all metrics from last data which are in NODATA state.
+        It also deletes all trigger patterns of those metrics.
 
         :return: True if success, False otherwise
         """
         try:
-            self._client.delete('trigger/' + self.id + '/metrics/nodata')
+            self._client.delete('trigger/{id}/metrics/nodata'.format(id=self.id))
             return True
         except InvalidJSONError:
             return False
@@ -349,7 +338,7 @@ def fetch_by_id(self, trigger_id):
 
         :raises: ResponseStructureError
         """
-        result = self._client.get(self._full_path(trigger_id + '/state'))
+        result = self._client.get(self._full_path('{id}/state'.format(id=trigger_id)))
         if 'state' in result:
             trigger = self._client.get(self._full_path(trigger_id))
             return Trigger(self._client, **trigger)
@@ -400,7 +389,7 @@ def get_throttling(self, trigger_id):
         :return: trigger throttle value or None
         """
         try:
-            result = self._client.get(self._full_path(trigger_id + '/throttling'))
+            result = self._client.get(self._full_path('{id}/throttling'.format(id=trigger_id)))
             if 'throttling' in result:
                 return result['throttling']
             return None
@@ -415,7 +404,7 @@ def reset_throttling(self, trigger_id):
         :return: True if reset, False otherwise
         """
         try:
-            self._client.delete(self._full_path(trigger_id + '/throttling'))
+            self._client.delete(self._full_path('{id}/throttling'.format(id=trigger_id)))
             return True
         except InvalidJSONError:
             return False
@@ -427,12 +416,13 @@ def get_state(self, trigger_id):
         :param trigger_id: str trigger id
         :return: state of trigger
         """
-        return self._client.get(self._full_path(trigger_id + '/state'))
+        return self._client.get(self._full_path('{id}/state'.format(id=trigger_id)))
 
     def get_metrics(self, trigger_id, _from, to):
         """
         Get metrics associated with certain trigger
 
+        :param trigger_id: str trigger id
         :param _from: The start period of metrics to get. Example : -1hour
         :param to: The end period of metrics to get. Example : now
 
@@ -443,7 +433,7 @@ def get_metrics(self, trigger_id, _from, to):
                 'from': _from,
                 'to': to,
             }
-            result = self._client.get('trigger/' + trigger_id + '/metrics', params=params)
+            result = self._client.get(self._full_path('{id}/metrics'.format(id=trigger_id)), params=params)
             return result
         except InvalidJSONError:
             return []
@@ -460,7 +450,7 @@ def remove_metric(self, trigger_id, metric):
             params = {
                 'name': metric
             }
-            self._client.delete(self._full_path(trigger_id + '/metrics'), params=params)
+            self._client.delete(self._full_path('{id}/metrics'.format(id=trigger_id)), params=params)
             return True
         except InvalidJSONError:
             return False
@@ -473,7 +463,7 @@ def remove_nodata_metrics(self, trigger_id):
         :return: True if removed, False otherwise
         """
         try:
-            self._client.delete(self._full_path(trigger_id + '/metrics/nodata'))
+            self._client.delete(self._full_path('{id}/metrics/nodata'.format(id=trigger_id)))
             return True
         except InvalidJSONError:
             return False
@@ -488,7 +478,7 @@ def is_exist(self, trigger):
         for moira_trigger in self.fetch_all():
             if trigger.name == moira_trigger.name and \
                 set(trigger.targets) == set(moira_trigger.targets) and \
-                set(trigger.tags) == set(moira_trigger.tags):
+                    set(trigger.tags) == set(moira_trigger.tags):
                 return True
         return False
 
@@ -529,7 +519,7 @@ def set_maintenance(self, trigger_id, end_time, metrics=None):
             }
             if metrics is not None:
                 data['metrics'] = metrics
-            self._client.put(self._full_path(trigger_id + '/setMaintenance'), json=data)
+            self._client.put(self._full_path('{id}/setMaintenance'.format(id=trigger_id)), json=data)
             return True
         except InvalidJSONError:
             return False
@@ -592,5 +582,5 @@ def create(
 
     def _full_path(self, path=''):
         if path:
-            return 'trigger/' + path
+            return 'trigger/{}'.format(path)
         return 'trigger'