)]}'
{"monasca_notification/processors/notification_processor.py":[{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"a84ded1e68b6d31a583618c0711faf6dd44bd20c","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            if invalid \u003e 0:"},{"line_number":58,"context_line":"                invalid_type_count.increment(invalid)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"            if len(sent) \u003d\u003d 0:  # All notifications failed"},{"line_number":61,"context_line":"                self._add_to_queue(self.finished_queue,"},{"line_number":62,"context_line":"                                   \u0027finished\u0027,"},{"line_number":63,"context_line":"                                   (notifications[0].src_partition, notifications[0].src_offset))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_18f0b4fc","line":60,"updated":"2015-01-20 19:41:57.000000000","message":"if len(sent):\n\nelse:","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"}],"monasca_notification/types/BaseNotifier.py":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":21,"context_line":"class BaseNotifier(object):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def __init__(self):"},{"line_number":24,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_208920ca","line":21,"updated":"2015-01-16 14:16:39.000000000","message":"Files for Python classes should be using snake case. For example\n\nbase_notifier.py\n\nAlso, wondering about using the convention BaseNotifier vs just AbstractNotifier or just Notifier.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":21,"context_line":"class BaseNotifier(object):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def __init__(self):"},{"line_number":24,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_1b900b4d","line":21,"in_reply_to":"3a961159_208920ca","updated":"2015-01-16 18:24:18.000000000","message":"AbstractNotifier makes sense to me.  In a lot of other places we call the base class BaseFoo so I was following that pattern.  I\u0027d guess that none of those are purely abstract though.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def type(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @abc.abstractmethod"},{"line_number":31,"context_line":"    def statsd_name(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_60540822","line":28,"updated":"2015-01-16 14:16:39.000000000","message":"Do you want to pass, or do you want to raise NotImplemented exception? As this class inherits from abstract base class, I would probably expect all methods to have a default implemention of raise NotImplemented. \n\nAlong those lines, this really isn\u0027t a base class, it is an abstract class or a class declaring an interface for which every member is expected to be implemented in a derived class.\n\nI would call it AbstractNotifier or just Notifier as it really isn\u0027t a base, it it pure abstract.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def type(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @abc.abstractmethod"},{"line_number":31,"context_line":"    def statsd_name(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_3b9b0774","line":28,"in_reply_to":"3a961159_60540822","updated":"2015-01-16 18:24:18.000000000","message":"The abstractmethod decorator on those functions raises a TypeError if you try to instantiate a class without defining those methods.  Would raising the exception directly in the class be better?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"}],"monasca_notification/types/Email.py":[{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"4242876010b87e64e6539d79840575ccaacb3c46","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            self.smtp \u003d smtp"},{"line_number":110,"context_line":"        except Exception:"},{"line_number":111,"context_line":"            self.log.exception(\"Unable to connect to email server.  Exiting.\")"},{"line_number":112,"context_line":"            sys.exit(1)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # ------------------------------------------------------------------------"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_94b5cc33","line":112,"updated":"2015-01-15 21:55:45.000000000","message":"This seems wrong. It causes the whole notification engine to die if the email server is down","commit_id":"1c68fcc52249dd584db8b2e9dfec499677d48085"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"a58aed950849a18f87248cc5735556f52f875d9d","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            self.smtp \u003d smtp"},{"line_number":110,"context_line":"        except Exception:"},{"line_number":111,"context_line":"            self.log.exception(\"Unable to connect to email server.  Exiting.\")"},{"line_number":112,"context_line":"            sys.exit(1)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # ------------------------------------------------------------------------"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_379fba03","line":112,"updated":"2015-01-15 22:26:07.000000000","message":"This seems really wrong. If the smtp server is down, then no pager duty or webhook notifications will go out. Besides, this is too low level of a method to have a sys.exit","commit_id":"4dbefc1165d8e236cd31964f4f64bc33f3f91739"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import sys"},{"line_number":19,"context_line":"import time"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from BaseNotifier import BaseNotifier"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class EmailNotifier(BaseNotifier):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_fbb34906","line":21,"updated":"2015-01-16 14:16:39.000000000","message":"Similar to another comment, I think this file should be named either email.py or email_notifier.py","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import sys"},{"line_number":19,"context_line":"import time"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from BaseNotifier import BaseNotifier"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class EmailNotifier(BaseNotifier):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_bbac5789","line":21,"in_reply_to":"3a961159_fbb34906","updated":"2015-01-16 18:24:18.000000000","message":"Naming it email would conflict with the import of the standard email library so it\u0027d have to be email_notifier.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":24,"context_line":"class EmailNotifier(BaseNotifier):"},{"line_number":25,"context_line":"    def __init__(self, log):"},{"line_number":26,"context_line":"        self.log \u003d log"},{"line_number":27,"context_line":"        self.smtp \u003d None"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # ------------------------------------------------------------------------"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_a018b084","line":27,"updated":"2015-01-16 14:16:39.000000000","message":"As these are private members (I think so) it might be good to get in the habit of declaring them as self._log or self._smtp, with the underscore prepended. You are following that convention on the methods so using that for member variables is a good idea too.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # ------------------------------------------------------------------------"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def config(self, config):"},{"line_number":32,"context_line":"        self.config \u003d config"},{"line_number":33,"context_line":"        self._smtp_connect()"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_3b77913c","line":31,"updated":"2015-01-16 14:16:39.000000000","message":"Just wondering why this method. Is this initialization expected to be done just once and could it be in the constructor?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    def send_notification(self, notification):"},{"line_number":48,"context_line":"        \"\"\"Send the notification via email"},{"line_number":49,"context_line":"             Returns the notification upon success, None upon failure"},{"line_number":50,"context_line":"        \"\"\""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        # Get the \"hostname\" from the notification metrics if there is one"},{"line_number":53,"context_line":"        hostname \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_fb40a907","line":50,"updated":"2015-01-16 14:16:39.000000000","message":"The comment says return the notification upon success, None ..., but the code returns True or False.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    def send_notification(self, notification):"},{"line_number":48,"context_line":"        \"\"\"Send the notification via email"},{"line_number":49,"context_line":"             Returns the notification upon success, None upon failure"},{"line_number":50,"context_line":"        \"\"\""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        # Get the \"hostname\" from the notification metrics if there is one"},{"line_number":53,"context_line":"        hostname \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_fbd09ff8","line":50,"in_reply_to":"3a961159_fb40a907","updated":"2015-01-16 18:24:18.000000000","message":"Ah, sorry that\u0027s an old comment from what it was returning before.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        except smtplib.SMTPServerDisconnected:"},{"line_number":68,"context_line":"            self.log.warn(\u0027SMTP server disconnected. \u0027"},{"line_number":69,"context_line":"                          \u0027Will reconnect and retry message.\u0027)"},{"line_number":70,"context_line":"            self._smtp_connect()"},{"line_number":71,"context_line":"        except smtplib.SMTPException:"},{"line_number":72,"context_line":"            self._email_error(notification)"},{"line_number":73,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_5b5e1d14","line":70,"updated":"2015-01-16 14:16:39.000000000","message":"All the other exception blocks return True or False, should this return False?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        except smtplib.SMTPServerDisconnected:"},{"line_number":68,"context_line":"            self.log.warn(\u0027SMTP server disconnected. \u0027"},{"line_number":69,"context_line":"                          \u0027Will reconnect and retry message.\u0027)"},{"line_number":70,"context_line":"            self._smtp_connect()"},{"line_number":71,"context_line":"        except smtplib.SMTPException:"},{"line_number":72,"context_line":"            self._email_error(notification)"},{"line_number":73,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_fb5dff48","line":70,"in_reply_to":"3a961159_5b5e1d14","updated":"2015-01-16 18:24:18.000000000","message":"This is attempting to reconnect to the SMTP server.  If it succedes we fall down to the second try below.  This was originally a set of nested try/catch statements.  I found that a little hard to follow but would that be more standard?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _email_error(self, notification):"},{"line_number":91,"context_line":"        self.log.exception(\"Error sending Email Notification\")"},{"line_number":92,"context_line":"        self.log.error(\"Failed email: %s\" % notification.to_json())"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    # ------------------------------------------------------------------------"},{"line_number":95,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_1be73513","line":92,"updated":"2015-01-16 14:16:39.000000000","message":"I think it would be better to have just one error in the log file for this if possible.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _email_error(self, notification):"},{"line_number":91,"context_line":"        self.log.exception(\"Error sending Email Notification\")"},{"line_number":92,"context_line":"        self.log.error(\"Failed email: %s\" % notification.to_json())"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    # ------------------------------------------------------------------------"},{"line_number":95,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_db3a03e5","line":92,"in_reply_to":"3a961159_1be73513","updated":"2015-01-16 18:24:18.000000000","message":"I only broke that up because of some annoying limitations in the unittest framework.  I\u0027ll see if I can come up with a better way to test this error condition.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"}],"monasca_notification/types/Pagerduty.py":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import requests"},{"line_number":18,"context_line":"import sys"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from BaseNotifier import BaseNotifier"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class PagerdutyNotifier(BaseNotifier):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_bb92a148","line":20,"updated":"2015-01-16 14:16:39.000000000","message":"Same comment, should be pagerduty.py or pagerduty_notifier.py","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                                   headers\u003dheaders,"},{"line_number":67,"context_line":"                                   timeout\u003dself.config[\u0027timeout\u0027])"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            valid_http_codes \u003d [200, 201, 204]"},{"line_number":70,"context_line":"            if result.status_code in valid_http_codes:"},{"line_number":71,"context_line":"                return True"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_3bab111f","line":69,"updated":"2015-01-16 14:16:39.000000000","message":"As this is a constant array it could be moved to the top of the file.\n\nVALID_HTTP_CODES \u003d [200, 201, 204]","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                                   headers\u003dheaders,"},{"line_number":67,"context_line":"                                   timeout\u003dself.config[\u0027timeout\u0027])"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            valid_http_codes \u003d [200, 201, 204]"},{"line_number":70,"context_line":"            if result.status_code in valid_http_codes:"},{"line_number":71,"context_line":"                return True"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_3b1ce7c1","line":69,"in_reply_to":"3a961159_3bab111f","updated":"2015-01-16 18:24:18.000000000","message":"It is a constant but should it be accessible to anyone that imports the file?  Is there a way to make values actually const so no one can damage them?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"}],"monasca_notification/types/abstract_notifier.py":[{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"c30a932feec03e7594954cb6789eb2742f12e20c","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        pass"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def type(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_c219ddd2","line":27,"updated":"2015-01-20 18:53:48.000000000","message":"type and statsd_name are just strings they should just be values not methods, in this abstract class you can set them as None.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"}],"monasca_notification/types/email_notifier.py":[{"author":{"_account_id":10046,"name":"Henrique Truta","email":"henrique@lsd.ufcg.edu.br","username":"henriquetruta"},"change_message_id":"709435a582cf1652459452969e3b017d66d38745","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def config(self, config):"},{"line_number":30,"context_line":"        self._config \u003d config"},{"line_number":31,"context_line":"        self._smtp_connect()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def type(self):"},{"line_number":34,"context_line":"        return \"email\""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_61683589","line":31,"updated":"2015-01-20 14:06:06.000000000","message":"Shouldn\u0027t the smtp connection be on send_notification? I think the connection may not be closed in some scenarios","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"6f243d854ad4b55773c729af8ea6ee2ec1d4edfa","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def config(self, config):"},{"line_number":30,"context_line":"        self._config \u003d config"},{"line_number":31,"context_line":"        self._smtp_connect()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def type(self):"},{"line_number":34,"context_line":"        return \"email\""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_49dfd83a","line":31,"in_reply_to":"3a961159_61683589","updated":"2015-01-20 18:48:17.000000000","message":"The connection is often closed when we attempt to send so we do attempt a reconnect there.  I think this is attempting to see if we can connect to an SMTP server when the object is constructed rather than finding out at runtime.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"a84ded1e68b6d31a583618c0711faf6dd44bd20c","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        for metric in notification.metrics:"},{"line_number":48,"context_line":"            for dimension in metric[\u0027dimensions\u0027]:"},{"line_number":49,"context_line":"                if \u0027hostname\u0027 in dimension:"},{"line_number":50,"context_line":"                    if not metric[\u0027dimensions\u0027][\u0027%s\u0027 % dimension] in hostname[:]:"},{"line_number":51,"context_line":"                        hostname.append(metric[\u0027dimensions\u0027][\u0027%s\u0027 % dimension])"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # Generate the message"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_78445819","line":50,"updated":"2015-01-20 19:41:57.000000000","message":"Do we need a \u0027%\u0027 operator here? Can it just be \u0027[dimension]\u0027?","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"a84ded1e68b6d31a583618c0711faf6dd44bd20c","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            for dimension in metric[\u0027dimensions\u0027]:"},{"line_number":49,"context_line":"                if \u0027hostname\u0027 in dimension:"},{"line_number":50,"context_line":"                    if not metric[\u0027dimensions\u0027][\u0027%s\u0027 % dimension] in hostname[:]:"},{"line_number":51,"context_line":"                        hostname.append(metric[\u0027dimensions\u0027][\u0027%s\u0027 % dimension])"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # Generate the message"},{"line_number":54,"context_line":"        msg \u003d self._create_msg(hostname, notification)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_b84ec036","line":51,"updated":"2015-01-20 19:41:57.000000000","message":"Same question as above.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"c30a932feec03e7594954cb6789eb2742f12e20c","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            self._log.warn(\u0027SMTP server disconnected. \u0027"},{"line_number":61,"context_line":"                           \u0027Will reconnect and retry message.\u0027)"},{"line_number":62,"context_line":"            self._smtp_connect()"},{"line_number":63,"context_line":"        except smtplib.SMTPException:"},{"line_number":64,"context_line":"            self._email_error(notification)"},{"line_number":65,"context_line":"            return False"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_ece5f23e","line":63,"updated":"2015-01-20 18:53:48.000000000","message":"This exception should just be part of _sendmail so it need not be repeated. Also it should make it a bit clearer what is going on. Looks like _email_error could be collapsed into that also as it is only used in this case.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"32fed02f770a7df303229caea63b8235834e4d54","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            self._log.warn(\u0027SMTP server disconnected. \u0027"},{"line_number":61,"context_line":"                           \u0027Will reconnect and retry message.\u0027)"},{"line_number":62,"context_line":"            self._smtp_connect()"},{"line_number":63,"context_line":"        except smtplib.SMTPException:"},{"line_number":64,"context_line":"            self._email_error(notification)"},{"line_number":65,"context_line":"            return False"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_05aa4670","line":63,"in_reply_to":"1a930d6b_5a9fc903","updated":"2015-01-20 23:41:38.000000000","message":"You can re-raise SMTPServerDisconnected, but there is no way to add exceptions to the trapping.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"8ad261353c1406f8b7ca22c8e9e32ee96d24afd6","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            self._log.warn(\u0027SMTP server disconnected. \u0027"},{"line_number":61,"context_line":"                           \u0027Will reconnect and retry message.\u0027)"},{"line_number":62,"context_line":"            self._smtp_connect()"},{"line_number":63,"context_line":"        except smtplib.SMTPException:"},{"line_number":64,"context_line":"            self._email_error(notification)"},{"line_number":65,"context_line":"            return False"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_5a9fc903","line":63,"in_reply_to":"3a961159_ece5f23e","updated":"2015-01-20 23:33:27.000000000","message":"SMTPException is a superset that contains SMTPServerDisconnected.  Is there a way to trap anything other than a disconnect?  Otherwise it also traps the fact that we need to try and reconnect.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"a84ded1e68b6d31a583618c0711faf6dd44bd20c","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def _sendmail(self, notification, msg):"},{"line_number":75,"context_line":"        self._smtp.sendmail(self._config[\u0027from_addr\u0027], notification.address, msg.as_string())"},{"line_number":76,"context_line":"        self._log.debug(\u0027Sent email to %s, notification %s\u0027 % (notification.address, notification.to_json()))"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _email_error(self, notification):"},{"line_number":79,"context_line":"        self._log.exception(\"Error sending Email Notification\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_786b7886","line":76,"updated":"2015-01-20 19:41:57.000000000","message":"We need to move away from the \u0027%\u0027 operator to the \u0027format\u0027 operator.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":10046,"name":"Henrique Truta","email":"henrique@lsd.ufcg.edu.br","username":"henriquetruta"},"change_message_id":"709435a582cf1652459452969e3b017d66d38745","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            self._smtp \u003d smtp"},{"line_number":96,"context_line":"        except Exception:"},{"line_number":97,"context_line":"            self._log.exception(\"Unable to connect to email server.  Exiting.\")"},{"line_number":98,"context_line":"            sys.exit(1)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def _create_msg(self, hostname, notification):"},{"line_number":101,"context_line":"        \"\"\"Create two kind of messages:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_7c161a4b","line":98,"updated":"2015-01-20 14:06:06.000000000","message":"I think you should consider the previous comment from Craig here","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"6f243d854ad4b55773c729af8ea6ee2ec1d4edfa","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            self._smtp \u003d smtp"},{"line_number":96,"context_line":"        except Exception:"},{"line_number":97,"context_line":"            self._log.exception(\"Unable to connect to email server.  Exiting.\")"},{"line_number":98,"context_line":"            sys.exit(1)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def _create_msg(self, hostname, notification):"},{"line_number":101,"context_line":"        \"\"\"Create two kind of messages:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_029765ec","line":98,"in_reply_to":"3a961159_7c161a4b","updated":"2015-01-20 18:48:17.000000000","message":"No one really likes that line but we\u0027re planning to fix this the right way shortly with a more major refactoring.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"e5b1f38ef16246108cc26a45cf8eb230937cd4a7","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        for metric in notification.metrics:"},{"line_number":50,"context_line":"            for dimension in metric[\u0027dimensions\u0027]:"},{"line_number":51,"context_line":"                if \u0027hostname\u0027 in dimension:"},{"line_number":52,"context_line":"                    if not metric[\u0027dimensions\u0027][dimension] in hostname[:]:"},{"line_number":53,"context_line":"                        hostname.append(metric[\u0027dimensions\u0027][dimension])"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Generate the message"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_60121864","line":52,"updated":"2015-01-21 00:52:04.000000000","message":"I think \u0027hostname[:]\u0027 should just be \u0027hostname\u0027.","commit_id":"683e87803e990131fe954543555efd23631ac7ab"},{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"3a2dcdeb5370bd6c82203f3d0d54c864aed6795c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        for metric in notification.metrics:"},{"line_number":50,"context_line":"            for dimension in metric[\u0027dimensions\u0027]:"},{"line_number":51,"context_line":"                if \u0027hostname\u0027 in dimension:"},{"line_number":52,"context_line":"                    if not metric[\u0027dimensions\u0027][dimension] in hostname[:]:"},{"line_number":53,"context_line":"                        hostname.append(metric[\u0027dimensions\u0027][dimension])"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Generate the message"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_ab204a9c","line":52,"in_reply_to":"1a930d6b_60121864","updated":"2015-01-21 15:28:30.000000000","message":"Yes","commit_id":"683e87803e990131fe954543555efd23631ac7ab"}],"monasca_notification/types/interface.py":[{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"4242876010b87e64e6539d79840575ccaacb3c46","unresolved":false,"context_lines":[{"line_number":11,"context_line":"# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or"},{"line_number":12,"context_line":"# implied."},{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import logging"},{"line_number":17,"context_line":"import time"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_171d5e62","line":14,"updated":"2015-01-15 21:55:45.000000000","message":"Rename this class to notifier or something like that","commit_id":"1c68fcc52249dd584db8b2e9dfec499677d48085"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"4242876010b87e64e6539d79840575ccaacb3c46","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"log \u003d logging.getLogger(__name__)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"notification_list \u003d []"},{"line_number":26,"context_line":"notification_table \u003d {}"},{"line_number":27,"context_line":"statsd_counter \u003d {}"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_37209a99","line":25,"updated":"2015-01-15 21:55:45.000000000","message":"Rename to possible_notifiers","commit_id":"1c68fcc52249dd584db8b2e9dfec499677d48085"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"4242876010b87e64e6539d79840575ccaacb3c46","unresolved":false,"context_lines":[{"line_number":23,"context_line":"log \u003d logging.getLogger(__name__)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"notification_list \u003d []"},{"line_number":26,"context_line":"notification_table \u003d {}"},{"line_number":27,"context_line":"statsd_counter \u003d {}"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"statsd \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_7735c2d9","line":26,"updated":"2015-01-15 21:55:45.000000000","message":"Rename to configured_notifiiers","commit_id":"1c68fcc52249dd584db8b2e9dfec499677d48085"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"4242876010b87e64e6539d79840575ccaacb3c46","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    statsd \u003d statsd_obj"},{"line_number":38,"context_line":"    statsd_timer \u003d statsd.get_timer()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    notification_list.append(Email.EmailNotification(log))"},{"line_number":41,"context_line":"    notification_list.append(Webhook.WebhookNotification(log))"},{"line_number":42,"context_line":"    notification_list.append(Pagerduty.PagerdutyNotification(log))"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_5741a637","line":40,"updated":"2015-01-15 21:55:45.000000000","message":"Rename this to Email.EmailNotifier. Same for the other ones","commit_id":"1c68fcc52249dd584db8b2e9dfec499677d48085"}],"monasca_notification/types/notifiers.py":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2014 Hewlett-Packard Development Company, L.P."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":4,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_8dedacf2","line":1,"updated":"2015-01-16 14:16:39.000000000","message":"2015","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"statsd_timer \u003d None"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# ------------------------------------------------------------------------"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def init(statsd_obj):"},{"line_number":36,"context_line":"    global statsd, statsd_timer"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_5b029d0c","line":33,"updated":"2015-01-16 14:16:39.000000000","message":"No one in OpenStack land is going to appreciate these line breaks. Probably should just remove them.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":54,"context_line":"# ------------------------------------------------------------------------"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def config(config):"},{"line_number":57,"context_line":"    for obj in possible_notifiers:"},{"line_number":58,"context_line":"        type \u003d obj.type()"},{"line_number":59,"context_line":"        if type in config:"},{"line_number":60,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_ad6348a9","line":57,"updated":"2015-01-16 14:16:39.000000000","message":"obj isn\u0027t very descriptive","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        type \u003d obj.type()"},{"line_number":59,"context_line":"        if type in config:"},{"line_number":60,"context_line":"            try:"},{"line_number":61,"context_line":"                obj.config(config[type])"},{"line_number":62,"context_line":"                configured_notifiers[type] \u003d obj"},{"line_number":63,"context_line":"                statsd_counter[type] \u003d statsd.get_counter(obj.statsd_name())"},{"line_number":64,"context_line":"                log.info(\"%s notification ready\" % type)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_cdd33436","line":61,"updated":"2015-01-16 14:16:39.000000000","message":"Can this invocation of config be removed here?\n\nIn general, objects should be constructed in their fully constructed state if possible. So, when an object is created, it should be ready to go without any other methods needing to be invoked on it.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        type \u003d obj.type()"},{"line_number":59,"context_line":"        if type in config:"},{"line_number":60,"context_line":"            try:"},{"line_number":61,"context_line":"                obj.config(config[type])"},{"line_number":62,"context_line":"                configured_notifiers[type] \u003d obj"},{"line_number":63,"context_line":"                statsd_counter[type] \u003d statsd.get_counter(obj.statsd_name())"},{"line_number":64,"context_line":"                log.info(\"%s notification ready\" % type)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_7bee6f8c","line":61,"in_reply_to":"3a961159_cdd33436","updated":"2015-01-16 18:24:18.000000000","message":"What Tim wanted, and this goes back to a comment you had on the Email.py file as well, was the ability to detect if we\u0027re trying to send notifications with an unconfigured notification type.  The idea here is that we fill a list of possible notifiers, all the notification types available in the system, and then we configure them with the data read in from the config file.  Only configured notifications are valid for sending.  I\u0027d prefer to only construct the objects if a valid configuration exists, that would allow us to pass the configuration into the constructor, but I don\u0027t know how to introspect a class definition to determine if it is the type I want to construct.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def send_notifications(notifications):"},{"line_number":74,"context_line":"    sent \u003d []"},{"line_number":75,"context_line":"    failed \u003d 0"},{"line_number":76,"context_line":"    invalid \u003d 0"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    for item in notifications:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_0dc7fc98","line":75,"updated":"2015-01-16 14:16:39.000000000","message":"failed_count would be more descriptive.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":73,"context_line":"def send_notifications(notifications):"},{"line_number":74,"context_line":"    sent \u003d []"},{"line_number":75,"context_line":"    failed \u003d 0"},{"line_number":76,"context_line":"    invalid \u003d 0"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    for item in notifications:"},{"line_number":79,"context_line":"        if not valid_notification(item):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_2dcaf89f","line":76,"updated":"2015-01-16 14:16:39.000000000","message":"invalid_count","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    failed \u003d 0"},{"line_number":76,"context_line":"    invalid \u003d 0"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    for item in notifications:"},{"line_number":79,"context_line":"        if not valid_notification(item):"},{"line_number":80,"context_line":"            invalid +\u003d 1"},{"line_number":81,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_6dbfa0fc","line":78,"updated":"2015-01-16 14:16:39.000000000","message":"for notification in notifications\n\n\"item\" isn\u0027t very descriptive","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    for item in notifications:"},{"line_number":79,"context_line":"        if not valid_notification(item):"},{"line_number":80,"context_line":"            invalid +\u003d 1"},{"line_number":81,"context_line":"            continue"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        with statsd_timer.time(item.type + \u0027_time\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_6d46c022","line":80,"updated":"2015-01-16 14:16:39.000000000","message":"Do you want to log warn that you received an invalid notification?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"a00bb9969bc2dd2749ecdafb37914965ab6ae7d3","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    for item in notifications:"},{"line_number":79,"context_line":"        if not valid_notification(item):"},{"line_number":80,"context_line":"            invalid +\u003d 1"},{"line_number":81,"context_line":"            continue"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        with statsd_timer.time(item.type + \u0027_time\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_db9e8305","line":80,"in_reply_to":"3a961159_6d46c022","updated":"2015-01-16 18:24:18.000000000","message":"The log message for that was happening in the valid_notification function.  Would you prefer log messages at a centralized location or more local to where a test failed?","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"284ee871126045af30f90118d477a74aa0f10c3d","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            sent.append(item)"},{"line_number":89,"context_line":"            statsd_counter[item.type].increment(1)"},{"line_number":90,"context_line":"        else:"},{"line_number":91,"context_line":"            failed +\u003d 1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    return (sent, failed, invalid)"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_cde9741b","line":91,"updated":"2015-01-16 14:16:39.000000000","message":"Personally, I would have wrote this with send_single_notification raising an exception, rather than returning True or False and needing to check the return value.","commit_id":"acf53fd9a9e71d7e7e45a8ede61c0fbc7e236e65"},{"author":{"_account_id":10046,"name":"Henrique Truta","email":"henrique@lsd.ufcg.edu.br","username":"henriquetruta"},"change_message_id":"709435a582cf1652459452969e3b017d66d38745","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"log \u003d logging.getLogger(__name__)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"possible_notifiers \u003d []"},{"line_number":26,"context_line":"configured_notifiers \u003d {}"},{"line_number":27,"context_line":"statsd_counter \u003d {}"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_419c195c","line":25,"updated":"2015-01-20 14:06:06.000000000","message":"Why not just \"notifiers\"?","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"6f243d854ad4b55773c729af8ea6ee2ec1d4edfa","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"log \u003d logging.getLogger(__name__)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"possible_notifiers \u003d []"},{"line_number":26,"context_line":"configured_notifiers \u003d {}"},{"line_number":27,"context_line":"statsd_counter \u003d {}"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_290c4cec","line":25,"in_reply_to":"3a961159_419c195c","updated":"2015-01-20 18:48:17.000000000","message":"Until they\u0027re configured they\u0027re not usable so possible seemed the right term there.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"c30a932feec03e7594954cb6789eb2742f12e20c","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    invalid_count \u003d 0"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    for notification in notifications:"},{"line_number":71,"context_line":"        type \u003d notification.type"},{"line_number":72,"context_line":"        if type not in configured_notifiers:"},{"line_number":73,"context_line":"            log.warn(\"attempting to send unconfigured notification: %s\" % type)"},{"line_number":74,"context_line":"            invalid_count +\u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_42b24de6","line":71,"updated":"2015-01-20 18:53:48.000000000","message":"type is a built-in name you should avoid redefining it. Same above in the config method.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"3a2dcdeb5370bd6c82203f3d0d54c864aed6795c","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"def send_single_notification(notification):"},{"line_number":90,"context_line":"    type \u003d notification.type"},{"line_number":91,"context_line":"    try:"},{"line_number":92,"context_line":"        return configured_notifiers[type].send_notification(notification)"},{"line_number":93,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_eb125257","line":90,"updated":"2015-01-21 15:28:30.000000000","message":"You used plain type again here. FWIW above you fixed one by not having a local variable and the other by renaming the local variable. Either works but it is a tiny bit nicer to be consistent.","commit_id":"683e87803e990131fe954543555efd23631ac7ab"}],"monasca_notification/types/pagerduty_notifier.py":[{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"a84ded1e68b6d31a583618c0711faf6dd44bd20c","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self._config.update(config)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def type(self):"},{"line_number":36,"context_line":"        return \"pagerduty\""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def statsd_name(self):"},{"line_number":39,"context_line":"        return \u0027sent_pagerduty_count\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_78e0d8f8","line":36,"updated":"2015-01-20 19:41:57.000000000","message":"Take a look at Python @property.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"},{"author":{"_account_id":11094,"name":"Tim Kuhlman","email":"timothy.kuhlman@canonical.com","username":"kuhlmant"},"change_message_id":"c30a932feec03e7594954cb6789eb2742f12e20c","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"            self._log.error(\"Error with pagerduty request. key\u003d\u003c%s\u003e response\u003d%s\""},{"line_number":67,"context_line":"                            % (notification.address, result.status_code))"},{"line_number":68,"context_line":"            return False"},{"line_number":69,"context_line":"        except:"},{"line_number":70,"context_line":"            self._log.exception(\"Exception on pagerduty request. key\u003d\u003c%s\u003e exception\u003d%s\""},{"line_number":71,"context_line":"                                % (notification.address, sys.exc_info()[0]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_1d06e61c","line":68,"updated":"2015-01-20 18:53:48.000000000","message":"It is generally best practice to have \u0027except Exception:\u0027 over a raw except, https://www.python.org/dev/peps/pep-0008/#programming-recommendations\n\nThis is in the webhook also.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"}],"notification.yaml":[{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"a84ded1e68b6d31a583618c0711faf6dd44bd20c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# A dictionary set according to the params defined in, http://dev.mysql.com/doc/refman/5.0/en/mysql-ssl-set.html"},{"line_number":14,"context_line":"#    ssl: {\u0027ca\u0027: \u0027/path/to/ca\u0027}"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"notification_types:"},{"line_number":17,"context_line":"    email:"},{"line_number":18,"context_line":"        server: 192.168.10.4"},{"line_number":19,"context_line":"        port: 25"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"3a961159_187fd418","line":16,"updated":"2015-01-20 19:41:57.000000000","message":"Eventually, the yaml file needs to be turned into a .ini file using oslo.config. oslo.config is the OpenStack standard.","commit_id":"3b70d9f3b689a580c435da890cfa5f652040b93e"}]}