)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"7e785eee888d794856e98b042cdcddfaded2d733","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michael James Hoppal \u003cmichael.jam.hoppal@hp.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-08-17 15:44:34 -0600"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Query mysql to see if the periodic notificiation changes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Also added notification id to notification object we construct in the"},{"line_number":10,"context_line":"alarm processor"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"3ac371cc_2c105ea8","line":7,"updated":"2016-08-19 05:19:28.000000000","message":"There are also changes in ORM layer, so I guess that commit msg is not accurate :). Maybe let\u0027s put this in a general way like:\n\n Query database to see...\n\n?","commit_id":"de741c3687d57c3a453bb8785364bdfc143df419"}],"monasca_notification/common/repositories/mysql/mysql_repo.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            cur.execute(self._find_alarm_action_sql, (alarm[\u0027alarmDefinitionId\u0027], alarm[\u0027newState\u0027]))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            for row in cur:"},{"line_number":70,"context_line":"                yield (row[0], row[2].lower(), row[1], row[3], row[4])"},{"line_number":71,"context_line":"        except pymysql.Error as e:"},{"line_number":72,"context_line":"            self._mysql \u003d None"},{"line_number":73,"context_line":"            log.exception(\"Couldn\u0027t fetch alarms actions %s\", e)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_86b35f97","line":70,"range":{"start_line":70,"start_character":23,"end_line":70,"end_character":26},"updated":"2016-07-21 18:04:42.000000000","message":"I can\u0027t say I understood the order we had before but why the order change now?","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"7df2fb78e17183cd584daca34a11fd3ba37e38e5","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            cur.execute(self._find_alarm_action_sql, (alarm[\u0027alarmDefinitionId\u0027], alarm[\u0027newState\u0027]))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            for row in cur:"},{"line_number":70,"context_line":"                yield (row[0], row[2].lower(), row[1], row[3], row[4])"},{"line_number":71,"context_line":"        except pymysql.Error as e:"},{"line_number":72,"context_line":"            self._mysql \u003d None"},{"line_number":73,"context_line":"            log.exception(\"Couldn\u0027t fetch alarms actions %s\", e)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bacf61ea_3b5faf0f","line":70,"range":{"start_line":70,"start_character":23,"end_line":70,"end_character":26},"in_reply_to":"dada55a8_69db9c7c","updated":"2016-07-29 05:21:09.000000000","message":"maybe it\u0027d be better to assign some labels to those magic numbers.","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            cur.execute(self._find_alarm_action_sql, (alarm[\u0027alarmDefinitionId\u0027], alarm[\u0027newState\u0027]))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            for row in cur:"},{"line_number":70,"context_line":"                yield (row[0], row[2].lower(), row[1], row[3], row[4])"},{"line_number":71,"context_line":"        except pymysql.Error as e:"},{"line_number":72,"context_line":"            self._mysql \u003d None"},{"line_number":73,"context_line":"            log.exception(\"Couldn\u0027t fetch alarms actions %s\", e)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_69db9c7c","line":70,"range":{"start_line":70,"start_character":23,"end_line":70,"end_character":26},"in_reply_to":"dada55a8_86b35f97","updated":"2016-07-21 18:09:07.000000000","message":"we changed the query to mysql and it returns id first","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"b915d83fa8554c5c0290dfdfea6d14ceff565487","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    def get_notification(self, notification_id):"},{"line_number":91,"context_line":"        try:"},{"line_number":92,"context_line":"            if self._mysql is None:"},{"line_number":93,"context_line":"                self._connect_to_mysql()"},{"line_number":94,"context_line":"            cur \u003d self._mysql.cursor()"},{"line_number":95,"context_line":"            cur.execute(self._get_notification_sql, notification_id)"},{"line_number":96,"context_line":"            row \u003d cur.fetchone()"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_0a6d2598","line":93,"updated":"2016-07-25 22:43:45.000000000","message":"I realize you didn\u0027t start the precedent in this code, but I was just wondering if having the test for if self._mysql is None, is the best approach, or whether this should be moved to the initializer and done once at start-up.","commit_id":"324138de8a7727556cdaf357068ea46b813ab95f"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"2ba75f346a8c008489fef3ed07f58e052711603d","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    def get_notification(self, notification_id):"},{"line_number":91,"context_line":"        try:"},{"line_number":92,"context_line":"            if self._mysql is None:"},{"line_number":93,"context_line":"                self._connect_to_mysql()"},{"line_number":94,"context_line":"            cur \u003d self._mysql.cursor()"},{"line_number":95,"context_line":"            cur.execute(self._get_notification_sql, notification_id)"},{"line_number":96,"context_line":"            row \u003d cur.fetchone()"}],"source_content_type":"text/x-python","patch_set":14,"id":"bacf61ea_16fe1238","line":93,"in_reply_to":"dada55a8_0a6d2598","updated":"2016-07-28 20:28:02.000000000","message":"Joe can comment on this a little more but we do this because we have run into issues where the connection is severed/timeouted or the mysql database is down and we need to reconnect and instead of restarting the notification engine we just set the mysql connection to none and retry next time we need to interact with it.","commit_id":"324138de8a7727556cdaf357068ea46b813ab95f"},{"author":{"_account_id":220,"name":"Haneef Ali","email":"haneef.ali@hp.com","username":"haneef"},"change_message_id":"0d4cde695216d340d09cc7109b2ad585d9ab9307","unresolved":false,"context_lines":[{"line_number":123,"context_line":"            if row is None:"},{"line_number":124,"context_line":"                return None"},{"line_number":125,"context_line":"            else:"},{"line_number":126,"context_line":"                return [row[0], row[1].lower(), row[2], row[3]]"},{"line_number":127,"context_line":"        except pymysql.Error as e:"},{"line_number":128,"context_line":"            self._mysql \u003d None"},{"line_number":129,"context_line":"            log.exception(\"Couldn\u0027t fetch the notification method %s\", e)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_ef471a75","line":126,"updated":"2016-08-09 19:47:17.000000000","message":"Why do you want to do lower?  In DB it is in upper case.","commit_id":"cc4c536ce0ac93cefcd663c9f6c28b2a3ca9d613"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"c81f1c659a337e85f2fde19ad3001af6bec4aee3","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            cur.execute(self._find_alarm_action_sql, (alarm[\u0027alarmDefinitionId\u0027], alarm[\u0027newState\u0027]))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            for row in cur:"},{"line_number":70,"context_line":"                yield (row[0], row[1].upper(), row[2], row[3], row[4])"},{"line_number":71,"context_line":"        except pymysql.Error as e:"},{"line_number":72,"context_line":"            self._mysql \u003d None"},{"line_number":73,"context_line":"            log.exception(\"Couldn\u0027t fetch alarms actions %s\", e)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3ac371cc_7e2127fb","line":70,"updated":"2016-08-16 08:20:01.000000000","message":"Personally, I find using magic numbers here a bad practice making code harder to read.\n\nThat\u0027s really not much an effort to add them here and allow other developers to grab a concept.","commit_id":"8a16a4887310495fa358750e69fa8d2d6beb4841"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"9bfa6f163607cac66ff4d94c562a9f867ff419c9","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            cur.execute(self._find_alarm_action_sql, (alarm[\u0027alarmDefinitionId\u0027], alarm[\u0027newState\u0027]))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            for row in cur:"},{"line_number":70,"context_line":"                yield (row[0], row[1].upper(), row[2], row[3], row[4])"},{"line_number":71,"context_line":"        except pymysql.Error as e:"},{"line_number":72,"context_line":"            self._mysql \u003d None"},{"line_number":73,"context_line":"            log.exception(\"Couldn\u0027t fetch alarms actions %s\", e)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3ac371cc_a2b524ef","line":70,"in_reply_to":"3ac371cc_7e2127fb","updated":"2016-08-16 16:46:57.000000000","message":"Hi Tomasz I agree that having the list indexes here is not the cleanest but I couldnt find a cleaner way to implement it. I looked into seeing if the mysql library can return the result as a dictionary instead of the list with the column names as the keys but I couldnt find anything \n\nDo you have any suggestions? I am open to anything","commit_id":"8a16a4887310495fa358750e69fa8d2d6beb4841"}],"monasca_notification/common/repositories/orm/models.py":[{"author":{"_account_id":12133,"name":"Cindy O\u0027Neill","email":"cindy.o-neill@hpe.com","username":"oneilcin"},"change_message_id":"6cfd790191ea4ef35e5082b0c61859951c630979","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def create_alarm_action_model(metadata\u003dNone):"},{"line_number":30,"context_line":"    ALARM_STATES \u003d (\u0027UNDETERMINED\u0027, \u0027OK\u0027, \u0027ALARM\u0027)"},{"line_number":31,"context_line":"    return Table(\u0027alarm_action\u0027, metadata,"},{"line_number":32,"context_line":"                 Column(\u0027action_id\u0027,"},{"line_number":33,"context_line":"                        String(36), ForeignKey(\u0027notification_method.id\u0027),"}],"source_content_type":"text/x-python","patch_set":21,"id":"3ac371cc_45f18f00","line":30,"updated":"2016-08-17 20:57:36.000000000","message":"Is this ALARM_STATES line still needed here, since it is defined above?","commit_id":"95588bda49a4cb66886526c4c2458dc88cf7e62a"}],"monasca_notification/common/repositories/orm/orm_repo.py":[{"author":{"_account_id":220,"name":"Haneef Ali","email":"haneef.ali@hp.com","username":"haneef"},"change_message_id":"0d4cde695216d340d09cc7109b2ad585d9ab9307","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                                             alarm_definition_id\u003dalarm[\u0027alarmDefinitionId\u0027],"},{"line_number":51,"context_line":"                                             alarm_state\u003dalarm[\u0027newState\u0027])"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"                return [(row[0], row[1].lower(), row[2], row[3], row[4]) for row in notifications]"},{"line_number":54,"context_line":"        except DatabaseError as e:"},{"line_number":55,"context_line":"            log.exception(\"Couldn\u0027t fetch alarms actions %s\", e)"},{"line_number":56,"context_line":"            raise exc.DatabaseException(e)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_4f2626cf","line":53,"updated":"2016-08-09 19:47:17.000000000","message":"Same. Why lower?","commit_id":"cc4c536ce0ac93cefcd663c9f6c28b2a3ca9d613"}],"monasca_notification/common/repositories/postgres/pgsql_repo.py":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"b915d83fa8554c5c0290dfdfea6d14ceff565487","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    def get_notification(self, notification_id):"},{"line_number":64,"context_line":"        try:"},{"line_number":65,"context_line":"            if self._pgsql is None:"},{"line_number":66,"context_line":"                self._connect_to_pgsql()"},{"line_number":67,"context_line":"            cur \u003d self._pgsql.cursor()"},{"line_number":68,"context_line":"            cur.execute(self._get_notification_sql, notification_id)"},{"line_number":69,"context_line":"            row \u003d cur.fetchone()"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_30617ca9","line":66,"updated":"2016-07-25 22:43:45.000000000","message":"Similar comment here related to checking self._pgsql. In general, I try and create objects in there fully constructed state.","commit_id":"324138de8a7727556cdaf357068ea46b813ab95f"}],"monasca_notification/common/utils.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"5b2276b169cadc6874292c8bfca35502f88a7a0f","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            stored_notification \u003d db_repo.get_notification(notification.id)"},{"line_number":50,"context_line":"        except exceptions.DatabaseException:"},{"line_number":51,"context_line":"            log.debug(\u0027Database Error.  Attempting reconnect\u0027)"},{"line_number":52,"context_line":"            stored_notification \u003d db_repo.get_notification(notification.id)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        # Notification method was deleted"},{"line_number":55,"context_line":"        if stored_notification is None:"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_b90790c8","line":52,"range":{"start_line":52,"start_character":12,"end_line":52,"end_character":31},"updated":"2016-07-22 06:04:30.000000000","message":"If there is a second error here we won\u0027t commit the offset.  It\u0027s probably not an issue because there shouldn\u0027t be anything that the notification.id could do to cause this query to consistently fail but I\u0027m not certain.","commit_id":"1fca98d5ff2ab1cd472b335d05f23a9611f541de"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"dde800470e0fc957d23b9e8b086947e81490a41f","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        return simport.load(\u0027monasca_notification.common.repositories.mysql.mysql_repo:MysqlRepo\u0027)(config)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def construct_notification_method(db_repo, notification_json):"},{"line_number":33,"context_line":"    try:"},{"line_number":34,"context_line":"        notification \u003d Notification(notification_json[\u0027id\u0027],"},{"line_number":35,"context_line":"                                    notification_json[\u0027type\u0027],"}],"source_content_type":"text/x-python","patch_set":13,"id":"dada55a8_42e39433","line":32,"range":{"start_line":32,"start_character":4,"end_line":32,"end_character":33},"updated":"2016-07-25 16:11:26.000000000","message":"I\u0027m not sure the name really says what you\u0027re doing.  You\u0027re creating a notification object not really doing anything with a method.  If we hadn\u0027t used \"notification method\" throughout our documentation I probably wouldn\u0027t care but it might be better to avoid any confusion.","commit_id":"b888ec0401066dd518d11ee54489b40f5a590425"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"1d2bb937d07283d04ce87e22cff6e3991755eb9a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            stored_notification \u003d db_repo.get_notification(notification.id)"},{"line_number":51,"context_line":"        except exceptions.DatabaseException:"},{"line_number":52,"context_line":"            log.debug(\u0027Database Error.  Attempting reconnect\u0027)"},{"line_number":53,"context_line":"            stored_notification \u003d db_repo.get_notification(notification.id)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Notification method was deleted"},{"line_number":56,"context_line":"        if stored_notification is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9ad45d7e_47200f9c","line":53,"range":{"start_line":53,"start_character":12,"end_line":53,"end_character":75},"updated":"2016-08-08 18:54:24.000000000","message":"What happens if this get_notification() fails as well? Does the calling code handle the exception?","commit_id":"c00607616c83c0d0a53111097ee2be88b502938a"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"7b6dbef4db00098e5bc361e7ff0207405a3aa2b5","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            stored_notification \u003d db_repo.get_notification(notification.id)"},{"line_number":51,"context_line":"        except exceptions.DatabaseException:"},{"line_number":52,"context_line":"            log.debug(\u0027Database Error.  Attempting reconnect\u0027)"},{"line_number":53,"context_line":"            stored_notification \u003d db_repo.get_notification(notification.id)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Notification method was deleted"},{"line_number":56,"context_line":"        if stored_notification is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9ad45d7e_0959095b","line":53,"range":{"start_line":53,"start_character":12,"end_line":53,"end_character":75},"in_reply_to":"9ad45d7e_47200f9c","updated":"2016-08-08 19:39:04.000000000","message":"yes the try catch should above","commit_id":"c00607616c83c0d0a53111097ee2be88b502938a"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"1d2bb937d07283d04ce87e22cff6e3991755eb9a","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Notification method was deleted"},{"line_number":56,"context_line":"        if stored_notification is None:"},{"line_number":57,"context_line":"            return False"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        notification.name \u003d stored_notification[0]"},{"line_number":60,"context_line":"        notification.type \u003d stored_notification[1]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9ad45d7e_0774579a","line":57,"range":{"start_line":57,"start_character":12,"end_line":57,"end_character":24},"updated":"2016-08-08 18:54:24.000000000","message":"If the notification method no longer exists, shouldn\u0027t we stop attempting to deliver this notification?","commit_id":"c00607616c83c0d0a53111097ee2be88b502938a"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"7b6dbef4db00098e5bc361e7ff0207405a3aa2b5","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Notification method was deleted"},{"line_number":56,"context_line":"        if stored_notification is None:"},{"line_number":57,"context_line":"            return False"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        notification.name \u003d stored_notification[0]"},{"line_number":60,"context_line":"        notification.type \u003d stored_notification[1]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9ad45d7e_a9227db7","line":57,"range":{"start_line":57,"start_character":12,"end_line":57,"end_character":24},"in_reply_to":"9ad45d7e_0774579a","updated":"2016-08-08 19:39:04.000000000","message":"this is why i -1 the code. found that this morning testing my new change in this file","commit_id":"c00607616c83c0d0a53111097ee2be88b502938a"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"45bebfe474462ec50c53ba4973810fe93c6f5cb5","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            stored_notification \u003d db_repo.get_notification(notification_id)"},{"line_number":61,"context_line":"        except exceptions.DatabaseException:"},{"line_number":62,"context_line":"            log.debug(\u0027Database Error.  Attempting reconnect\u0027)"},{"line_number":63,"context_line":"            stored_notification \u003d db_repo.get_notification(notification_id)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        return stored_notification"}],"source_content_type":"text/x-python","patch_set":20,"id":"3ac371cc_e5b1e690","line":63,"range":{"start_line":63,"start_character":34,"end_line":63,"end_character":75},"updated":"2016-08-16 17:20:45.000000000","message":"If the database is down, an exception will be thrown from here and caught above. None will be returned and periodic_engine.py will just delete the notification. That seems wrong. I think the code needs to distinguish between the notification having been deleted and the database being temporarily down.","commit_id":"8a16a4887310495fa358750e69fa8d2d6beb4841"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"7e785eee888d794856e98b042cdcddfaded2d733","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        # Grab notification method from database to see if it was changed"},{"line_number":42,"context_line":"        stored_notification \u003d grab_stored_notification_method(db_repo, notification.id)"},{"line_number":43,"context_line":"        # Notification method was deleted"},{"line_number":44,"context_line":"        if stored_notification is None:"},{"line_number":45,"context_line":"            return None"},{"line_number":46,"context_line":"        # Update notification method with most up to date values"},{"line_number":47,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":23,"id":"3ac371cc_6c69b625","line":44,"updated":"2016-08-19 05:19:28.000000000","message":"Maybe a log (debug) message would be nice here.","commit_id":"de741c3687d57c3a453bb8785364bdfc143df419"}],"monasca_notification/notification.py":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"b915d83fa8554c5c0290dfdfea6d14ceff565487","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \u0027periodic_topic\u0027"},{"line_number":43,"context_line":"    )"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, nid, ntype, name, address, period, retry_count, alarm):"},{"line_number":46,"context_line":"        \"\"\"Setup the notification object"},{"line_number":47,"context_line":"             The src_partition and src_offset allow the notification"},{"line_number":48,"context_line":"              to be linked to the alarm which triggered it."}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_6aeb0179","line":45,"range":{"start_line":45,"start_character":23,"end_line":45,"end_character":26},"updated":"2016-07-25 22:43:45.000000000","message":"Would id and type worked as variable names. Just seemed like the code would be slightly more descriptive without the \"n\" prefix?","commit_id":"324138de8a7727556cdaf357068ea46b813ab95f"},{"author":{"_account_id":12133,"name":"Cindy O\u0027Neill","email":"cindy.o-neill@hpe.com","username":"oneilcin"},"change_message_id":"6cfd790191ea4ef35e5082b0c61859951c630979","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def __init__(self, id, type, name, address, period, retry_count, alarm):"},{"line_number":46,"context_line":"        \"\"\"Setup the notification object"},{"line_number":47,"context_line":"             The src_partition and src_offset allow the notification"},{"line_number":48,"context_line":"              to be linked to the alarm which triggered it."},{"line_number":49,"context_line":"             id - The notification id"},{"line_number":50,"context_line":"             type - The notification type"},{"line_number":51,"context_line":"             name - Name used in sending"}],"source_content_type":"text/x-python","patch_set":21,"id":"3ac371cc_65a2cb9a","line":48,"updated":"2016-08-17 20:57:36.000000000","message":"You removed the src_partition, and src_offset, so should this change to say you link to the alarm with the alarm_id, or just remove the comment.","commit_id":"95588bda49a4cb66886526c4c2458dc88cf7e62a"}],"monasca_notification/notification_engine.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def _add_periodic_notifications(self, notifications):"},{"line_number":49,"context_line":"        for notification in notifications:"},{"line_number":50,"context_line":"            topic \u003d notification.periodic_topic"},{"line_number":51,"context_line":"            if topic in self._config[\u0027kafka\u0027][\u0027periodic\u0027] and notification.type.lower() \u003d\u003d \"webhook\":"},{"line_number":52,"context_line":"                notification.notification_timestamp \u003d time.time()"},{"line_number":53,"context_line":"                self._producer.publish(self._config[\u0027kafka\u0027][\u0027periodic\u0027][topic],"},{"line_number":54,"context_line":"                                       [notification.to_json()])"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_a9459443","line":51,"range":{"start_line":51,"start_character":80,"end_line":51,"end_character":85},"updated":"2016-07-21 18:04:42.000000000","message":"The notification object should properly format the type.  We shouldn\u0027t have to be adjusting case outside of the object.","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def _add_periodic_notifications(self, notifications):"},{"line_number":49,"context_line":"        for notification in notifications:"},{"line_number":50,"context_line":"            topic \u003d notification.periodic_topic"},{"line_number":51,"context_line":"            if topic in self._config[\u0027kafka\u0027][\u0027periodic\u0027] and notification.type.lower() \u003d\u003d \"webhook\":"},{"line_number":52,"context_line":"                notification.notification_timestamp \u003d time.time()"},{"line_number":53,"context_line":"                self._producer.publish(self._config[\u0027kafka\u0027][\u0027periodic\u0027][topic],"},{"line_number":54,"context_line":"                                       [notification.to_json()])"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_2646f37a","line":51,"range":{"start_line":51,"start_character":0,"end_line":51,"end_character":101},"updated":"2016-07-21 18:04:42.000000000","message":"This line is rather long.","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def _add_periodic_notifications(self, notifications):"},{"line_number":49,"context_line":"        for notification in notifications:"},{"line_number":50,"context_line":"            topic \u003d notification.periodic_topic"},{"line_number":51,"context_line":"            if topic in self._config[\u0027kafka\u0027][\u0027periodic\u0027] and notification.type.lower() \u003d\u003d \"webhook\":"},{"line_number":52,"context_line":"                notification.notification_timestamp \u003d time.time()"},{"line_number":53,"context_line":"                self._producer.publish(self._config[\u0027kafka\u0027][\u0027periodic\u0027][topic],"},{"line_number":54,"context_line":"                                       [notification.to_json()])"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_e9d5cc42","line":51,"range":{"start_line":51,"start_character":80,"end_line":51,"end_character":85},"in_reply_to":"dada55a8_a9459443","updated":"2016-07-21 18:09:07.000000000","message":"it does i missed that","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"}],"monasca_notification/periodic_engine.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self._notifier \u003d NotificationProcessor(config[\u0027notification_types\u0027])"},{"line_number":49,"context_line":"        self._db_repo \u003d get_db_repo(config)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _keep_sending(self, notification):"},{"line_number":52,"context_line":"        # Go to DB and check alarm state and the notification method for changes"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_a629439d","line":51,"range":{"start_line":51,"start_character":28,"end_line":51,"end_character":40},"updated":"2016-07-21 18:04:42.000000000","message":"There\u0027s a lot of repetition of notifiction.\u003csomething\u003e here.  Would it be more clear to just pass in the three variables we care about?","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"9086c87e4a2b5b21aee21b32ca96b857d9e0d95b","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self._notifier \u003d NotificationProcessor(config[\u0027notification_types\u0027])"},{"line_number":49,"context_line":"        self._db_repo \u003d get_db_repo(config)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _keep_sending(self, notification):"},{"line_number":52,"context_line":"        # Go to DB and check alarm state and the notification method for changes"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_9c457044","line":51,"range":{"start_line":51,"start_character":28,"end_line":51,"end_character":40},"in_reply_to":"dada55a8_894ed038","updated":"2016-07-21 18:27:39.000000000","message":"Ah, I didn\u0027t see that at the bottom.  I don\u0027t like that much.  A function that\u0027s returning a boolean shouldn\u0027t be doing destructive changes like that to an object I pass in.  Can you break it into two functions?  One to update the notification object with the current state of the DB and one to see if the current state of the notification object requires sending?","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self._notifier \u003d NotificationProcessor(config[\u0027notification_types\u0027])"},{"line_number":49,"context_line":"        self._db_repo \u003d get_db_repo(config)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _keep_sending(self, notification):"},{"line_number":52,"context_line":"        # Go to DB and check alarm state and the notification method for changes"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_894ed038","line":51,"range":{"start_line":51,"start_character":28,"end_line":51,"end_character":40},"in_reply_to":"dada55a8_a629439d","updated":"2016-07-21 18:09:07.000000000","message":"well we are also looking at id, alarm_id, state, topic name, type, address and notification name","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        if stored_notification is None:"},{"line_number":70,"context_line":"            return False"},{"line_number":71,"context_line":"        # Period changed"},{"line_number":72,"context_line":"        # TODO(Michael) in the future once we support multiple period check if we can put it on another topic"},{"line_number":73,"context_line":"        if stored_notification[3] !\u003d self._topic_name:"},{"line_number":74,"context_line":"            return False"},{"line_number":75,"context_line":"        if stored_notification[1].lower() !\u003d \"webhook\":"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_c6055714","line":72,"range":{"start_line":72,"start_character":0,"end_line":72,"end_character":109},"updated":"2016-07-21 18:04:42.000000000","message":"This line is rather long","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        if stored_notification is None:"},{"line_number":70,"context_line":"            return False"},{"line_number":71,"context_line":"        # Period changed"},{"line_number":72,"context_line":"        # TODO(Michael) in the future once we support multiple period check if we can put it on another topic"},{"line_number":73,"context_line":"        if stored_notification[3] !\u003d self._topic_name:"},{"line_number":74,"context_line":"            return False"},{"line_number":75,"context_line":"        if stored_notification[1].lower() !\u003d \"webhook\":"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_69c27c86","line":72,"range":{"start_line":72,"start_character":0,"end_line":72,"end_character":109},"in_reply_to":"dada55a8_c6055714","updated":"2016-07-21 18:09:07.000000000","message":"will fix","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # TODO(Michael) in the future once we support multiple period check if we can put it on another topic"},{"line_number":73,"context_line":"        if stored_notification[3] !\u003d self._topic_name:"},{"line_number":74,"context_line":"            return False"},{"line_number":75,"context_line":"        if stored_notification[1].lower() !\u003d \"webhook\":"},{"line_number":76,"context_line":"            return False"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        notification.address \u003d stored_notification[2]"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_e952ec11","line":75,"range":{"start_line":75,"start_character":11,"end_line":75,"end_character":30},"updated":"2016-07-21 18:04:42.000000000","message":"Rather than reformatting here the DB access method and the notification object should be formatting the data properly.","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # TODO(Michael) in the future once we support multiple period check if we can put it on another topic"},{"line_number":73,"context_line":"        if stored_notification[3] !\u003d self._topic_name:"},{"line_number":74,"context_line":"            return False"},{"line_number":75,"context_line":"        if stored_notification[1].lower() !\u003d \"webhook\":"},{"line_number":76,"context_line":"            return False"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        notification.address \u003d stored_notification[2]"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_c95448cb","line":75,"range":{"start_line":75,"start_character":11,"end_line":75,"end_character":30},"in_reply_to":"dada55a8_e952ec11","updated":"2016-07-21 18:09:07.000000000","message":"it does missed it will fix","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"5b2276b169cadc6874292c8bfca35502f88a7a0f","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"            notification_method_exists \u003d update_notification_method(self._db_repo, notification)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"            if notification_method_exists and self._keep_sending(notification.alarm_id, notification.state,"},{"line_number":88,"context_line":"                                                                 notification.type, notification.period):"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"                wait_duration \u003d notification.period - ("}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_79b8782a","line":87,"range":{"start_line":87,"start_character":15,"end_line":87,"end_character":41},"updated":"2016-07-22 06:04:30.000000000","message":"I would do this as:\n\nif not notification_method:\n    self._consumer.commit()\n    return\n\ninstead of trying to chain these two statements together.\n\nReally though using the \"with\" clause I mentioned earlier is starting to look better and better now that we\u0027ve got multiple failure modes for a notification object.  It should be much easier to read and maintain if we just hide all this complexity behind a simple interface.","commit_id":"1fca98d5ff2ab1cd472b335d05f23a9611f541de"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"dde800470e0fc957d23b9e8b086947e81490a41f","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                self._consumer.commit()"},{"line_number":82,"context_line":"                continue"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if self._keep_sending(notification.alarm_id, notification.state, notification.type, notification.period):"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"                wait_duration \u003d notification.period - ("},{"line_number":87,"context_line":"                    time.time() - notification_data[\u0027notification_timestamp\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"dada55a8_c715d649","line":84,"range":{"start_line":84,"start_character":34,"end_line":84,"end_character":46},"updated":"2016-07-25 16:11:26.000000000","message":"This line is a little long","commit_id":"b888ec0401066dd518d11ee54489b40f5a590425"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"b915d83fa8554c5c0290dfdfea6d14ceff565487","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class PeriodicEngine(object):"},{"line_number":33,"context_line":"    def __init__(self, config, interval):"},{"line_number":34,"context_line":"        self._topic_name \u003d config[\u0027kafka\u0027][\u0027periodic\u0027][interval]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        self._statsd \u003d monascastatsd.Client(name\u003d\u0027monasca\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_ca778dd2","line":33,"updated":"2016-07-25 22:43:45.000000000","message":"If you are going to call this _periodic_interval internally, it might be good to change the name here as well. Or change the method variable _periodic_interval used down below, to interval, or even \"period\".","commit_id":"324138de8a7727556cdaf357068ea46b813ab95f"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"b915d83fa8554c5c0290dfdfea6d14ceff565487","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        if current_state !\u003d original_state:"},{"line_number":64,"context_line":"            return False"},{"line_number":65,"context_line":"        # Period changed"},{"line_number":66,"context_line":"        if period !\u003d self._periodic_interval:"},{"line_number":67,"context_line":"            return False"},{"line_number":68,"context_line":"        if type !\u003d \"webhook\":"},{"line_number":69,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_75a806dc","line":66,"updated":"2016-07-25 22:43:45.000000000","message":"At this point you are comparing period with _periodic_interval and _periodic_interval is supposed to be the period at which you are resending/processing the notification again, which also happens to be the same as the period for the notification method. In the case of the state, you get that from the database, up above in get_alarm_current_state. But the current period is just gotten from the _periodic_interval. I was wondering if you should get both the state and the period from the database, which would be more consistent.","commit_id":"324138de8a7727556cdaf357068ea46b813ab95f"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"dbb9ba8aafcaf40bca34ac9e6e21d2481c12fc2f","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                    time.time() - notification_data[\u0027notification_timestamp\u0027])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"                if wait_duration \u003e 0:"},{"line_number":93,"context_line":"                    time.sleep(wait_duration)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"                notification.notification_timestamp \u003d time.time()"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"fa7ab95a_0ee8087c","line":93,"range":{"start_line":93,"start_character":20,"end_line":93,"end_character":45},"updated":"2016-08-26 03:14:01.000000000","message":"If I change the notification period to 0 from 60, then notification always gets sent one more time. That is because we sleep after the construct_notification_object() which gets the current period of the notification. Seems wrong. If I change the period, I would expect it take effect almost immediately, not after one more notification.","commit_id":"9e8c8be7af5878d0f773077a90261acd104a427b"}],"monasca_notification/processors/alarm_processor.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        return True"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _build_notification(self, partition, offset, alarm):"},{"line_number":82,"context_line":"        db_time \u003d self._statsd.get_timer()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        with db_time.time(\u0027config_db_time\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_06f12f6b","line":81,"range":{"start_line":81,"start_character":8,"end_line":81,"end_character":27},"updated":"2016-07-21 18:04:42.000000000","message":"If we\u0027re pluralizing fetch_notifications we should pluralize this too.","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        return True"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _build_notification(self, partition, offset, alarm):"},{"line_number":82,"context_line":"        db_time \u003d self._statsd.get_timer()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        with db_time.time(\u0027config_db_time\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_096140a9","line":81,"range":{"start_line":81,"start_character":8,"end_line":81,"end_character":27},"in_reply_to":"dada55a8_06f12f6b","updated":"2016-07-21 18:09:07.000000000","message":"sure sure","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"7be09adc489f81880ba9b4275b7cc283caa40fae","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            alarms_actions \u003d self._db_repo.fetch_notifications(alarm)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        return [Notification(alarms_action[0],"},{"line_number":88,"context_line":"                             alarms_action[1],"},{"line_number":89,"context_line":"                             partition,"},{"line_number":90,"context_line":"                             offset,"},{"line_number":91,"context_line":"                             alarms_action[2],"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_a6f6635a","line":88,"range":{"start_line":88,"start_character":29,"end_line":88,"end_character":42},"updated":"2016-07-21 18:04:42.000000000","message":"Why the change to argument order?  It doesn\u0027t look like you\u0027ve changed the order in the notification object so was it wrong before?","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":15027,"name":"Michael Hoppal","email":"michael.jam.hoppal@hpe.com","username":"michael-hoppal"},"change_message_id":"99d23207a9e1ea3fbb778dd53d094ef4e816aec7","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            alarms_actions \u003d self._db_repo.fetch_notifications(alarm)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        return [Notification(alarms_action[0],"},{"line_number":88,"context_line":"                             alarms_action[1],"},{"line_number":89,"context_line":"                             partition,"},{"line_number":90,"context_line":"                             offset,"},{"line_number":91,"context_line":"                             alarms_action[2],"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_295a44f2","line":88,"range":{"start_line":88,"start_character":29,"end_line":88,"end_character":42},"in_reply_to":"dada55a8_a6f6635a","updated":"2016-07-21 18:09:07.000000000","message":"look at the mysql query","commit_id":"ecada534b39bf3f233156b72364ea6a542281833"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"c81f1c659a337e85f2fde19ad3001af6bec4aee3","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                             alarms_action[1],"},{"line_number":89,"context_line":"                             alarms_action[2],"},{"line_number":90,"context_line":"                             alarms_action[3],"},{"line_number":91,"context_line":"                             alarms_action[4],"},{"line_number":92,"context_line":"                             0,"},{"line_number":93,"context_line":"                             alarm) for alarms_action in alarms_actions]"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3ac371cc_3ea4cfa4","line":91,"updated":"2016-08-16 08:20:01.000000000","message":"Sorry, but for me having these numbers here makes reading and understanding this code pretty much impossible. \n\nI think that we should try and avoid doing that.","commit_id":"8a16a4887310495fa358750e69fa8d2d6beb4841"}],"monasca_notification/retry_engine.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"5b2276b169cadc6874292c8bfca35502f88a7a0f","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"            notification \u003d construct_notification_from_json(notification_data)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"            if notification is None or not update_notification_method(self._db_repo, notification):"},{"line_number":64,"context_line":"                self._consumer.commit()"},{"line_number":65,"context_line":"                continue"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_394e003b","line":63,"range":{"start_line":63,"start_character":15,"end_line":63,"end_character":27},"updated":"2016-07-22 06:04:30.000000000","message":"Same suggestion here as the periodic_engine","commit_id":"1fca98d5ff2ab1cd472b335d05f23a9611f541de"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"dde800470e0fc957d23b9e8b086947e81490a41f","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                if notification.retry_count \u003c self._retry_max:"},{"line_number":82,"context_line":"                    log.error(u\"retry failed for {} with name {} \""},{"line_number":83,"context_line":"                              u\"at {}.  \""},{"line_number":84,"context_line":"                              u\"Saving for later retry.\".format(notification.type, notification.name,"},{"line_number":85,"context_line":"                                                                notification.address))"},{"line_number":86,"context_line":"                    self._producer.publish(self._topics[\u0027retry_topic\u0027],"},{"line_number":87,"context_line":"                                           [notification.to_json()])"}],"source_content_type":"text/x-python","patch_set":13,"id":"dada55a8_c27144aa","line":84,"range":{"start_line":84,"start_character":64,"end_line":84,"end_character":76},"updated":"2016-07-25 16:11:26.000000000","message":"If you\u0027re wrapping here, which I think is a good idea, then I find it more clear to have one entry per line rather than having some lines with multiple entries and some with one.","commit_id":"b888ec0401066dd518d11ee54489b40f5a590425"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"dde800470e0fc957d23b9e8b086947e81490a41f","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                    log.error(u\"retry failed for {} with name {} \""},{"line_number":90,"context_line":"                              u\"at {} after {} retries.  \""},{"line_number":91,"context_line":"                              u\"Giving up on retry.\""},{"line_number":92,"context_line":"                              .format(notification.type, notification.name, notification.address, self._retry_max))"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"            self._consumer.commit()"}],"source_content_type":"text/x-python","patch_set":13,"id":"dada55a8_e2a56804","line":92,"range":{"start_line":92,"start_character":38,"end_line":92,"end_character":50},"updated":"2016-07-25 16:11:26.000000000","message":"This should be wrapped like the line above.","commit_id":"b888ec0401066dd518d11ee54489b40f5a590425"}],"monasca_notification/types/notifiers.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"b4cdc6f10b2f6df5e7e40dc3eabc29b9684bee02","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            except Exception:"},{"line_number":70,"context_line":"                log.exception(\"config exception for {}\".format(ntype))"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            log.warn(\"No config data for type: {}\".format(ntype))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def send_notifications(notifications):"}],"source_content_type":"text/x-python","patch_set":25,"id":"1ac06dbe_e2347a4b","line":72,"range":{"start_line":72,"start_character":0,"end_line":72,"end_character":65},"updated":"2016-08-25 02:02:30.000000000","message":"We still need to add the test for formatted_config values that are not attached to types in configured_notifiers don\u0027t we?","commit_id":"43667d4373efeaaf6cdc9c87438a82df01f4843a"}]}