)]}'
{"ceilometer/api/controllers/v2.py":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"84a37c5af490af15e6946c2581c426457c3a5aa5","unresolved":false,"context_lines":[{"line_number":980,"context_line":""},{"line_number":981,"context_line":"        aggregate \u003d utils.uniq(aggregate, [\u0027func\u0027, \u0027param\u0027])"},{"line_number":982,"context_line":"        try:"},{"line_number":983,"context_line":"            computed \u003d pecan.request.storage_conn.get_meter_statistics("},{"line_number":984,"context_line":"                f,"},{"line_number":985,"context_line":"                period,"},{"line_number":986,"context_line":"                g,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_b0eda9b9","line":983,"updated":"2014-11-14 02:00:42.000000000","message":"Can you paste the stack trace of this OverFlowError? Then we can catch that error in specific line, not try this large block. Or if the OverFlowError may caused by the storage call and the large list expression, then we can consider move L90-L1000 to L982.","commit_id":"8e9eb418f20864baa7f37cd82388ee755b140fd1"},{"author":{"_account_id":12637,"name":"Srinivas Sakhamuri","email":"srini.openstack@gmail.com","username":"srsakhamuri"},"change_message_id":"f61419931fa039daae03fd687a7a2e2cd8a858c6","unresolved":false,"context_lines":[{"line_number":980,"context_line":""},{"line_number":981,"context_line":"        aggregate \u003d utils.uniq(aggregate, [\u0027func\u0027, \u0027param\u0027])"},{"line_number":982,"context_line":"        try:"},{"line_number":983,"context_line":"            computed \u003d pecan.request.storage_conn.get_meter_statistics("},{"line_number":984,"context_line":"                f,"},{"line_number":985,"context_line":"                period,"},{"line_number":986,"context_line":"                g,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_0b9d6b0b","line":983,"in_reply_to":"5a890539_b0eda9b9","updated":"2014-11-14 04:00:59.000000000","message":"The error depending on database implementation occuring either in L983 or the generator 1003. So it is necessary to keep both in the exception  block.","commit_id":"8e9eb418f20864baa7f37cd82388ee755b140fd1"},{"author":{"_account_id":7729,"name":"Ilya Tyaptin","email":"ityaptin@mirantis.com","username":"ityaptin"},"change_message_id":"36207238000e9795435f6a02ca2eb42e4cd249bf","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"                               end_timestamp\u003dend,"},{"line_number":1003,"context_line":"                               **c.as_dict())"},{"line_number":1004,"context_line":"                    for c in computed]"},{"line_number":1005,"context_line":"        except (OverflowError) as e:"},{"line_number":1006,"context_line":"            raise ClientSideError(_(\"Invalid period: %s\") % e.message)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_e494c08c","line":1005,"updated":"2014-11-14 14:40:50.000000000","message":"I think parentheses are redundant here","commit_id":"8e9eb418f20864baa7f37cd82388ee755b140fd1"},{"author":{"_account_id":10987,"name":"Igor Degtiarov","username":"tovstun","inactive":true},"change_message_id":"fb43ac13ba8cdb0451f50ff810ba1efae72f30c6","unresolved":false,"context_lines":[{"line_number":1003,"context_line":"                               **c.as_dict())"},{"line_number":1004,"context_line":"                    for c in computed]"},{"line_number":1005,"context_line":"        except OverflowError as e:"},{"line_number":1006,"context_line":"            raise ClientSideError(_(\"Invalid period: %s\") % e.message)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"class Meter(_Base):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_cab18b90","line":1006,"updated":"2014-11-14 15:24:26.000000000","message":"It seems that s/e.message/e would be better.","commit_id":"b7f845b3179aa44bd932c947aaa6994d4732d421"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"715a1745fcd8067c9eae039cbc72d8c9474604b5","unresolved":false,"context_lines":[{"line_number":1003,"context_line":"                               **c.as_dict())"},{"line_number":1004,"context_line":"                    for c in computed]"},{"line_number":1005,"context_line":"        except OverflowError as e:"},{"line_number":1006,"context_line":"            raise ClientSideError(_(\"Invalid period: %s\") % e)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"class Meter(_Base):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_121645a8","line":1006,"updated":"2014-11-17 12:29:01.000000000","message":"It\u0027s fantastic to see these types of errors trapped. 500 should never happen based on user input.\n\nTo make this error message a little bit better would it be possible to put the the value of the period in the message:\n\n  \"Invalid period %s: %s\" % (period, e)","commit_id":"baa0c62ead2ada13228f4cbd0bf75c7af760fce1"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"b3ddc7cdfdc148123f3a51fa41bd8036c1e4a957","unresolved":false,"context_lines":[{"line_number":987,"context_line":"                aggregate)"},{"line_number":988,"context_line":"            LOG.debug(_(\u0027computed value coming from %r\u0027),"},{"line_number":989,"context_line":"                      pecan.request.storage_conn)"},{"line_number":990,"context_line":"            # Find the original timestamp in the query to use for clamping"},{"line_number":991,"context_line":"            # the duration returned in the statistics."},{"line_number":992,"context_line":"            start \u003d end \u003d None"},{"line_number":993,"context_line":"            for i in q:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_38aeb72e","line":990,"updated":"2014-11-17 17:51:34.000000000","message":"I think there is no need to put L990-L1000 in this try block, it is not quite easy to read, how about put them outside the try...except block, as I suggested in previous comment?","commit_id":"d9d61f2526aaf8f8a433da0cd01e3954b2ecf832"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"b3ddc7cdfdc148123f3a51fa41bd8036c1e4a957","unresolved":false,"context_lines":[{"line_number":1003,"context_line":"                               **c.as_dict())"},{"line_number":1004,"context_line":"                    for c in computed]"},{"line_number":1005,"context_line":"        except OverflowError as e:"},{"line_number":1006,"context_line":"            raise ClientSideError(_(\"Invalid period %s: %s\") % (period, e))"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"class Meter(_Base):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_984deb9c","line":1006,"updated":"2014-11-17 17:51:34.000000000","message":"I don\u0027t think large period is the only reason can cause the OverflowError, Overflow because data amount is too large, but if we truly have large amount of data and timestamp is right start from the beginning till the end, it also can cause overflow issue.\n\nnit: consider %d for period?","commit_id":"d9d61f2526aaf8f8a433da0cd01e3954b2ecf832"},{"author":{"_account_id":10987,"name":"Igor Degtiarov","username":"tovstun","inactive":true},"change_message_id":"4490074ba333cd854df071171662dd3a21501ae0","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                period,"},{"line_number":986,"context_line":"                g,"},{"line_number":987,"context_line":"                aggregate)"},{"line_number":988,"context_line":"            LOG.debug(_(\u0027computed value coming from %r\u0027),"},{"line_number":989,"context_line":"                      pecan.request.storage_conn)"},{"line_number":990,"context_line":"        except OverflowError as e:"},{"line_number":991,"context_line":"            params \u003d dict(period\u003dperiod, err\u003de)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_7f9ef7b9","line":988,"updated":"2014-11-20 10:05:05.000000000","message":"Do you think that repr is better then str here?","commit_id":"25512ad115dfc00dc77dfc61142f513f60621c3d"},{"author":{"_account_id":12637,"name":"Srinivas Sakhamuri","email":"srini.openstack@gmail.com","username":"srsakhamuri"},"change_message_id":"e29ce50383bff0d3f6532804f84b1ba2ba0be70c","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                period,"},{"line_number":986,"context_line":"                g,"},{"line_number":987,"context_line":"                aggregate)"},{"line_number":988,"context_line":"            LOG.debug(_(\u0027computed value coming from %r\u0027),"},{"line_number":989,"context_line":"                      pecan.request.storage_conn)"},{"line_number":990,"context_line":"        except OverflowError as e:"},{"line_number":991,"context_line":"            params \u003d dict(period\u003dperiod, err\u003de)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_7c9fd203","line":988,"in_reply_to":"5a890539_7f9ef7b9","updated":"2014-11-20 17:23:55.000000000","message":"I haven\u0027t changed this line just intended it. \n\nTypically repr format is preferred for debug statement according to this post, so let\u0027s keep it as is. \n\nhttp://stackoverflow.com/questions/1436703/difference-between-str-and-repr-in-python","commit_id":"25512ad115dfc00dc77dfc61142f513f60621c3d"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"d1b364b1ffaa735ca03c02eb5b67f2680df39467","unresolved":false,"context_lines":[{"line_number":991,"context_line":"            params \u003d dict(period\u003dperiod, err\u003de)"},{"line_number":992,"context_line":"            raise ClientSideError(_(\"Invalid period %(period)s: %(err)s\")"},{"line_number":993,"context_line":"                                  % params)"},{"line_number":994,"context_line":"        # Find the original timestamp in the query to use for clamping"},{"line_number":995,"context_line":"        # the duration returned in the statistics."},{"line_number":996,"context_line":"        start \u003d end \u003d None"},{"line_number":997,"context_line":"        for i in q:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_e8f69d5c","line":994,"updated":"2014-11-20 17:00:18.000000000","message":"I think you misunderstand my comment, but maybe my word is not clear enough, sorry.\n\nWhat I mean is L994-L1004 only calculate start and end variables, L983-L989 doesn\u0027t use it. So we can put L994-L1004 in front of L 982, and put the two statements, L983-L989, L1006-L1009 to a single try...except block","commit_id":"25512ad115dfc00dc77dfc61142f513f60621c3d"},{"author":{"_account_id":12637,"name":"Srinivas Sakhamuri","email":"srini.openstack@gmail.com","username":"srsakhamuri"},"change_message_id":"e29ce50383bff0d3f6532804f84b1ba2ba0be70c","unresolved":false,"context_lines":[{"line_number":991,"context_line":"            params \u003d dict(period\u003dperiod, err\u003de)"},{"line_number":992,"context_line":"            raise ClientSideError(_(\"Invalid period %(period)s: %(err)s\")"},{"line_number":993,"context_line":"                                  % params)"},{"line_number":994,"context_line":"        # Find the original timestamp in the query to use for clamping"},{"line_number":995,"context_line":"        # the duration returned in the statistics."},{"line_number":996,"context_line":"        start \u003d end \u003d None"},{"line_number":997,"context_line":"        for i in q:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_dc58c659","line":994,"in_reply_to":"5a890539_e8f69d5c","updated":"2014-11-20 17:23:55.000000000","message":"Done","commit_id":"25512ad115dfc00dc77dfc61142f513f60621c3d"},{"author":{"_account_id":8052,"name":"Nejc Saje","email":"nejc@saje.info","username":"nsaje"},"change_message_id":"9b9d81782399b1047ef1c44a809efed2ed2a0811","unresolved":false,"context_lines":[{"line_number":995,"context_line":"                f,"},{"line_number":996,"context_line":"                period,"},{"line_number":997,"context_line":"                g,"},{"line_number":998,"context_line":"                aggregate)"},{"line_number":999,"context_line":"            LOG.debug(_(\u0027computed value coming from %r\u0027),"},{"line_number":1000,"context_line":"                      pecan.request.storage_conn)"},{"line_number":1001,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_8123ee56","line":998,"updated":"2014-11-24 08:40:38.000000000","message":"These parameters don\u0027t need to be on separate lines - it would be more readable if they were only in one.","commit_id":"d72aad36ccd22b941cd7bac0365353b129b3e0a0"}],"ceilometer/tests/api/v2/test_statistics_scenarios.py":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"b3ddc7cdfdc148123f3a51fa41bd8036c1e4a957","unresolved":false,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def test_period_with_large_value(self):"},{"line_number":184,"context_line":"        resp \u003d self.get_json(self.PATH, expect_errors\u003dTrue,"},{"line_number":185,"context_line":"                             q\u003d[{\u0027field\u0027: \u0027user-id\u0027,"},{"line_number":186,"context_line":"                                 \u0027value\u0027: \u0027user-id\u0027}],"},{"line_number":187,"context_line":"                             period\u003d10000000000000)"},{"line_number":188,"context_line":"        self.assertEqual(400, resp.status_code)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_acc26850","line":185,"updated":"2014-11-17 17:51:34.000000000","message":"user-id is not an valid field, use `user_id` or `user` instead","commit_id":"d9d61f2526aaf8f8a433da0cd01e3954b2ecf832"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"b3ddc7cdfdc148123f3a51fa41bd8036c1e4a957","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        resp \u003d self.get_json(self.PATH, expect_errors\u003dTrue,"},{"line_number":185,"context_line":"                             q\u003d[{\u0027field\u0027: \u0027user-id\u0027,"},{"line_number":186,"context_line":"                                 \u0027value\u0027: \u0027user-id\u0027}],"},{"line_number":187,"context_line":"                             period\u003d10000000000000)"},{"line_number":188,"context_line":"        self.assertEqual(400, resp.status_code)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_start_timestamp(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_ccbdf4d9","line":187,"updated":"2014-11-17 17:51:34.000000000","message":"The 400 is not raised for OverflowError, is raised because date value out of change, please check the resp.body for more detail. it would be better if you add another check such as: self.assertEqual(\u0027xxx\u0027, json.loads(resp.body)[\u0027error_message\u0027][\u0027faultstring\u0027]) (note you should import json firstly...)\n\nPeriod is int type, this value is too large for int on 32-bits system, it may cause issue, but I think it is not a block issue for this patch, I can upload another one after this one is merged and if it does cause issue on 32-bits system.....","commit_id":"d9d61f2526aaf8f8a433da0cd01e3954b2ecf832"}]}