)]}'
{"ceilometer/api/controllers/v2.py":[{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"6ca845b0dd2826df32f3b216e03cba04490ba310","unresolved":false,"context_lines":[{"line_number":284,"context_line":"    \u0027\u0027\u0027"},{"line_number":285,"context_line":"    auth_project \u003d acl.get_limited_to_project(headers or"},{"line_number":286,"context_line":"                                              pecan.request.headers)"},{"line_number":287,"context_line":"    created_by \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":288,"context_line":"    eventual_auth_project \u003d (auth_project or"},{"line_number":289,"context_line":"                             (on_behalf_of if on_behalf_of !\u003d created_by"},{"line_number":290,"context_line":"                              else None))"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2Fvx4%3D","line":287,"updated":"2013-10-10 12:14:35.000000000","message":"You are getting passed the headers argument, use it? :)","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"ece8eb413ece530f090f292d800059d39a8ffc2c","unresolved":false,"context_lines":[{"line_number":284,"context_line":"    \u0027\u0027\u0027"},{"line_number":285,"context_line":"    auth_project \u003d acl.get_limited_to_project(headers or"},{"line_number":286,"context_line":"                                              pecan.request.headers)"},{"line_number":287,"context_line":"    created_by \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":288,"context_line":"    eventual_auth_project \u003d (auth_project or"},{"line_number":289,"context_line":"                             (on_behalf_of if on_behalf_of !\u003d created_by"},{"line_number":290,"context_line":"                              else None))"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FvDI%3D","line":287,"in_reply_to":"AAAATH%2F%2Fvx4%3D","updated":"2013-10-10 15:01:00.000000000","message":"Note that the purpose the headers param was to enable an override of these headers in tests.api.v2.test_query ... I\u0027ve replaced that now with proper mock usage, so that this kwarg is now unnecessary and we can just use pecan.request.headers across the board.","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"6ca845b0dd2826df32f3b216e03cba04490ba310","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                                              pecan.request.headers)"},{"line_number":287,"context_line":"    created_by \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":288,"context_line":"    eventual_auth_project \u003d (auth_project or"},{"line_number":289,"context_line":"                             (on_behalf_of if on_behalf_of !\u003d created_by"},{"line_number":290,"context_line":"                              else None))"},{"line_number":291,"context_line":"    if eventual_auth_project:"},{"line_number":292,"context_line":"        proj_q \u003d [i for i in q if i.field \u003d\u003d \u0027project_id\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FvxE%3D","line":289,"updated":"2013-10-10 12:14:35.000000000","message":"You can move this or to line L285 and keep the value as auth_project; with a comment about why you do this complicated computing","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"ece8eb413ece530f090f292d800059d39a8ffc2c","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                                              pecan.request.headers)"},{"line_number":287,"context_line":"    created_by \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":288,"context_line":"    eventual_auth_project \u003d (auth_project or"},{"line_number":289,"context_line":"                             (on_behalf_of if on_behalf_of !\u003d created_by"},{"line_number":290,"context_line":"                              else None))"},{"line_number":291,"context_line":"    if eventual_auth_project:"},{"line_number":292,"context_line":"        proj_q \u003d [i for i in q if i.field \u003d\u003d \u0027project_id\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2Fu%2Fw%3D","line":289,"in_reply_to":"AAAATH%2F%2FvxE%3D","updated":"2013-10-10 15:01:00.000000000","message":"See if you like the latest version any better.","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"6ca845b0dd2826df32f3b216e03cba04490ba310","unresolved":false,"context_lines":[{"line_number":306,"context_line":"def _verify_query_segregation(q):"},{"line_number":307,"context_line":"    \u0027\u0027\u0027Ensure non-admin queries are not constrained to another project.\u0027\u0027\u0027"},{"line_number":308,"context_line":"    auth_project \u003d acl.get_limited_to_project(pecan.request.headers)"},{"line_number":309,"context_line":"    if auth_project:"},{"line_number":310,"context_line":"        proj_q \u003d [i for i in q if i.field \u003d\u003d \u0027project_id\u0027]"},{"line_number":311,"context_line":"        for i in proj_q:"},{"line_number":312,"context_line":"            if auth_project !\u003d i.value or i.op !\u003d \u0027eq\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FvuY%3D","line":309,"updated":"2013-10-10 12:14:35.000000000","message":"This kind of check is already done in _sanitize_query(), I think there\u0027s something to factorize out there.","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"ece8eb413ece530f090f292d800059d39a8ffc2c","unresolved":false,"context_lines":[{"line_number":306,"context_line":"def _verify_query_segregation(q):"},{"line_number":307,"context_line":"    \u0027\u0027\u0027Ensure non-admin queries are not constrained to another project.\u0027\u0027\u0027"},{"line_number":308,"context_line":"    auth_project \u003d acl.get_limited_to_project(pecan.request.headers)"},{"line_number":309,"context_line":"    if auth_project:"},{"line_number":310,"context_line":"        proj_q \u003d [i for i in q if i.field \u003d\u003d \u0027project_id\u0027]"},{"line_number":311,"context_line":"        for i in proj_q:"},{"line_number":312,"context_line":"            if auth_project !\u003d i.value or i.op !\u003d \u0027eq\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2Fu%2FM%3D","line":309,"in_reply_to":"AAAATH%2F%2FvuY%3D","updated":"2013-10-10 15:01:00.000000000","message":"OK it\u0027s factored out now, but note that I still have to re-run the check, for reasons explained in another comment below (\"However I thought of a slight edge case that required _sanitize_query() to recheck ...\")","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"15ccd4a12537bd299ffab5a362ccb9d90864949f","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        proj_q \u003d [i for i in q if i.field \u003d\u003d \u0027project_id\u0027]"},{"line_number":311,"context_line":"        for i in proj_q:"},{"line_number":312,"context_line":"            if auth_project !\u003d i.value or i.op !\u003d \u0027eq\u0027:"},{"line_number":313,"context_line":"                raise ProjectNotAuthorized(i.value)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"def _validate_query(query, db_func, internal_keys\u003d[]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FxQk%3D","line":313,"updated":"2013-10-10 03:50:06.000000000","message":"how about using 1 iteration instead of 2 here, change Ln310-Ln313 to the following:\nfor i in q:\n    if i.field \u003d\u003d \u0027project_id\u0027 and auth_project !\u003d i.value or i.op !\u003d \u0027eq\u0027:\n        raise ProjectNotAuthorized(i.value)","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"60c55e342c3f76df3a956fed64d64733942664cd","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        proj_q \u003d [i for i in q if i.field \u003d\u003d \u0027project_id\u0027]"},{"line_number":311,"context_line":"        for i in proj_q:"},{"line_number":312,"context_line":"            if auth_project !\u003d i.value or i.op !\u003d \u0027eq\u0027:"},{"line_number":313,"context_line":"                raise ProjectNotAuthorized(i.value)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"def _validate_query(query, db_func, internal_keys\u003d[]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2Fwy4%3D","line":313,"in_reply_to":"AAAATH%2F%2FxQk%3D","updated":"2013-10-10 07:09:48.000000000","message":"Sure, that\u0027s neater.","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"15ccd4a12537bd299ffab5a362ccb9d90864949f","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    _validate_query(query, db_func, internal_keys\u003dinternal_keys)"},{"line_number":363,"context_line":"    valid_keys \u003d inspect.getargspec(db_func)[0]"},{"line_number":364,"context_line":"    query \u003d _sanitize_query(query, valid_keys, headers\u003dheaders,"},{"line_number":365,"context_line":"                            on_behalf_of\u003don_behalf_of)"},{"line_number":366,"context_line":"    internal_keys.append(\u0027self\u0027)"},{"line_number":367,"context_line":"    valid_keys \u003d set(valid_keys) - set(internal_keys)"},{"line_number":368,"context_line":"    translation \u003d {\u0027user_id\u0027: \u0027user\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FxN8%3D","line":365,"updated":"2013-10-10 03:50:06.000000000","message":"The _verify_query_segregation()\u0027s job done by calling _validate_query()-\u003e _verify_query_segregation() is either useless or duplicated here, with the jobs done by _sanitize_query() here.\n\nHow about unify _verify_query_segregation() and _sanitize_query() into one single function?","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"60c55e342c3f76df3a956fed64d64733942664cd","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    _validate_query(query, db_func, internal_keys\u003dinternal_keys)"},{"line_number":363,"context_line":"    valid_keys \u003d inspect.getargspec(db_func)[0]"},{"line_number":364,"context_line":"    query \u003d _sanitize_query(query, valid_keys, headers\u003dheaders,"},{"line_number":365,"context_line":"                            on_behalf_of\u003don_behalf_of)"},{"line_number":366,"context_line":"    internal_keys.append(\u0027self\u0027)"},{"line_number":367,"context_line":"    valid_keys \u003d set(valid_keys) - set(internal_keys)"},{"line_number":368,"context_line":"    translation \u003d {\u0027user_id\u0027: \u0027user\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FwzU%3D","line":365,"in_reply_to":"AAAATH%2F%2FxN8%3D","updated":"2013-10-10 07:09:48.000000000","message":"The original idea was that _verify_query_segregation() would *only* doing the checking of the project_id constraint and that _sanitize_query() would *only* do the addition of a constraint if required.\n\nSo that _verify_query_segregation() would be non-modifying and _sanitize_query() would be non-checking.\n\nHowever I thought of a slight edge case that required _sanitize_query() to recheck: an admin creating an alarm on behalf of non-admin tenant T1 but with an explicit query constraining on the project_id of another tenant T2\n\nThis recheck must occur in _sanitize_query(), as the full context of the alarm (including the owning project_id) is only available at that point.\n\nThat case should fail also for consistency. Though I\u0027d be open to it just failing silently (i.e. the alarm staying forever in \u0027insufficient data\u0027) if you really object to the checking done in _sanitize_query().","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"fc52fbc4b591ef92ee6923cdb0167bf74f66ace4","unresolved":false,"context_lines":[{"line_number":1174,"context_line":"                error \u003d _(\"%s is mandatory\") % field"},{"line_number":1175,"context_line":"                pecan.response.translatable_error \u003d error"},{"line_number":1176,"context_line":"                raise wsme.exc.ClientSideError(unicode(error))"},{"line_number":1177,"context_line":"        if (alarm.threshold_rule):"},{"line_number":1178,"context_line":"            #note(sileht): _query_to_kwargs implicitly call _sanitize_query"},{"line_number":1179,"context_line":"            #that add project_id in query"},{"line_number":1180,"context_line":"            timestamp_keys \u003d [\u0027timestamp\u0027, \u0027start\u0027, \u0027start_timestamp\u0027 \u0027end\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FyJQ%3D","line":1177,"updated":"2013-10-10 06:31:47.000000000","message":"The parentheses seems not useful, here","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"60c55e342c3f76df3a956fed64d64733942664cd","unresolved":false,"context_lines":[{"line_number":1174,"context_line":"                error \u003d _(\"%s is mandatory\") % field"},{"line_number":1175,"context_line":"                pecan.response.translatable_error \u003d error"},{"line_number":1176,"context_line":"                raise wsme.exc.ClientSideError(unicode(error))"},{"line_number":1177,"context_line":"        if (alarm.threshold_rule):"},{"line_number":1178,"context_line":"            #note(sileht): _query_to_kwargs implicitly call _sanitize_query"},{"line_number":1179,"context_line":"            #that add project_id in query"},{"line_number":1180,"context_line":"            timestamp_keys \u003d [\u0027timestamp\u0027, \u0027start\u0027, \u0027start_timestamp\u0027 \u0027end\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2Fwys%3D","line":1177,"in_reply_to":"AAAATH%2F%2FyJQ%3D","updated":"2013-10-10 07:09:48.000000000","message":"Yep, good spot, fixing now.","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"fc52fbc4b591ef92ee6923cdb0167bf74f66ace4","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            _query_to_kwargs(alarm.threshold_rule.query,"},{"line_number":1183,"context_line":"                             storage.SampleFilter.__init__,"},{"line_number":1184,"context_line":"                             internal_keys\u003dtimestamp_keys,"},{"line_number":1185,"context_line":"                             on_behalf_of\u003dalarm.project_id)"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"        if alarm.threshold_rule and alarm.combination_rule:"},{"line_number":1188,"context_line":"            error \u003d _(\"threshold_rule and combination_rule \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2Fw4k%3D","line":1185,"updated":"2013-10-10 06:31:47.000000000","message":"This validate the threshold_rule a seconds time, wsme should already have run the AlarmThresholdRule.validate on this rule.\nWe only needs to call _sanitize_query here, no ?","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"60c55e342c3f76df3a956fed64d64733942664cd","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            _query_to_kwargs(alarm.threshold_rule.query,"},{"line_number":1183,"context_line":"                             storage.SampleFilter.__init__,"},{"line_number":1184,"context_line":"                             internal_keys\u003dtimestamp_keys,"},{"line_number":1185,"context_line":"                             on_behalf_of\u003dalarm.project_id)"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"        if alarm.threshold_rule and alarm.combination_rule:"},{"line_number":1188,"context_line":"            error \u003d _(\"threshold_rule and combination_rule \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATH%2F%2FwxU%3D","line":1185,"in_reply_to":"AAAATH%2F%2Fw4k%3D","updated":"2013-10-10 07:09:48.000000000","message":"Yeah, I guess I just wanted to avoid the complexity of the call to:\n\n  inspect.getargspec(db_func)[0]\n\nhere. But I could re-work that to call instead within _sanitize_query().","commit_id":"57aeed4a638cc79a9e3c97284f42253475b07f0c"}],"tests/api/v2/test_alarm_scenarios.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"83e3c564af99aab86c4c0e1d359af601ef8a847d","unresolved":false,"context_lines":[{"line_number":500,"context_line":"                                         json[key])"},{"line_number":501,"context_line":"                    else:"},{"line_number":502,"context_line":"                        query \u003d getattr(alarms[0], storage_key).get(\u0027query\u0027)"},{"line_number":503,"context_line":"                        self.assertEquals(len(query), 2)"},{"line_number":504,"context_line":"                        implicit_constraint \u003d {"},{"line_number":505,"context_line":"                            u\u0027field\u0027: u\u0027project_id\u0027,"},{"line_number":506,"context_line":"                            u\u0027value\u0027: u\u0027aprojectidthatisnotmine\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2Fu2A%3D","line":503,"updated":"2013-10-10 15:11:59.000000000","message":"assertEquals -\u003e assertEqual","commit_id":"5fb6858e34689171c6f34f9fc2e1175b36b4b8b2"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"f082d938f0ab985c4062454250b4eba3a6727654","unresolved":false,"context_lines":[{"line_number":500,"context_line":"                                         json[key])"},{"line_number":501,"context_line":"                    else:"},{"line_number":502,"context_line":"                        query \u003d getattr(alarms[0], storage_key).get(\u0027query\u0027)"},{"line_number":503,"context_line":"                        self.assertEquals(len(query), 2)"},{"line_number":504,"context_line":"                        implicit_constraint \u003d {"},{"line_number":505,"context_line":"                            u\u0027field\u0027: u\u0027project_id\u0027,"},{"line_number":506,"context_line":"                            u\u0027value\u0027: u\u0027aprojectidthatisnotmine\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2Fuyc%3D","line":503,"in_reply_to":"AAAATH%2F%2Fu2A%3D","updated":"2013-10-10 15:17:24.000000000","message":"Good spot, done.","commit_id":"5fb6858e34689171c6f34f9fc2e1175b36b4b8b2"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"83e3c564af99aab86c4c0e1d359af601ef8a847d","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                            u\u0027value\u0027: u\u0027aprojectidthatisnotmine\u0027,"},{"line_number":507,"context_line":"                            u\u0027op\u0027: u\u0027eq\u0027"},{"line_number":508,"context_line":"                        }"},{"line_number":509,"context_line":"                        self.assertEquals(query[1], implicit_constraint)"},{"line_number":510,"context_line":"                else:"},{"line_number":511,"context_line":"                    self.assertEqual(getattr(alarms[0], key), json[key])"},{"line_number":512,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2Fu18%3D","line":509,"updated":"2013-10-10 15:11:59.000000000","message":"assertEquals -\u003e assertEqual","commit_id":"5fb6858e34689171c6f34f9fc2e1175b36b4b8b2"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"f082d938f0ab985c4062454250b4eba3a6727654","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                            u\u0027value\u0027: u\u0027aprojectidthatisnotmine\u0027,"},{"line_number":507,"context_line":"                            u\u0027op\u0027: u\u0027eq\u0027"},{"line_number":508,"context_line":"                        }"},{"line_number":509,"context_line":"                        self.assertEquals(query[1], implicit_constraint)"},{"line_number":510,"context_line":"                else:"},{"line_number":511,"context_line":"                    self.assertEqual(getattr(alarms[0], key), json[key])"},{"line_number":512,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2FuyY%3D","line":509,"in_reply_to":"AAAATH%2F%2Fu18%3D","updated":"2013-10-10 15:17:24.000000000","message":"Similarly.","commit_id":"5fb6858e34689171c6f34f9fc2e1175b36b4b8b2"}]}