From d396753a839c1317a59fb2867dc2188fc88cf95a Mon Sep 17 00:00:00 2001 From: Jui Chandwaskar Date: Wed, 11 Apr 2018 14:15:08 +0200 Subject: [PATCH] Update pep8 checks * Set max line length to 100 * Clean up code for pep8 checks Change-Id: Ie00dc204f522fb2112f02f4151ec8a15d5523459 Signed-off-by: Jui Chandwaskar --- .../common/repositories/base/base_repo.py | 19 +++-- .../common/repositories/mysql/mysql_repo.py | 5 +- .../common/repositories/orm/orm_repo.py | 6 +- .../repositories/postgres/pgsql_repo.py | 5 +- monasca_notification/main.py | 6 +- .../plugins/email_notifier.py | 6 +- .../plugins/hipchat_notifier.py | 30 +++---- monasca_notification/plugins/jira_notifier.py | 11 ++- .../processors/alarm_processor.py | 9 ++- tests/test_alarm_processor.py | 78 ++++++++++++++----- tests/test_email_notification.py | 18 +++-- tests/test_notification_processor.py | 18 ++++- tests/test_webhook_notification.py | 15 +++- tox.ini | 2 +- 14 files changed, 161 insertions(+), 67 deletions(-) diff --git a/monasca_notification/common/repositories/base/base_repo.py b/monasca_notification/common/repositories/base/base_repo.py index eed94bf..8dc86e5 100644 --- a/monasca_notification/common/repositories/base/base_repo.py +++ b/monasca_notification/common/repositories/base/base_repo.py @@ -14,14 +14,17 @@ class BaseRepo(object): def __init__(self, config): - self._find_alarm_action_sql = """SELECT id, type, name, address, period - FROM alarm_action as aa - JOIN notification_method as nm ON aa.action_id = nm.id - WHERE aa.alarm_definition_id = %s and aa.alarm_state = %s""" - self._find_alarm_state_sql = """SELECT state - FROM alarm - WHERE alarm.id = %s""" - self._insert_notification_types_sql = """INSERT INTO notification_method_type (name) VALUES ( %s)""" + self._find_alarm_action_sql = \ + """SELECT id, type, name, address, period + FROM alarm_action as aa + JOIN notification_method as nm ON aa.action_id = nm.id + WHERE aa.alarm_definition_id = %s and aa.alarm_state = %s""" + self._find_alarm_state_sql = \ + """SELECT state + FROM alarm + WHERE alarm.id = %s""" + self._insert_notification_types_sql = \ + """INSERT INTO notification_method_type (name) VALUES ( %s)""" self._find_all_notification_types_sql = """SELECT name from notification_method_type """ self._get_notification_sql = """SELECT name, type, address, period FROM notification_method diff --git a/monasca_notification/common/repositories/mysql/mysql_repo.py b/monasca_notification/common/repositories/mysql/mysql_repo.py index eb4a926..c23ba5d 100644 --- a/monasca_notification/common/repositories/mysql/mysql_repo.py +++ b/monasca_notification/common/repositories/mysql/mysql_repo.py @@ -53,7 +53,10 @@ class MysqlRepo(base_repo.BaseRepo): if self._mysql is None: self._connect_to_mysql() cur = self._mysql.cursor() - cur.execute(self._find_alarm_action_sql, (alarm['alarmDefinitionId'], alarm['newState'])) + cur.execute( + self._find_alarm_action_sql, + (alarm['alarmDefinitionId'], + alarm['newState'])) for row in cur: yield (row[0], row[1].lower(), row[2], row[3], row[4]) diff --git a/monasca_notification/common/repositories/orm/orm_repo.py b/monasca_notification/common/repositories/orm/orm_repo.py index cd76d21..697fa3f 100644 --- a/monasca_notification/common/repositories/orm/orm_repo.py +++ b/monasca_notification/common/repositories/orm/orm_repo.py @@ -115,7 +115,11 @@ class OrmRepo(object): if notification is None: return None else: - return [notification[0], notification[1].lower(), notification[2], notification[3]] + return [ + notification[0], + notification[1].lower(), + notification[2], + notification[3]] except DatabaseError as e: LOG.exception("Couldn't fetch the notification method %s", e) raise exc.DatabaseException(e) diff --git a/monasca_notification/common/repositories/postgres/pgsql_repo.py b/monasca_notification/common/repositories/postgres/pgsql_repo.py index 1ba1e7c..ccc030d 100644 --- a/monasca_notification/common/repositories/postgres/pgsql_repo.py +++ b/monasca_notification/common/repositories/postgres/pgsql_repo.py @@ -40,7 +40,10 @@ class PostgresqlRepo(base_repo.BaseRepo): if self._pgsql is None: self._connect_to_pgsql() cur = self._pgsql.cursor() - cur.execute(self._find_alarm_action_sql, (alarm['alarmDefinitionId'], alarm['newState'])) + cur.execute( + self._find_alarm_action_sql, + (alarm['alarmDefinitionId'], + alarm['newState'])) for row in cur: yield (row[0], row[1].lower(), row[2], row[3], row[4]) except psycopg2.Error as e: diff --git a/monasca_notification/main.py b/monasca_notification/main.py index 8f091a1..ce0ef32 100644 --- a/monasca_notification/main.py +++ b/monasca_notification/main.py @@ -15,7 +15,8 @@ # limitations under the License. """ Notification Engine - This engine reads alarms from Kafka and then notifies the customer using their configured notification method. + This engine reads alarms from Kafka and then notifies the customer using their configured + notification method. """ import multiprocessing @@ -58,7 +59,8 @@ def clean_exit(signum, frame=None): for process in processors: try: if process.is_alive(): - process.terminate() # Sends sigterm which any processes after a notification is sent attempt to handle + # Sends sigterm which any processes after a notification is sent attempt to handle + process.terminate() wait_for_exit = True except Exception: # nosec # There is really nothing to do if the kill fails, so just go on. diff --git a/monasca_notification/plugins/email_notifier.py b/monasca_notification/plugins/email_notifier.py index 571716f..acb499f 100644 --- a/monasca_notification/plugins/email_notifier.py +++ b/monasca_notification/plugins/email_notifier.py @@ -179,9 +179,11 @@ class EmailNotifier(abstract_notifier.AbstractNotifier): def _create_msg(self, hostname, notification, targethost=None): """Create two kind of messages: - 1. Notifications that include metrics with a hostname as a dimension. There may be more than one hostname. + 1. Notifications that include metrics with a hostname as a dimension. + There may be more than one hostname. We will only report the hostname if there is only one. - 2. Notifications that do not include metrics and therefore no hostname. Example: API initiated changes. + 2. Notifications that do not include metrics and therefore no hostname. + Example: API initiated changes. * A third notification type which include metrics but do not include a hostname will be treated as type #2. """ diff --git a/monasca_notification/plugins/hipchat_notifier.py b/monasca_notification/plugins/hipchat_notifier.py index d9a2ea4..a901bfb 100644 --- a/monasca_notification/plugins/hipchat_notifier.py +++ b/monasca_notification/plugins/hipchat_notifier.py @@ -26,19 +26,19 @@ from monasca_notification.plugins import abstract_notifier CONF = cfg.CONF """ - notification.address = https://hipchat.hpcloud.net/v2/room//notification?auth_token=432432 +notification.address = https://hipchat.hpcloud.net/v2/room//notification?auth_token=432432 - How to get access token? - 1) Login to Hipchat with the user account which is used for notification - 2) Go to this page. https://hipchat.hpcloud.net/account/api (Replace your hipchat server name) - 3) You can see option to "Create token". Use the capability "SendNotification" +How to get access token? + 1) Login to Hipchat with the user account which is used for notification + 2) Go to this page. https://hipchat.hpcloud.net/account/api (Replace your hipchat server name) + 3) You can see option to "Create token". Use the capability "SendNotification" - How to get the Room ID? - 1) Login to Hipchat with the user account which is used for notification - 2) Go to this page. https://hipchat.hpcloud.net/account/api (Replace your hipchat server name) - 3) Click on the Rooms tab - 4) Click on any Room of your choice. - 5) Room ID is the API ID field +How to get the Room ID? + 1) Login to Hipchat with the user account which is used for notification + 2) Go to this page. https://hipchat.hpcloud.net/account/api (Replace your hipchat server name) + 3) Click on the Rooms tab + 4) Click on any Room of your choice. + 5) Room ID is the API ID field """ @@ -115,7 +115,10 @@ class HipChatNotifier(abstract_notifier.AbstractNotifier): query_params = urllib.parse.parse_qs(parsed_url.query) # URL without query params - url = urllib.parse.urljoin(notification.address, urllib.parse.urlparse(notification.address).path) + url = urllib.parse.urljoin( + notification.address, + urllib.parse.urlparse( + notification.address).path) # Default option is to do cert verification verify = not CONF.hipchat_notifier.insecure @@ -143,7 +146,8 @@ class HipChatNotifier(abstract_notifier.AbstractNotifier): self._log.info("Notification successfully posted.") return True else: - msg = "Received an HTTP code {} when trying to send to hipchat on URL {} with response {}." + msg = ("Received an HTTP code {} when trying to send to hipchat on URL {}" + " with response {}.") self._log.error(msg.format(result.status_code, url, result.text)) return False except Exception: diff --git a/monasca_notification/plugins/jira_notifier.py b/monasca_notification/plugins/jira_notifier.py index 86c2341..0152368 100644 --- a/monasca_notification/plugins/jira_notifier.py +++ b/monasca_notification/plugins/jira_notifier.py @@ -147,7 +147,8 @@ class JiraNotifier(AbstractNotifier): 'metrics': notification.metrics} jira_fields = {} - summary_format_string = "Monasca alarm for alarm_defintion {0} status changed to {1} for the alarm_id {2}" + summary_format_string = ("Monasca alarm for alarm_defintion {0} status changed to {1} " + "for the alarm_id {2}") jira_fields["summary"] = summary_format_string.format(notification.alarm_name, notification.state, notification.alarm_id) @@ -173,7 +174,10 @@ class JiraNotifier(AbstractNotifier): parsed_url = urllib.parse.urlsplit(notification.address) query_params = urllib.parse.parse_qs(parsed_url.query) # URL without query params - url = urllib.parse.urljoin(notification.address, urllib.parse.urlparse(notification.address).path) + url = urllib.parse.urljoin( + notification.address, + urllib.parse.urlparse( + notification.address).path) jira_fields["project"] = query_params["project"][0] if query_params.get("component"): @@ -229,7 +233,8 @@ class JiraNotifier(AbstractNotifier): if current_state.lower() in ["resolved", "closed"]: # Open the the issue transitions = jira_obj.transitions(issue) - allowed_transistions = [(t['id'], t['name']) for t in transitions if "reopen" in t['name'].lower()] + allowed_transistions = [(t['id'], t['name']) + for t in transitions if "reopen" in t['name'].lower()] if allowed_transistions: # Reopen the issue jira_obj.transition_issue(issue, allowed_transistions[0][0]) diff --git a/monasca_notification/processors/alarm_processor.py b/monasca_notification/processors/alarm_processor.py index 20ac84a..bb58108 100644 --- a/monasca_notification/processors/alarm_processor.py +++ b/monasca_notification/processors/alarm_processor.py @@ -113,7 +113,9 @@ class AlarmProcessor(object): alarm = self._parse_alarm(raw_alarm[1].message.value) except Exception as e: # This is general because of a lack of json exception base class failed_parse_count += 1 - log.exception("Invalid Alarm format skipping partition %d, offset %d\nError%s" % (partition, offset, e)) + log.exception( + "Invalid Alarm format skipping partition %d, offset %d\nError%s" % + (partition, offset, e)) return [], partition, offset log.debug("Read alarm from alarms sent_queue. Partition %d, Offset %d, alarm data %s" @@ -131,8 +133,9 @@ class AlarmProcessor(object): if len(notifications) == 0: no_notification_count += 1 - log.debug('No notifications found for this alarm, partition %d, offset %d, alarm data %s' - % (partition, offset, alarm)) + log.debug( + 'No notifications found for this alarm, partition %d, offset %d, alarm data %s' % + (partition, offset, alarm)) return [], partition, offset else: log.debug('Found %d notifications: [%s]', len(notifications), notifications) diff --git a/tests/test_alarm_processor.py b/tests/test_alarm_processor.py index c05691b..dba5f17 100644 --- a/tests/test_alarm_processor.py +++ b/tests/test_alarm_processor.py @@ -45,9 +45,11 @@ class TestAlarmProcessor(base.BaseTestCase): @mock.patch('pymysql.connect') @mock.patch('monasca_notification.processors.alarm_processor.log') def _run_alarm_processor(self, alarm, sql_response, mock_log, mock_mysql): - """Runs a mocked alarm processor reading from queue while running, returns (queue_message, log_message) + """Runs a mocked alarm processor reading from queue while running, + returns (queue_message, log_message) """ - # Since the log runs in another thread I can mock it directly, instead change the methods to put to a queue + # Since the log runs in another thread I can mock it directly, instead + # change the methods to put to a queue mock_log.warn = self.trap.append mock_log.error = self.trap.append mock_log.exception = self.trap.append @@ -84,10 +86,20 @@ class TestAlarmProcessor(base.BaseTestCase): def test_old_timestamp(self): """Should cause the alarm_ttl to fire log a warning and push to finished queue.""" timestamp = 1375346830042 - alarm_dict = {"tenantId": "0", "alarmDefinitionId": "0", "alarmId": "1", "alarmName": "test Alarm", - "oldState": "OK", "newState": "ALARM", "stateChangeReason": "I am alarming!", - "timestamp": timestamp, "actionsEnabled": 1, "metrics": "cpu_util", - "severity": "LOW", "link": "http://some-place.com", "lifecycleState": "OPEN"} + alarm_dict = { + "tenantId": "0", + "alarmDefinitionId": "0", + "alarmId": "1", + "alarmName": "test Alarm", + "oldState": "OK", + "newState": "ALARM", + "stateChangeReason": "I am alarming!", + "timestamp": timestamp, + "actionsEnabled": 1, + "metrics": "cpu_util", + "severity": "LOW", + "link": "http://some-place.com", + "lifecycleState": "OPEN"} alarm = self._create_raw_alarm(0, 2, alarm_dict) expected_datetime = time.ctime(timestamp / 1000) @@ -105,10 +117,20 @@ class TestAlarmProcessor(base.BaseTestCase): def test_no_notifications(self): """Test an alarm with no defined notifications """ - alarm_dict = {"tenantId": "0", "alarmDefinitionId": "0", "alarmId": "1", "alarmName": "test Alarm", - "oldState": "OK", "newState": "ALARM", "stateChangeReason": "I am alarming!", - "timestamp": time.time() * 1000, "actionsEnabled": 1, "metrics": "cpu_util", - "severity": "LOW", "link": "http://some-place.com", "lifecycleState": "OPEN"} + alarm_dict = { + "tenantId": "0", + "alarmDefinitionId": "0", + "alarmId": "1", + "alarmName": "test Alarm", + "oldState": "OK", + "newState": "ALARM", + "stateChangeReason": "I am alarming!", + "timestamp": time.time() * 1000, + "actionsEnabled": 1, + "metrics": "cpu_util", + "severity": "LOW", + "link": "http://some-place.com", + "lifecycleState": "OPEN"} alarm = self._create_raw_alarm(0, 3, alarm_dict) notifications, partition, offset = self._run_alarm_processor(alarm, None) @@ -120,10 +142,20 @@ class TestAlarmProcessor(base.BaseTestCase): def test_valid_notification(self): """Test a valid notification, being put onto the notification_queue """ - alarm_dict = {"tenantId": "0", "alarmDefinitionId": "0", "alarmId": "1", "alarmName": "test Alarm", - "oldState": "OK", "newState": "ALARM", "stateChangeReason": "I am alarming!", - "timestamp": time.time() * 1000, "actionsEnabled": 1, "metrics": "cpu_util", - "severity": "LOW", "link": "http://some-place.com", "lifecycleState": "OPEN"} + alarm_dict = { + "tenantId": "0", + "alarmDefinitionId": "0", + "alarmId": "1", + "alarmName": "test Alarm", + "oldState": "OK", + "newState": "ALARM", + "stateChangeReason": "I am alarming!", + "timestamp": time.time() * 1000, + "actionsEnabled": 1, + "metrics": "cpu_util", + "severity": "LOW", + "link": "http://some-place.com", + "lifecycleState": "OPEN"} alarm = self._create_raw_alarm(0, 4, alarm_dict) sql_response = [[1, 'EMAIL', 'test notification', 'me@here.com', 0]] @@ -137,10 +169,20 @@ class TestAlarmProcessor(base.BaseTestCase): self.assertEqual(offset, 4) def test_two_valid_notifications(self): - alarm_dict = {"tenantId": "0", "alarmDefinitionId": "0", "alarmId": "1", "alarmName": "test Alarm", - "oldState": "OK", "newState": "ALARM", "stateChangeReason": "I am alarming!", - "timestamp": time.time() * 1000, "actionsEnabled": 1, "metrics": "cpu_util", - "severity": "LOW", "link": "http://some-place.com", "lifecycleState": "OPEN"} + alarm_dict = { + "tenantId": "0", + "alarmDefinitionId": "0", + "alarmId": "1", + "alarmName": "test Alarm", + "oldState": "OK", + "newState": "ALARM", + "stateChangeReason": "I am alarming!", + "timestamp": time.time() * 1000, + "actionsEnabled": 1, + "metrics": "cpu_util", + "severity": "LOW", + "link": "http://some-place.com", + "lifecycleState": "OPEN"} alarm = self._create_raw_alarm(0, 5, alarm_dict) diff --git a/tests/test_email_notification.py b/tests/test_email_notification.py index 6833693..f108a8b 100644 --- a/tests/test_email_notification.py +++ b/tests/test_email_notification.py @@ -143,7 +143,8 @@ class TestEmail(base.PluginTestCase): alarm_dict = alarm(metric) - notification = Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm_dict) self.trap.append(email.send_notification(notification)) @@ -267,7 +268,8 @@ class TestEmail(base.PluginTestCase): alarm_dict = alarm(metrics) - notification = Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm_dict) self.trap.append(email.send_notification(notification)) @@ -313,7 +315,8 @@ class TestEmail(base.PluginTestCase): alarm_dict = alarm(metrics) - notification = Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm_dict) email_result = email.send_notification(notification) @@ -355,7 +358,8 @@ class TestEmail(base.PluginTestCase): alarm_dict = alarm(metrics) - notification = Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm_dict) self.trap.append(email.send_notification(notification)) @@ -395,7 +399,8 @@ class TestEmail(base.PluginTestCase): alarm_dict = alarm(metrics) - notification = Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm_dict) self.trap.append(email.send_notification(notification)) @@ -435,7 +440,8 @@ class TestEmail(base.PluginTestCase): alarm_dict = alarm(metrics) - notification = Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm_dict) self.trap.append(email.send_notification(notification)) diff --git a/tests/test_notification_processor.py b/tests/test_notification_processor.py index 92fd009..8765bc6 100644 --- a/tests/test_notification_processor.py +++ b/tests/test_notification_processor.py @@ -93,7 +93,8 @@ class TestNotificationProcessor(base.BaseTestCase): "timestamp": time.time(), "metrics": metric} - notification = m_notification.Notification(0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) + notification = m_notification.Notification( + 0, 'email', 'email notification', 'me@here.com', 0, 0, alarm_dict) self._start_processor([notification]) @@ -104,9 +105,18 @@ class TestNotificationProcessor(base.BaseTestCase): def test_invalid_notification(self): """Verify invalid notification type is rejected. """ - alarm_dict = {"tenantId": "0", "alarmId": "0", "alarmName": "test Alarm", "oldState": "OK", "newState": "ALARM", - "stateChangeReason": "I am alarming!", "timestamp": time.time(), "metrics": "cpu_util", - "severity": "LOW", "link": "http://some-place.com", "lifecycleState": "OPEN"} + alarm_dict = { + "tenantId": "0", + "alarmId": "0", + "alarmName": "test Alarm", + "oldState": "OK", + "newState": "ALARM", + "stateChangeReason": "I am alarming!", + "timestamp": time.time(), + "metrics": "cpu_util", + "severity": "LOW", + "link": "http://some-place.com", + "lifecycleState": "OPEN"} invalid_notification = m_notification.Notification(0, 'invalid', 'test notification', 'me@here.com', 0, 0, alarm_dict) diff --git a/tests/test_webhook_notification.py b/tests/test_webhook_notification.py index cd386d1..d43f0ab 100644 --- a/tests/test_webhook_notification.py +++ b/tests/test_webhook_notification.py @@ -111,10 +111,17 @@ class TestWebhook(base.PluginTestCase): self.maxDiff = None # timestamp is in milliseconds while alarm_timestamp is in seconds self.assertEqual(json.loads(data), - {"metrics": [{"dimensions": {"hostname": "foo1", "service": "bar1"}}], "alarm_id": "0", - "state": "ALARM", "alarm_timestamp": 1429023453, "tenant_id": "0", - "old_state": "OK", "alarm_description": "test Alarm description", - "message": "I am alarming!", "alarm_definition_id": 0, "alarm_name": "test Alarm"}) + {"metrics": [{"dimensions": {"hostname": "foo1", + "service": "bar1"}}], + "alarm_id": "0", + "state": "ALARM", + "alarm_timestamp": 1429023453, + "tenant_id": "0", + "old_state": "OK", + "alarm_description": "test Alarm description", + "message": "I am alarming!", + "alarm_definition_id": 0, + "alarm_name": "test Alarm"}) self.assertEqual(headers, {'content-type': 'application/json'}) return_value = self.trap.get() diff --git a/tox.ini b/tox.ini index 85dc6ee..80792df 100644 --- a/tox.ini +++ b/tox.ini @@ -78,7 +78,7 @@ description = Generates an example of monasca-notification configuration file commands = oslo-config-generator --config-file={toxinidir}/config-generator/notification.conf [flake8] -max-line-length = 120 +max-line-length = 100 # TODO: ignored checks should be enabled in the future # H201 no 'except:' at least use 'except Exception:' # H202: assertRaises Exception too broad