)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8358,"name":"zhangguoqing","email":"zhang.guoqing@99cloud.net","username":"zhangguoqing"},"change_message_id":"bc87a84ee6ec6683c297486684febd4a0dc06da2","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Modify policy of get total/summary"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Update policy of get_total/summary, admin user can"},{"line_number":10,"context_line":"get price of all tenants and nonadmin user can"},{"line_number":11,"context_line":"get price of itsself."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"For example, get total api change as follows:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"5a3905b3_fada27e7","line":10,"range":{"start_line":10,"start_character":4,"end_line":10,"end_character":9},"updated":"2017-01-22 15:02:55.000000000","message":"All of the \u0027price\u0027 is not a good expression in the comment.","commit_id":"fd4139bb6715477e084664a998edb671e086c3ec"},{"author":{"_account_id":8358,"name":"zhangguoqing","email":"zhang.guoqing@99cloud.net","username":"zhangguoqing"},"change_message_id":"bc87a84ee6ec6683c297486684febd4a0dc06da2","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Modify policy of get total/summary"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Update policy of get_total/summary, admin user can"},{"line_number":10,"context_line":"get price of all tenants and nonadmin user can"},{"line_number":11,"context_line":"get price of itsself."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"For example, get total api change as follows:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"5a3905b3_1ae453aa","line":10,"range":{"start_line":10,"start_character":29,"end_line":10,"end_character":37},"updated":"2017-01-22 15:02:55.000000000","message":"non-admin","commit_id":"fd4139bb6715477e084664a998edb671e086c3ec"},{"author":{"_account_id":8358,"name":"zhangguoqing","email":"zhang.guoqing@99cloud.net","username":"zhangguoqing"},"change_message_id":"bc87a84ee6ec6683c297486684febd4a0dc06da2","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Update policy of get_total/summary, admin user can"},{"line_number":10,"context_line":"get price of all tenants and nonadmin user can"},{"line_number":11,"context_line":"get price of itsself."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"For example, get total api change as follows:"},{"line_number":14,"context_line":"    cloudkitty total-get"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"5a3905b3_5af09b67","line":11,"range":{"start_line":11,"start_character":13,"end_line":11,"end_character":20},"updated":"2017-01-22 15:02:55.000000000","message":"itself","commit_id":"fd4139bb6715477e084664a998edb671e086c3ec"}],"cloudkitty/api/v1/controllers/report.py":[{"author":{"_account_id":23173,"name":"Maxime Cottret","email":"maxime.cottret@gmail.com","username":"aolwas"},"change_message_id":"1c3c395b9ff5f64794b9c9ad0f3cc0ba425ff70f","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        if tenant_id:"},{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_0c180884","line":64,"range":{"start_line":60,"start_character":8,"end_line":64,"end_character":52},"updated":"2017-01-02 09:29:27.000000000","message":"I suggest the following code:\n\n\"\"\"\nif not tenant_id: \n  tenant_id \u003d pecan.request.context.tenant\n\npolicy.enforce(pecan.request.context: \u0027report:get_total\u0027\n               {\"tenant_id\": tenant_id})\n\"\"\"\n\nThis way, the tenant parameter can be omitted and you ensure the user have right on the tenant from the context","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":21797,"name":"Jeremy Liu","display_name":"Jeremy Liu","email":"liujiong.cc@gmail.com","username":"liujiong"},"change_message_id":"51fcd90e053374b22027765137132005388e4124","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_0b1727a4","line":64,"range":{"start_line":64,"start_character":12,"end_line":64,"end_character":52},"updated":"2016-12-30 03:11:45.000000000","message":"Why not just return \"403 Forbidden\" to users instead of retrieving the total price for the request tenant?","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":15917,"name":"Aaron-DH","email":"344677472@qq.com","username":"aaron"},"change_message_id":"2f86f2fd75639cf5f2064a1c8eb74a9dd5309bd1","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_69f8b8e6","line":64,"range":{"start_line":64,"start_character":12,"end_line":64,"end_character":52},"in_reply_to":"ba5201f7_0b1727a4","updated":"2016-12-30 09:51:48.000000000","message":"hi, Jeremy, thanks for your review.\nIf return \"403 Forbidden\" here, all nonadmin can only get their own total price when put their tenant_id in request parameter, used like \u0027cloudkitty total-get -t tenant_id\u0027.\nAs i think for nonadmin user, they can get total price without input their own tenant_id(just get from context)","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":21797,"name":"Jeremy Liu","display_name":"Jeremy Liu","email":"liujiong.cc@gmail.com","username":"liujiong"},"change_message_id":"aebbbc7f96359946c4bc88580e9f405bc9f8f94d","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        if tenant_id:"},{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_985eee43","line":64,"range":{"start_line":60,"start_character":8,"end_line":64,"end_character":52},"in_reply_to":"ba5201f7_0c180884","updated":"2017-01-02 13:40:50.000000000","message":"Exactly this way.","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":15917,"name":"Aaron-DH","email":"344677472@qq.com","username":"aaron"},"change_message_id":"4a5fc2e877d68be92702ea6c9a3962df2a35cbd6","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        if tenant_id:"},{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_86682189","line":64,"range":{"start_line":60,"start_character":8,"end_line":64,"end_character":52},"in_reply_to":"ba5201f7_734a4485","updated":"2017-01-04 02:09:45.000000000","message":"yeah, admin has been allowed to get price of other tenants.\nbut what i think is that, when admin tenant  `cloudkitty total-get` without tenant_id(-t), this will return the price of admin tenant. as i think it should return total price of all tenants while nonadmin tenants should return the price of itsself(get tenant from context).","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":21797,"name":"Jeremy Liu","display_name":"Jeremy Liu","email":"liujiong.cc@gmail.com","username":"liujiong"},"change_message_id":"6c9ea85352644d7f00e1d040be0167219c899692","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        if tenant_id:"},{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_67d17857","line":64,"range":{"start_line":60,"start_character":8,"end_line":64,"end_character":52},"in_reply_to":"ba5201f7_86682189","updated":"2017-01-04 03:13:35.000000000","message":"Well, that needs some discussions.\nIMO, if admin wants to get price of all tenants, a flag must be passed to CK(like `all-tenants`, you said) instead of retrieving all price based on the role in request.context.\nAnyway, you better make the commit message clear enough to express what you want to do in the patch.","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":15917,"name":"Aaron-DH","email":"344677472@qq.com","username":"aaron"},"change_message_id":"be6f57e13d72dadb572ed5322c9089cbb42d0c72","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        if tenant_id:"},{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_f3e71410","line":64,"range":{"start_line":60,"start_character":8,"end_line":64,"end_character":52},"in_reply_to":"ba5201f7_985eee43","updated":"2017-01-03 01:41:18.000000000","message":"if so, admin can just get price of admin tenant?\nshould it be:\nif not tenant_id and not pecan.request.context.is_admin: \n  tenant_id \u003d pecan.request.context.tenant\nor add all-tenant parameter?","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":21797,"name":"Jeremy Liu","display_name":"Jeremy Liu","email":"liujiong.cc@gmail.com","username":"liujiong"},"change_message_id":"7bd726e0ea5a4039c26f93cccaba5521115d8912","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        if tenant_id:"},{"line_number":61,"context_line":"            policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":62,"context_line":"                           {\"tenant_id\": tenant_id})"},{"line_number":63,"context_line":"        elif not pecan.request.context.is_admin:"},{"line_number":64,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":67,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_734a4485","line":64,"range":{"start_line":60,"start_character":8,"end_line":64,"end_character":52},"in_reply_to":"ba5201f7_f3e71410","updated":"2017-01-03 02:30:49.000000000","message":"You already change the policy \"report:get_total\": \"rule:admin_or_owner\", this way, admin will be allowed to get price of other tenants.","commit_id":"dda681e5530c06e934d3c7d04c8c60184d895a46"},{"author":{"_account_id":23173,"name":"Maxime Cottret","email":"maxime.cottret@gmail.com","username":"aolwas"},"change_message_id":"38b6a60d3d1698459d6ee8111a83e7f4f1f2e524","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                         datetime.datetime,"},{"line_number":55,"context_line":"                         wtypes.text,"},{"line_number":56,"context_line":"                         wtypes.text,"},{"line_number":57,"context_line":"                         wtypes.text)"},{"line_number":58,"context_line":"    def total(self, begin\u003dNone, end\u003dNone, tenant_id\u003dNone, service\u003dNone,"},{"line_number":59,"context_line":"              all_tenants\u003dNone):"},{"line_number":60,"context_line":"        \"\"\"Return the amount to pay for a given period."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_bf6ef52c","line":57,"range":{"start_line":57,"start_character":25,"end_line":57,"end_character":36},"updated":"2017-01-04 13:23:48.000000000","message":"Why not using directly wtype.bool for all_tenants parameter ?","commit_id":"797a6443b62e83c672db16d6a26fb3d718c86e3b"},{"author":{"_account_id":21797,"name":"Jeremy Liu","display_name":"Jeremy Liu","email":"liujiong.cc@gmail.com","username":"liujiong"},"change_message_id":"2e55ec01c6edbc7a6ebff80deeed74de66c24787","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        \"\"\""},{"line_number":62,"context_line":"        if not all_tenants and not tenant_id:"},{"line_number":63,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":64,"context_line":"        policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":65,"context_line":"                       {\"tenant_id\": tenant_id})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":68,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_75065087","line":65,"range":{"start_line":62,"start_character":0,"end_line":65,"end_character":48},"updated":"2017-01-05 03:45:24.000000000","message":"If a non-admin specify `all_tenants\u003dTrue`, the request will be denied by policy and 403 forbidden returned. Shouldn\u0027t we get the tenant_id from context and return the price of that one?\n\nI guess your code maybe something like this:\n\"\"\"\n\n  if all_tenants:\n      if policy.check_is_admin(pecan.request.context.roles):\n          tenant_id \u003d None\n      else:\n          tenant_id \u003d pecan.request.context.tenant\n  else:\n      tenant_id \u003d pecan.request.context.tenant if not tenant_id else tenant_id\n  policy.enforce(pecan.request.context, \u0027report:get_total\u0027, {\u0027tenant_id\u0027: tenant_id})\n  storage \u003d pecan.request.storage_backend\n  ...\n\n\"\"\"\nJust my opinion, not sure which is the proper way to go.","commit_id":"9e9cba97e58c7041ac289351a94cc5d62f90a875"},{"author":{"_account_id":15917,"name":"Aaron-DH","email":"344677472@qq.com","username":"aaron"},"change_message_id":"b6b9c6183e734bb20f0d6703e109f8f772c6a52e","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        \"\"\"Return the amount to pay for a given period."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        \"\"\""},{"line_number":62,"context_line":"        if not all_tenants and not tenant_id:"},{"line_number":63,"context_line":"            tenant_id \u003d pecan.request.context.tenant"},{"line_number":64,"context_line":"        policy.enforce(pecan.request.context, \u0027report:get_total\u0027,"},{"line_number":65,"context_line":"                       {\"tenant_id\": tenant_id})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        storage \u003d pecan.request.storage_backend"},{"line_number":68,"context_line":"        # FIXME(sheeprine): We should filter on user id."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_e4d18958","line":65,"range":{"start_line":62,"start_character":0,"end_line":65,"end_character":48},"in_reply_to":"ba5201f7_75065087","updated":"2017-01-06 02:35:32.000000000","message":"Hi, Jeremy.\nfirst, if non-admin only specify `all_tenants\u003dTrue`, i thinks it should reutrn \u0027403 Fribidden\u0027.\nsecond, when specfy both `all_tenants\u003dTrue` and `tenant_id\u003dXXX`, \nthis is confusing~ i think it should ignore tenant_id..\nmaybe like this:\n\"\"\"\n  if all_tenants:\n      tenant_id \u003d None\n  else:\n      tenant_id \u003d pecan.request.context.tenant if not tenant_id else tenant_id\n  policy.enforce(pecan.request.context, \u0027report:get_total\u0027, {\u0027tenant_id\u0027: tenant_id})\n  storage \u003d pecan.request.storage_backend\n  ...\n\"\"\"","commit_id":"9e9cba97e58c7041ac289351a94cc5d62f90a875"}],"cloudkitty/common/policy.py":[{"author":{"_account_id":8358,"name":"zhangguoqing","email":"zhang.guoqing@99cloud.net","username":"zhangguoqing"},"change_message_id":"bc87a84ee6ec6683c297486684febd4a0dc06da2","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    \"\"\""},{"line_number":78,"context_line":"    if CONF.auth_strategy !\u003d \"keystone\":"},{"line_number":79,"context_line":"        return True"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    init()"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5a3905b3_254ad055","line":79,"range":{"start_line":79,"start_character":15,"end_line":79,"end_character":19},"updated":"2017-01-22 15:02:55.000000000","message":"Why here return value is \u0027True\u0027? It is not same as the Line of 64. Could you give me some explainations.","commit_id":"fd4139bb6715477e084664a998edb671e086c3ec"},{"author":{"_account_id":21797,"name":"Jeremy Liu","display_name":"Jeremy Liu","email":"liujiong.cc@gmail.com","username":"liujiong"},"change_message_id":"b36edd6e7c222c284ebcf3062b933a280ced1ea0","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    \"\"\""},{"line_number":78,"context_line":"    if CONF.auth_strategy !\u003d \"keystone\":"},{"line_number":79,"context_line":"        return True"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    init()"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5a3905b3_644a1028","line":79,"range":{"start_line":79,"start_character":15,"end_line":79,"end_character":19},"in_reply_to":"5a3905b3_254ad055","updated":"2017-01-23 06:45:05.000000000","message":"We use `policy.enforce` to check if users have access to the API [1]. And use `policy.check_is_admin` to check if users are administrator [2]. The usage differs.\nIf `auth_strategy` is not keystone, we may not want to check the access of the user, so we just `return` or return True to mark the users as administrator.\n\n[1] https://github.com/openstack/cloudkitty/blob/master/cloudkitty/api/v1/controllers/rating.py#L76\n[2] https://github.com/openstack/cloudkitty/blob/master/cloudkitty/api/hooks.py#L46","commit_id":"fd4139bb6715477e084664a998edb671e086c3ec"}]}