)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4c108b559b9e31d9398e3b3c2d5eaa5ad1a1e732","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Change 200-\u003e404 for the history of non-existent alarm"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Non-existent alarm history now returns 404 not found"},{"line_number":10,"context_line":"instead of 200 ok."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I49128d49823b6e5a9add944c0f915798facb0b6c"},{"line_number":13,"context_line":"Closes-Bug: #1288104"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAAXH%2F%2FUuE%3D","line":10,"updated":"2014-04-22 17:14:19.000000000","message":"The return values are inverted in this sentence.","commit_id":"9536876df5ba3b83b04ee9c8d372d3b13fbabab5"}],"ceilometer/api/controllers/v2.py":[{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4c108b559b9e31d9398e3b3c2d5eaa5ad1a1e732","unresolved":false,"context_lines":[{"line_number":1983,"context_line":"        changes_gen \u003d conn.get_alarm_changes(self._id, auth_project, **kwargs)"},{"line_number":1984,"context_line":"        changes \u003d [AlarmChange.from_db_model(ac) for ac in changes_gen]"},{"line_number":1985,"context_line":"        # check that alarm itself was found, as if there is an alarm, it\u0027ll be"},{"line_number":1986,"context_line":"        # at least creation record in its history"},{"line_number":1987,"context_line":"        if len(changes) \u003d\u003d 0:"},{"line_number":1988,"context_line":"            raise EntityNotFound(_(\u0027Alarm\u0027), self._id)"},{"line_number":1989,"context_line":"        return changes"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAXH%2F%2FU08%3D","line":1986,"updated":"2014-04-22 17:14:19.000000000","message":"As there is a query filter, you can never know why the returned list is empty. So there is no reason here to return 404, as the query was successful and the empty list is a valid output of a query which was defined by the user.\n\nA valid check could be here is to verify, that the query is empty, the alarm does not exist and the returned list is empty at the same time.","commit_id":"9536876df5ba3b83b04ee9c8d372d3b13fbabab5"},{"author":{"_account_id":3012,"name":"Dina Belova","email":"dbelova@mirantis.com","username":"dbelova"},"change_message_id":"ee6f037668a81bc805bdf477891347afd3a78eeb","unresolved":false,"context_lines":[{"line_number":1983,"context_line":"        changes_gen \u003d conn.get_alarm_changes(self._id, auth_project, **kwargs)"},{"line_number":1984,"context_line":"        changes \u003d [AlarmChange.from_db_model(ac) for ac in changes_gen]"},{"line_number":1985,"context_line":"        # check that alarm itself was found, as if there is an alarm, it\u0027ll be"},{"line_number":1986,"context_line":"        # at least creation record in its history"},{"line_number":1987,"context_line":"        if len(changes) \u003d\u003d 0:"},{"line_number":1988,"context_line":"            raise EntityNotFound(_(\u0027Alarm\u0027), self._id)"},{"line_number":1989,"context_line":"        return changes"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAXH%2F%2FSK8%3D","line":1986,"in_reply_to":"AAAAXH%2F%2FU08%3D","updated":"2014-04-22 18:52:56.000000000","message":"I\u0027ve wrote in the next patch set the following version: both query and list are empty (as if query is not empty, existing (and even then deleted) alarm history contains at least record about its creation)","commit_id":"9536876df5ba3b83b04ee9c8d372d3b13fbabab5"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6db7994c103a4c946760914ee9fae2faac4f6159","unresolved":false,"context_lines":[{"line_number":1983,"context_line":"        changes_gen \u003d conn.get_alarm_changes(self._id, auth_project, **kwargs)"},{"line_number":1984,"context_line":"        changes \u003d [AlarmChange.from_db_model(ac) for ac in changes_gen]"},{"line_number":1985,"context_line":"        # check that alarm itself was found, as if there is an alarm, it\u0027ll be"},{"line_number":1986,"context_line":"        # at least creation record in its history. This error will be raised"},{"line_number":1987,"context_line":"        # only if there was no query, as empty result might be correct is there"},{"line_number":1988,"context_line":"        # was history items simply do not match with query. Moreover, if the"},{"line_number":1989,"context_line":"        # alarm has been existing before and was removed, history still will"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FR1k%3D","line":1986,"updated":"2014-04-22 19:44:28.000000000","message":"The first sentence is not valid here, if alarm history is not collected at all.","commit_id":"b279b68daad619acb339e3b76226ec92f53dfd60"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6db7994c103a4c946760914ee9fae2faac4f6159","unresolved":false,"context_lines":[{"line_number":1984,"context_line":"        changes \u003d [AlarmChange.from_db_model(ac) for ac in changes_gen]"},{"line_number":1985,"context_line":"        # check that alarm itself was found, as if there is an alarm, it\u0027ll be"},{"line_number":1986,"context_line":"        # at least creation record in its history. This error will be raised"},{"line_number":1987,"context_line":"        # only if there was no query, as empty result might be correct is there"},{"line_number":1988,"context_line":"        # was history items simply do not match with query. Moreover, if the"},{"line_number":1989,"context_line":"        # alarm has been existing before and was removed, history still will"},{"line_number":1990,"context_line":"        # be stored in the storage."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FR1c%3D","line":1987,"updated":"2014-04-22 19:44:28.000000000","message":"s/is/if/","commit_id":"b279b68daad619acb339e3b76226ec92f53dfd60"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6db7994c103a4c946760914ee9fae2faac4f6159","unresolved":false,"context_lines":[{"line_number":1985,"context_line":"        # check that alarm itself was found, as if there is an alarm, it\u0027ll be"},{"line_number":1986,"context_line":"        # at least creation record in its history. This error will be raised"},{"line_number":1987,"context_line":"        # only if there was no query, as empty result might be correct is there"},{"line_number":1988,"context_line":"        # was history items simply do not match with query. Moreover, if the"},{"line_number":1989,"context_line":"        # alarm has been existing before and was removed, history still will"},{"line_number":1990,"context_line":"        # be stored in the storage."},{"line_number":1991,"context_line":"        if len(changes) \u003d\u003d 0 and q \u003d\u003d []:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FR1Q%3D","line":1988,"updated":"2014-04-22 19:44:28.000000000","message":"s/was/were/\n\nitems*, but* simply","commit_id":"b279b68daad619acb339e3b76226ec92f53dfd60"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6db7994c103a4c946760914ee9fae2faac4f6159","unresolved":false,"context_lines":[{"line_number":1989,"context_line":"        # alarm has been existing before and was removed, history still will"},{"line_number":1990,"context_line":"        # be stored in the storage."},{"line_number":1991,"context_line":"        if len(changes) \u003d\u003d 0 and q \u003d\u003d []:"},{"line_number":1992,"context_line":"            raise EntityNotFound(_(\u0027Alarm\u0027), self._id)"},{"line_number":1993,"context_line":"        return changes"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @wsme.validate(state_kind_enum)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FR2M%3D","line":1992,"updated":"2014-04-22 19:44:28.000000000","message":"If the alarm history is not stored, then this error message will not be valid, so something like (_(\u0027No history found for Alarm %s.\u0027), self._id) would be better.","commit_id":"b279b68daad619acb339e3b76226ec92f53dfd60"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"70bbd84de4112de1da3d727275e5bbf499f44f19","unresolved":false,"context_lines":[{"line_number":1988,"context_line":"        kwargs \u003d _query_to_kwargs(q, conn.get_alarm_changes, [\u0027on_behalf_of\u0027,"},{"line_number":1989,"context_line":"                                                              \u0027alarm_id\u0027])"},{"line_number":1990,"context_line":"        changes_gen \u003d conn.get_alarm_changes(self._id, auth_project, **kwargs)"},{"line_number":1991,"context_line":"        changes \u003d [AlarmChange.from_db_model(ac) for ac in changes_gen]"},{"line_number":1992,"context_line":""},{"line_number":1993,"context_line":"        # This error will be raised only if there was no query, as empty result"},{"line_number":1994,"context_line":"        # might be correct if there were history items, but they simply do not"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2FMxg%3D","line":1991,"updated":"2014-04-23 11:51:50.000000000","message":"Is changes_gen really needed?\n\ni.e. would this be equivalent (and less disruptive to the existing code):\n\n  changes \u003d [AlarmChange.from_db_model(ac)\n             for ac in conn.get_alarm_changes(self._id, auth_project,\n                                              **kwargs)]","commit_id":"1963ac78f40018e6d37d27e076a0bfad60ee3fa2"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"70bbd84de4112de1da3d727275e5bbf499f44f19","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"        # might be correct if there were history items, but they simply do not"},{"line_number":1995,"context_line":"        # match with query. Moreover, if the alarm has been existing before and"},{"line_number":1996,"context_line":"        # was removed, history still will be stored in the storage, so there"},{"line_number":1997,"context_line":"        # will be no error."},{"line_number":1998,"context_line":"        if len(changes) \u003d\u003d 0 and q \u003d\u003d []:"},{"line_number":1999,"context_line":"            raise HistoryNotFound(self._id)"},{"line_number":2000,"context_line":"        return changes"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2FMyc%3D","line":1997,"updated":"2014-04-23 11:51:50.000000000","message":"We may have a potential problem coming down the road if we move towards limiting the history retention period (in the same style as the configured TTL value driving sample expiry).","commit_id":"1963ac78f40018e6d37d27e076a0bfad60ee3fa2"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"70bbd84de4112de1da3d727275e5bbf499f44f19","unresolved":false,"context_lines":[{"line_number":1995,"context_line":"        # match with query. Moreover, if the alarm has been existing before and"},{"line_number":1996,"context_line":"        # was removed, history still will be stored in the storage, so there"},{"line_number":1997,"context_line":"        # will be no error."},{"line_number":1998,"context_line":"        if len(changes) \u003d\u003d 0 and q \u003d\u003d []:"},{"line_number":1999,"context_line":"            raise HistoryNotFound(self._id)"},{"line_number":2000,"context_line":"        return changes"},{"line_number":2001,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2FMzI%3D","line":1998,"updated":"2014-04-23 11:51:50.000000000","message":"Clearer and more pythonic as:\n\n  if not (changes or query):","commit_id":"1963ac78f40018e6d37d27e076a0bfad60ee3fa2"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"c559139f451ccf3223a4bf2d8cfecb9f7d93a12f","unresolved":false,"context_lines":[{"line_number":1995,"context_line":"        # match with query. Moreover, if the alarm has been existing before and"},{"line_number":1996,"context_line":"        # was removed, history still will be stored in the storage, so there"},{"line_number":1997,"context_line":"        # will be no error."},{"line_number":1998,"context_line":"        if len(changes) \u003d\u003d 0 and q \u003d\u003d []:"},{"line_number":1999,"context_line":"            raise HistoryNotFound(self._id)"},{"line_number":2000,"context_line":"        return changes"},{"line_number":2001,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2FMqo%3D","line":1998,"updated":"2014-04-23 12:01:12.000000000","message":"This doesn\u0027t catch the case where an alarm (deleted or not) have a history but no items in the history are returned by get_alarm_changes because the user is not allowed to see these items due to the ACL configuration. In this case the history exists but no changes is visible for the user.\n\nAnd also I don\u0027t like to have a different behavior if q is set or not.","commit_id":"1963ac78f40018e6d37d27e076a0bfad60ee3fa2"}],"ceilometer/tests/api/v2/test_alarm_scenarios.py":[{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6db7994c103a4c946760914ee9fae2faac4f6159","unresolved":false,"context_lines":[{"line_number":1520,"context_line":"        self.CONF.set_override(\u0027record_history\u0027, False, group\u003d\u0027alarm\u0027)"},{"line_number":1521,"context_line":"        alarm \u003d self._get_alarm(\u0027a\u0027)"},{"line_number":1522,"context_line":"        history \u003d self._get_alarm_history(alarm)"},{"line_number":1523,"context_line":"        self.assertEqual(1, len(history))"},{"line_number":1524,"context_line":"        self._assert_is_subset(dict(alarm_id\u003dalarm[\u0027alarm_id\u0027],"},{"line_number":1525,"context_line":"                                    on_behalf_of\u003dalarm[\u0027project_id\u0027],"},{"line_number":1526,"context_line":"                                    project_id\u003dalarm[\u0027project_id\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FR9s%3D","line":1523,"updated":"2014-04-22 19:44:28.000000000","message":"There shouldn\u0027t be any alarm history here, as this test checks, if the config option \u0027record_history\u0027 with the value set to False is working. This config option defines whether the alarm history should be recorded or not and the default value of this is True.\n\nIn this case, when there isn\u0027t any existing alarm nor alarm history the added value could be to return a warning about this config option is set to False. Returning a text that the query returned an empty list as there is no alarm with the specified id, does not add more info, than the simple empty response itself.","commit_id":"b279b68daad619acb339e3b76226ec92f53dfd60"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6db7994c103a4c946760914ee9fae2faac4f6159","unresolved":false,"context_lines":[{"line_number":1693,"context_line":"                \u0027X-Project-Id\u0027: str(uuid.uuid4())}"},{"line_number":1694,"context_line":""},{"line_number":1695,"context_line":"        # here user with random creds won\u0027t find any alarm and its history"},{"line_number":1696,"context_line":"        history \u003d self._get_alarm_history(self._get_alarm(\u0027a\u0027), auth,"},{"line_number":1697,"context_line":"                                          expect_errors\u003dTrue, status\u003d404)"},{"line_number":1698,"context_line":""},{"line_number":1699,"context_line":"    def test_get_recorded_alarm_history_preserved_after_deletion(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FR6E%3D","line":1696,"updated":"2014-04-22 19:44:28.000000000","message":"I think here the returned error message misleading a bit, as the alarm itself exists, but the user does not have the permission to see it. I know that the empty list shows something similar, but it does not explicitly say why the returned list is empty, which is better in this case IMHO.","commit_id":"b279b68daad619acb339e3b76226ec92f53dfd60"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"70bbd84de4112de1da3d727275e5bbf499f44f19","unresolved":false,"context_lines":[{"line_number":180,"context_line":"             \u0027on_behalf_of\u0027: self.auth_headers[\u0027X-Project-Id\u0027],"},{"line_number":181,"context_line":"             \u0027timestamp\u0027: datetime.datetime.utcnow()},"},{"line_number":182,"context_line":"        ]:"},{"line_number":183,"context_line":"            self.conn.record_alarm_change(history_item)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @staticmethod"},{"line_number":186,"context_line":"    def _add_default_threshold_rule(alarm):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2FMus%3D","line":183,"updated":"2014-04-23 11:51:50.000000000","message":"More concise and clearer as something like:\n\n    for event, alarm in [(a, a), (b, b), (c, c), (d, d)]:\n        item \u003d dict(event_id\u003devent, alarm_id\u003dalarm, type\u003d\u0027creation\u0027,\n                    \u0027detail\u0027: \u0027{\"name\": \"%s\"}\u0027 % alarm,\n                    user_id\u003dself.auth_headers[\u0027X-User-Id\u0027],\n                    project_id\u003dself.auth_headers[\u0027X-Project-Id\u0027],\n                    on_behalf_of\u003dself.auth_headers[\u0027X-Project-Id\u0027],\n                    timestamp\u003ddatetime.datetime.utcnow())\n        self.conn.record_alarm_change(item)","commit_id":"1963ac78f40018e6d37d27e076a0bfad60ee3fa2"}]}