)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"ce28bae6474e1fd4f275612a0dc8592c8a2ddb63","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ildiko Vancsa \u003cildiko.vancsa@ericsson.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-02-18 17:43:59 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor timestamp exsitence validation in V2 API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There was a bug in AlarmThresholdRule in v2.py, the timestamp_keys"},{"line_number":10,"context_line":"were defined with a missing comma:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"AAAAUX%2F%2Bwdk%3D","line":7,"updated":"2014-02-19 09:10:48.000000000","message":"existence","commit_id":"64ebf9db803ebbcb93f99e319012677f039b4f88"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"cc2bdf5b0d649ac24aad581bdbdfbba650976b6d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ildiko Vancsa \u003cildiko.vancsa@ericsson.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-02-18 17:43:59 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor timestamp exsitence validation in V2 API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There was a bug in AlarmThresholdRule in v2.py, the timestamp_keys"},{"line_number":10,"context_line":"were defined with a missing comma:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"AAAAUX%2F%2BwJU%3D","line":7,"in_reply_to":"AAAAUX%2F%2Bwdk%3D","updated":"2014-02-19 10:01:02.000000000","message":"Done","commit_id":"64ebf9db803ebbcb93f99e319012677f039b4f88"}],"ceilometer/api/controllers/v2.py":[{"author":{"_account_id":9684,"name":"Jia Dong","email":"joey5566@yeah.net","username":"joey5678"},"change_message_id":"324841adad1770ae2b52141a76d584cc8d58e896","unresolved":false,"context_lines":[{"line_number":1305,"context_line":"        if not threshold_rule.query:"},{"line_number":1306,"context_line":"            threshold_rule.query \u003d []"},{"line_number":1307,"context_line":""},{"line_number":1308,"context_line":"        timestamp_keys \u003d [\u0027timestamp\u0027, \u0027start\u0027, \u0027end\u0027]"},{"line_number":1309,"context_line":"        _validate_query(threshold_rule.query, storage.SampleFilter.__init__,"},{"line_number":1310,"context_line":"                        internal_keys\u003dtimestamp_keys)"},{"line_number":1311,"context_line":"        return threshold_rule"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2FHhg%3D","line":1308,"updated":"2014-02-17 01:21:01.000000000","message":"I think it should be: timestamp_keys \u003d [\u0027timestamp\u0027, \u0027start\u0027, \u0027start_timestamp_op\u0027, \u0027end\u0027,\u0027end_timestamp_op\u0027], what do you think?","commit_id":"f0b4fd31db9dff77eb96029befd2a11a692b2a75"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"e664368348807cc5c47aabb4c840c42b24d7d9bd","unresolved":false,"context_lines":[{"line_number":1305,"context_line":"        if not threshold_rule.query:"},{"line_number":1306,"context_line":"            threshold_rule.query \u003d []"},{"line_number":1307,"context_line":""},{"line_number":1308,"context_line":"        timestamp_keys \u003d [\u0027timestamp\u0027, \u0027start\u0027, \u0027end\u0027]"},{"line_number":1309,"context_line":"        _validate_query(threshold_rule.query, storage.SampleFilter.__init__,"},{"line_number":1310,"context_line":"                        internal_keys\u003dtimestamp_keys)"},{"line_number":1311,"context_line":"        return threshold_rule"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2FFTQ%3D","line":1308,"in_reply_to":"AAAAUX%2F%2FHhg%3D","updated":"2014-02-17 08:46:14.000000000","message":"We do not use the information that these field names are in this timestamp_keys list, that is why I did not include them. In my view, if those will be needed to be here, then this modification should be done when this is decided and also additional test cases should be added, to check the functionality.","commit_id":"f0b4fd31db9dff77eb96029befd2a11a692b2a75"},{"author":{"_account_id":2860,"name":"litong01","email":"email4tong@yahoo.com","username":"litong01"},"change_message_id":"f920e14509faea1ffa116116b2afe067615c4f25","unresolved":false,"context_lines":[{"line_number":315,"context_line":""},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"def _validate_query(query, db_func, internal_keys\u003d[],"},{"line_number":318,"context_line":"                    timestamp_is_valid\u003dTrue):"},{"line_number":319,"context_line":"    _verify_query_segregation(query)"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"    valid_keys \u003d inspect.getargspec(db_func)[0]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2FAGo%3D","line":318,"updated":"2014-02-17 18:04:19.000000000","message":"can you change it to valid_timestamp?\ntimestamp_is_valid is more like a statement. and hard to read in the if \nstatements later on","commit_id":"9db4f2626be8b9812f3f877b9d9314b31efe2617"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"acef34addf7ae073f5867305b11345d11d95ea10","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                                            \u0027unimplemented operator for %s\u0027 %"},{"line_number":334,"context_line":"                                            i.field)"},{"line_number":335,"context_line":"        else:"},{"line_number":336,"context_line":"            if i.op \u003d\u003d \u0027eq\u0027:"},{"line_number":337,"context_line":"                if i.field \u003d\u003d \u0027search_offset\u0027:"},{"line_number":338,"context_line":"                    has_timestamp \u003d True"},{"line_number":339,"context_line":"                elif i.field \u003d\u003d \u0027enabled\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2B%2Blc%3D","line":336,"updated":"2014-02-17 21:24:28.000000000","message":"Since you\u0027re fixing up this whole timestamp validation logic anyway, this line also smells a bit suspicious.\n\ni.e. should the has_timestamp flag be set to true if the query includes a search_offset but no actual timestamp to offset from?\n\nThat case would seem like a mal-formed query to me.","commit_id":"fae2a7647f71229c4d8f493cd38db234e4aa516f"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"0fe7a23ecd7c4a1766382c58595ec1a19bb1863f","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                                            \u0027unimplemented operator for %s\u0027 %"},{"line_number":334,"context_line":"                                            i.field)"},{"line_number":335,"context_line":"        else:"},{"line_number":336,"context_line":"            if i.op \u003d\u003d \u0027eq\u0027:"},{"line_number":337,"context_line":"                if i.field \u003d\u003d \u0027search_offset\u0027:"},{"line_number":338,"context_line":"                    has_timestamp \u003d True"},{"line_number":339,"context_line":"                elif i.field \u003d\u003d \u0027enabled\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2B5eE%3D","line":336,"in_reply_to":"AAAAUX%2F%2B%2Blc%3D","updated":"2014-02-18 10:34:18.000000000","message":"BTW by \"this line\" I meant lines 337-8 below ...\n\nvvv","commit_id":"fae2a7647f71229c4d8f493cd38db234e4aa516f"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"1272c345bb9512ae0cb297da2f502a329cbe4060","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                                            \u0027unimplemented operator for %s\u0027 %"},{"line_number":334,"context_line":"                                            i.field)"},{"line_number":335,"context_line":"        else:"},{"line_number":336,"context_line":"            if i.op \u003d\u003d \u0027eq\u0027:"},{"line_number":337,"context_line":"                if i.field \u003d\u003d \u0027search_offset\u0027:"},{"line_number":338,"context_line":"                    has_timestamp \u003d True"},{"line_number":339,"context_line":"                elif i.field \u003d\u003d \u0027enabled\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2B2HU%3D","line":336,"in_reply_to":"AAAAUX%2F%2B5eE%3D","updated":"2014-02-19 01:27:29.000000000","message":"Yes, I got it. :) I refactored a bit more this part, I will upload it in the next patch set. I check if there is a timestamp, if the search_offset is set in the query and I also check if the timestamp is supported or not, etc. But I wrote some new test cases. :)","commit_id":"fae2a7647f71229c4d8f493cd38db234e4aa516f"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"acef34addf7ae073f5867305b11345d11d95ea10","unresolved":false,"context_lines":[{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"        #Timestamp is not allowed for AlarmThresholdRule query, as the alarm"},{"line_number":1315,"context_line":"        #evaluator will construct timestamp bounds for the actual statistics"},{"line_number":1316,"context_line":"        #query."},{"line_number":1317,"context_line":"        _validate_query(threshold_rule.query, storage.SampleFilter.__init__,"},{"line_number":1318,"context_line":"                        is_timestamp_valid\u003dFalse)"},{"line_number":1319,"context_line":"        return threshold_rule"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2B%2BpE%3D","line":1316,"updated":"2014-02-17 21:24:28.000000000","message":"s/actual statistics query/sequence of statistics queries as the sliding evaluation window advances over time./","commit_id":"fae2a7647f71229c4d8f493cd38db234e4aa516f"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"1272c345bb9512ae0cb297da2f502a329cbe4060","unresolved":false,"context_lines":[{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"        #Timestamp is not allowed for AlarmThresholdRule query, as the alarm"},{"line_number":1315,"context_line":"        #evaluator will construct timestamp bounds for the actual statistics"},{"line_number":1316,"context_line":"        #query."},{"line_number":1317,"context_line":"        _validate_query(threshold_rule.query, storage.SampleFilter.__init__,"},{"line_number":1318,"context_line":"                        is_timestamp_valid\u003dFalse)"},{"line_number":1319,"context_line":"        return threshold_rule"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2B3TU%3D","line":1316,"in_reply_to":"AAAAUX%2F%2B%2BpE%3D","updated":"2014-02-19 01:27:29.000000000","message":"Done","commit_id":"fae2a7647f71229c4d8f493cd38db234e4aa516f"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"3f9b607355ad72ee7eef02e916178060ace98911","unresolved":false,"context_lines":[{"line_number":358,"context_line":"                                            i.field)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"def _has_valid_timestamp_query(query, is_timestamp_valid):"},{"line_number":362,"context_line":"    return _validate_timestamp_fields(query,"},{"line_number":363,"context_line":"                                      \u0027timestamp\u0027,"},{"line_number":364,"context_line":"                                      (\u0027lt\u0027, \u0027le\u0027, \u0027gt\u0027, \u0027ge\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bv0E%3D","line":361,"updated":"2014-02-19 10:46:09.000000000","message":"TBH I don\u0027t think these two methods really add much, better IMO to just twice call _validate_timestamp_fields appropriately parameterized from _validate_query","commit_id":"cb9db558e22da2286e039fb0d52b1e2c5524314e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"bc7235a2abc6837552be3f3f3bfb498e4feb350c","unresolved":false,"context_lines":[{"line_number":358,"context_line":"                                            i.field)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"def _has_valid_timestamp_query(query, is_timestamp_valid):"},{"line_number":362,"context_line":"    return _validate_timestamp_fields(query,"},{"line_number":363,"context_line":"                                      \u0027timestamp\u0027,"},{"line_number":364,"context_line":"                                      (\u0027lt\u0027, \u0027le\u0027, \u0027gt\u0027, \u0027ge\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Buok%3D","line":361,"in_reply_to":"AAAAUX%2F%2Bv0E%3D","updated":"2014-02-19 14:39:56.000000000","message":"Yes, you have a point here, so I removed these two functions and I use only the _validate_timestamp_fields function.\n\nDone","commit_id":"cb9db558e22da2286e039fb0d52b1e2c5524314e"}]}