)]}'
{"ceilometer/api/controllers/v2/meters.py":[{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"e78362d4b4b92457e790cdf2a3dceade091f1dfb","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        limit \u003d v2_utils.enforce_limit(limit)"},{"line_number":493,"context_line":"        kwargs \u003d v2_utils.query_to_kwargs("},{"line_number":494,"context_line":"            q, pecan.request.storage_conn.get_meters,"},{"line_number":495,"context_line":"            [\u0027limit\u0027], allow_timestamps\u003dFalse)"},{"line_number":496,"context_line":"        if \u0027limit\u0027 in kwargs:"},{"line_number":497,"context_line":"            raise base.ClientSideError(_("},{"line_number":498,"context_line":"                \"Limit is not a valid field for queries, \""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_afd92a24","line":495,"updated":"2015-09-16 13:14:43.000000000","message":"would this make the following condition invalid as it would already fail in query_to_kwargs\n\nhttps://github.com/openstack/ceilometer/blob/master/ceilometer/api/controllers/v2/utils.py#L184-L187","commit_id":"738d85c7b97fa5acb97741b9eafa7b9ba95381d7"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"0c1d3dc5a96bee92a9657a9deca83c2ee3f59c79","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        limit \u003d v2_utils.enforce_limit(limit)"},{"line_number":493,"context_line":"        kwargs \u003d v2_utils.query_to_kwargs("},{"line_number":494,"context_line":"            q, pecan.request.storage_conn.get_meters,"},{"line_number":495,"context_line":"            [\u0027limit\u0027], allow_timestamps\u003dFalse)"},{"line_number":496,"context_line":"        if \u0027limit\u0027 in kwargs:"},{"line_number":497,"context_line":"            raise base.ClientSideError(_("},{"line_number":498,"context_line":"                \"Limit is not a valid field for queries, \""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_a1e91db9","line":495,"in_reply_to":"ba15a1d1_afd92a24","updated":"2015-09-17 06:00:25.000000000","message":"we need to add limit to internal keys, otherwise, user will get wrong prompt, see https://bugs.launchpad.net/ceilometer/+bug/1496677\n\nI think we can remove the following condition, so when user use command ceilometer -d meter-list -q limit\u003d1, then he will get: BadRequest: Unknown argument: \"limit\": unrecognized field in query: [\u003cQuery u\u0027limit\u0027 eq u\u00271\u0027 \u003e], valid keys: [\u0027project\u0027, \u0027resource\u0027, \u0027source\u0027, \u0027user\u0027] (HTTP 400)\n\nIf you think this need further discussion, I can split this patch to 2 patches, I think the interface problem should be fixed anyway.","commit_id":"738d85c7b97fa5acb97741b9eafa7b9ba95381d7"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"a4867dc5e0faaa665ba545c9ad2a35bdeac9b3b9","unresolved":false,"context_lines":[{"line_number":494,"context_line":"            q, pecan.request.storage_conn.get_meters, allow_timestamps\u003dFalse)"},{"line_number":495,"context_line":"        if \u0027limit\u0027 in kwargs:"},{"line_number":496,"context_line":"            raise base.ClientSideError(_("},{"line_number":497,"context_line":"                \"Limit is not a valid field for queries, \""},{"line_number":498,"context_line":"                \"use \u0027limit\u0027 parameter.\"))"},{"line_number":499,"context_line":"        return [Meter.from_db_model(m)"},{"line_number":500,"context_line":"                for m in pecan.request.storage_conn.get_meters(limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_e03e218a","side":"PARENT","line":497,"updated":"2015-09-17 12:16:06.000000000","message":"to be honest, i think this error is more useful than the new one.","commit_id":"67b55c01c85f4d002271c42d0cd94fee6b503522"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"79be04ea5e1e376b491c454aff07ed2d443d3324","unresolved":false,"context_lines":[{"line_number":494,"context_line":"            q, pecan.request.storage_conn.get_meters, allow_timestamps\u003dFalse)"},{"line_number":495,"context_line":"        if \u0027limit\u0027 in kwargs:"},{"line_number":496,"context_line":"            raise base.ClientSideError(_("},{"line_number":497,"context_line":"                \"Limit is not a valid field for queries, \""},{"line_number":498,"context_line":"                \"use \u0027limit\u0027 parameter.\"))"},{"line_number":499,"context_line":"        return [Meter.from_db_model(m)"},{"line_number":500,"context_line":"                for m in pecan.request.storage_conn.get_meters(limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_768bb768","side":"PARENT","line":497,"in_reply_to":"ba15a1d1_53c24535","updated":"2015-09-17 12:44:35.000000000","message":"hmm... fair enough. at least the status code is the same i guess","commit_id":"67b55c01c85f4d002271c42d0cd94fee6b503522"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"df2431ee0ee3c16ad87e7a2749524189995f76d0","unresolved":false,"context_lines":[{"line_number":494,"context_line":"            q, pecan.request.storage_conn.get_meters, allow_timestamps\u003dFalse)"},{"line_number":495,"context_line":"        if \u0027limit\u0027 in kwargs:"},{"line_number":496,"context_line":"            raise base.ClientSideError(_("},{"line_number":497,"context_line":"                \"Limit is not a valid field for queries, \""},{"line_number":498,"context_line":"                \"use \u0027limit\u0027 parameter.\"))"},{"line_number":499,"context_line":"        return [Meter.from_db_model(m)"},{"line_number":500,"context_line":"                for m in pecan.request.storage_conn.get_meters(limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_53c24535","side":"PARENT","line":497,"in_reply_to":"ba15a1d1_e03e218a","updated":"2015-09-17 12:34:26.000000000","message":"Yes, comparing the faultstring, this original one is better, but if we don\u0027t set limit to internal keys, when user runs command \u0027ceilometer -d meter-list -q limitxxx\u003d1\u0027, he will get faultstring: Unknown argument: \"limitxxx\": unrecognized field in query: [\u003cQuery u\u0027limitxxx\u0027 eq u\u00271\u0027 \u003e], valid keys: [\u0027limit\u0027, \u0027project\u0027, \u0027resource\u0027, \u0027source\u0027, \u0027user\u0027], so he may think that limit is a valid query filter key. (Actually not)\n\nthen if he tries `ceilometer -d meter-list -q limit\u003d1, then he will get faultstring: \"Limit is not a valid field for queries, use \u0027limit\u0027 parameter.\"\n\nI think he will complain that the first faultstring is misleading or wrong.\n\nMy change doesn\u0027t have better faultstring for \u0027ceilometer -d meter-list -q limit\u003d1\u0027, but at least it is correct for both command. What do you think?\n\nI have put this example in the bug report as well.","commit_id":"67b55c01c85f4d002271c42d0cd94fee6b503522"}]}