From 85cc9881ceb88c012cb5e968cba385cdc395c9c1 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Fri, 5 Feb 2021 10:40:39 +0400 Subject: [PATCH] Review fixes: * config params * string formats * lint fixes --- moira_client/models/common.py | 18 +++++++++ moira_client/models/config.py | 24 ++++++----- moira_client/models/contact.py | 6 +-- moira_client/models/event.py | 4 +- moira_client/models/health.py | 2 +- moira_client/models/notification.py | 2 +- moira_client/models/pattern.py | 2 +- moira_client/models/subscription.py | 25 ++++-------- moira_client/models/tag.py | 2 +- moira_client/models/trigger.py | 62 ++++++++++++----------------- moira_client/models/user.py | 6 +-- moira_client/moira.py | 2 +- tests/models/test_config.py | 2 +- 13 files changed, 79 insertions(+), 78 deletions(-) create mode 100644 moira_client/models/common.py diff --git a/moira_client/models/common.py b/moira_client/models/common.py new file mode 100644 index 0000000..64e39da --- /dev/null +++ b/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, + } diff --git a/moira_client/models/config.py b/moira_client/models/config.py index 36b741c..044e683 100644 --- a/moira_client/models/config.py +++ b/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' diff --git a/moira_client/models/contact.py b/moira_client/models/contact.py index 7d0cdbe..6f38407 100644 --- a/moira_client/models/contact.py +++ b/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' diff --git a/moira_client/models/event.py b/moira_client/models/event.py index c1b9400..7c6492f 100644 --- a/moira_client/models/event.py +++ b/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' diff --git a/moira_client/models/health.py b/moira_client/models/health.py index 5478a3f..61bf691 100644 --- a/moira_client/models/health.py +++ b/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' diff --git a/moira_client/models/notification.py b/moira_client/models/notification.py index 239f6a6..e9113ba 100644 --- a/moira_client/models/notification.py +++ b/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' diff --git a/moira_client/models/pattern.py b/moira_client/models/pattern.py index 0d5d3f6..8174ec2 100644 --- a/moira_client/models/pattern.py +++ b/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' diff --git a/moira_client/models/subscription.py b/moira_client/models/subscription.py index b097269..7d89278 100644 --- a/moira_client/models/subscription.py +++ b/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' diff --git a/moira_client/models/tag.py b/moira_client/models/tag.py index fbe61db..135189f 100644 --- a/moira_client/models/tag.py +++ b/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' diff --git a/moira_client/models/trigger.py b/moira_client/models/trigger.py index 2e1deed..0ea1fbf 100644 --- a/moira_client/models/trigger.py +++ b/moira_client/models/trigger.py @@ -1,9 +1,7 @@ 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' @@ -11,8 +9,6 @@ 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' diff --git a/moira_client/models/user.py b/moira_client/models/user.py index f71ddc2..8bb4f41 100644 --- a/moira_client/models/user.py +++ b/moira_client/models/user.py @@ -35,7 +35,7 @@ def get_user_settings(self): :raises: ResponseStructureError """ - result = self._client.get(self._full_path() + '/settings') + result = self._client.get(self._full_path('settings')) required = ['login', 'contacts', 'subscriptions'] for field in required: if field not in result: @@ -55,5 +55,5 @@ def get_user_settings(self): def _full_path(self, path=''): if path: - return 'user/' + path - return 'user' \ No newline at end of file + return 'user/{}'.format(path) + return 'user' diff --git a/moira_client/moira.py b/moira_client/moira.py index 99f1a19..8c18c9f 100644 --- a/moira_client/moira.py +++ b/moira_client/moira.py @@ -144,4 +144,4 @@ def user(self): """ if not self._user: self._user = UserManager(self._client) - return self._user \ No newline at end of file + return self._user diff --git a/tests/models/test_config.py b/tests/models/test_config.py index 766a765..4edf6b5 100644 --- a/tests/models/test_config.py +++ b/tests/models/test_config.py @@ -8,7 +8,7 @@ from moira_client.client import Client from moira_client.client import ResponseStructureError from .test_model import ModelTest -from moira_client.models.config import ConfigManager, WebContact +from moira_client.models.config import ConfigManager class ConfigTest(ModelTest):