)]}'
{"monasca_notification/notification.py":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"7c0d7ff391c6a8cd9730ca2b15b49214c13ffb8d","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        self.lifecycle_state \u003d alarm[\u0027lifecycleState\u0027]"},{"line_number":75,"context_line":"        self.tenant_id \u003d alarm[\u0027tenantId\u0027]"},{"line_number":76,"context_line":"        self.metrics \u003d alarm[\u0027metrics\u0027]"},{"line_number":77,"context_line":"        self.value_meta \u003d alarm[\u0027valueMeta\u0027]"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        # to be updated on actual notification send time"},{"line_number":80,"context_line":"        self.notification_timestamp \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_b6f57685","line":77,"range":{"start_line":77,"start_character":13,"end_line":77,"end_character":23},"updated":"2018-09-17 15:09:24.000000000","message":"If we run this service with an old storm topology won\u0027t this fail? Can we use alarm.get(\u0027valueMeta\u0027) so it defaults to None?","commit_id":"dc3438104e9e03edc6d10e4feaf8bc1c1064e570"},{"author":{"_account_id":16222,"name":"witek","email":"witold.bedyk@suse.com","username":"witek"},"change_message_id":"9dd305629db9cf1624051d369181a4e8c5bc7128","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        self.metrics \u003d alarm[\u0027metrics\u0027]"},{"line_number":77,"context_line":"        self.value_meta \u003d None"},{"line_number":78,"context_line":"        if alarm.get(\u0027valueMeta\u0027) is not None:"},{"line_number":79,"context_line":"            for subAlarmId, subAlarmObject in alarm[\u0027valueMeta\u0027].items():"},{"line_number":80,"context_line":"                if subAlarmObject.get(\u0027state\u0027) \u003d\u003d \u0027ALARM\u0027:"},{"line_number":81,"context_line":"                    self.value_meta \u003d subAlarmObject.get(\u0027value_meta\u0027)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # to be updated on actual notification send time"},{"line_number":84,"context_line":"        self.notification_timestamp \u003d None"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_1a43596e","line":81,"range":{"start_line":79,"start_character":12,"end_line":81,"end_character":70},"updated":"2018-09-27 12:59:58.000000000","message":"Please compare my comments in monasca-thresh. I think we should simplify the way how valueMeta is stored in Kafka message. AlarmStateTransitionedEvent includes already `transitionSubAlarms` which we could use here. `valueMeta` could similarly be stored as simple ArrayList. `sub_expression` field which you have added in Alarm object isn\u0027t even used here and can be removed. Additionally having `currentValues` would be nice.","commit_id":"df5b3921d3a888e35dcd710df8889699785db537"}],"monasca_notification/processors/alarm_processor.py":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"7c0d7ff391c6a8cd9730ca2b15b49214c13ffb8d","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            if field not in alarm:"},{"line_number":67,"context_line":"                raise notification_exceptions.AlarmFormatError("},{"line_number":68,"context_line":"                    \u0027Alarm data missing field %s\u0027 % field)"},{"line_number":69,"context_line":"            if field \u003d\u003d \u0027valueMeta\u0027 and alarm[\u0027valueMeta\u0027] is not None:"},{"line_number":70,"context_line":"                alarm[\u0027valueMeta\u0027] \u003d \\"},{"line_number":71,"context_line":"                    json.loads(alarm[\u0027valueMeta\u0027].replace(\"\\\\\", \"\"))"},{"line_number":72,"context_line":"        if (\u0027tenantId\u0027 not in alarm) or (\u0027alarmId\u0027 not in alarm):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_9675dace","line":69,"range":{"start_line":69,"start_character":15,"end_line":69,"end_character":20},"updated":"2018-09-17 15:09:24.000000000","message":"I think we should move this out of the for loop. We could put it below at which stage we know the valueMeta field exists so we can just do something like:\n\n# Comment about why we replace \\\\ ..\nif alarm[\u0027valueMeta\u0027]:\n    alarm[\u0027valueMeta\u0027] \u003d \n    json.loads(alarm[\u0027valueMeta\u0027].replace(\"\\\\\", \"\"))","commit_id":"dc3438104e9e03edc6d10e4feaf8bc1c1064e570"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b9508eb9b2b5262bc5b9d3ad4a6458b185cb5c14","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            if field not in alarm:"},{"line_number":67,"context_line":"                raise notification_exceptions.AlarmFormatError("},{"line_number":68,"context_line":"                    \u0027Alarm data missing field %s\u0027 % field)"},{"line_number":69,"context_line":"        if alarm.get(\u0027valueMeta\u0027) is not None:"},{"line_number":70,"context_line":"            alarm[\u0027valueMeta\u0027] \u003d \\"},{"line_number":71,"context_line":"                json.loads(alarm[\u0027valueMeta\u0027].replace(\"\\\\\", \"\"))"},{"line_number":72,"context_line":"        if (\u0027tenantId\u0027 not in alarm) or (\u0027alarmId\u0027 not in alarm):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ee86a7ba","line":69,"updated":"2018-09-18 09:13:16.000000000","message":"Please add a comment about why we do this.","commit_id":"654ef5598fc5de889b58b9cbfe4044f8ae9abd08"}],"tests/test_alarm_processor.py":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"7c0d7ff391c6a8cd9730ca2b15b49214c13ffb8d","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            \"severity\": \"LOW\","},{"line_number":101,"context_line":"            \"link\": \"http://some-place.com\","},{"line_number":102,"context_line":"            \"lifecycleState\": \"OPEN\","},{"line_number":103,"context_line":"            \"valueMeta\": None}"},{"line_number":104,"context_line":"        alarm \u003d self._create_raw_alarm(0, 2, alarm_dict)"},{"line_number":105,"context_line":"        expected_datetime \u003d time.ctime(timestamp / 1000)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_b62016b2","line":103,"range":{"start_line":103,"start_character":13,"end_line":103,"end_character":22},"updated":"2018-09-17 15:09:24.000000000","message":"Please can we add a test for when this isn\u0027t None to check the parsing works?","commit_id":"dc3438104e9e03edc6d10e4feaf8bc1c1064e570"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b9508eb9b2b5262bc5b9d3ad4a6458b185cb5c14","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        self.assertEqual(partition, 0)"},{"line_number":143,"context_line":"        self.assertEqual(offset, 3)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def test_valid_notification(self):"},{"line_number":146,"context_line":"        \"\"\"Test a valid notification, being put onto the notification_queue"},{"line_number":147,"context_line":"        \"\"\""},{"line_number":148,"context_line":"        value_meta_obj \u003d {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ce188bce","line":145,"updated":"2018-09-18 09:13:16.000000000","message":"Please can we add new tests rather than modifying the old ones so that we know this still works when valueMeta is None?","commit_id":"654ef5598fc5de889b58b9cbfe4044f8ae9abd08"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b9508eb9b2b5262bc5b9d3ad4a6458b185cb5c14","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        value_meta_obj \u003d {"},{"line_number":188,"context_line":"            \"408a90c3-0dec-401b-ae7e-2a7d12c93304\": {"},{"line_number":189,"context_line":"                \"state\": \"ALARM\","},{"line_number":190,"context_line":"                \"sub_expression\": \"avg(log.haproxy.info) \u003c 3.0\","},{"line_number":191,"context_line":"                \"value_meta\": {\"rc\": \"1\"}"},{"line_number":192,"context_line":"            }"},{"line_number":193,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_8ee053ce","line":190,"updated":"2018-09-18 09:13:16.000000000","message":"Why does the valuemeta object contain the alarm definition?","commit_id":"654ef5598fc5de889b58b9cbfe4044f8ae9abd08"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b9508eb9b2b5262bc5b9d3ad4a6458b185cb5c14","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            \"408a90c3-0dec-401b-ae7e-2a7d12c93304\": {"},{"line_number":189,"context_line":"                \"state\": \"ALARM\","},{"line_number":190,"context_line":"                \"sub_expression\": \"avg(log.haproxy.info) \u003c 3.0\","},{"line_number":191,"context_line":"                \"value_meta\": {\"rc\": \"1\"}"},{"line_number":192,"context_line":"            }"},{"line_number":193,"context_line":"        }"},{"line_number":194,"context_line":"        alarm_dict \u003d {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4e823bc9","line":191,"range":{"start_line":191,"start_character":17,"end_line":191,"end_character":27},"updated":"2018-09-18 09:13:16.000000000","message":"What happens when this is a map with multiple entries? Can we add a test for that?","commit_id":"654ef5598fc5de889b58b9cbfe4044f8ae9abd08"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b9508eb9b2b5262bc5b9d3ad4a6458b185cb5c14","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            \"severity\": \"LOW\","},{"line_number":206,"context_line":"            \"link\": \"http://some-place.com\","},{"line_number":207,"context_line":"            \"lifecycleState\": \"OPEN\","},{"line_number":208,"context_line":"            \"valueMeta\": ujson.dumps(value_meta_obj)"},{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        alarm \u003d self._create_raw_alarm(0, 5, alarm_dict)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ce25ebc0","line":208,"range":{"start_line":208,"start_character":25,"end_line":208,"end_character":30},"updated":"2018-09-18 09:13:16.000000000","message":"Can we not rely on _create_raw_alarm to dump this?","commit_id":"654ef5598fc5de889b58b9cbfe4044f8ae9abd08"}]}