)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c36c7f1c5269bf622dac49de9d8d656dbc6d59a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"and delete a group type."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Also fixed an issue in db API so that non-admin can only"},{"line_number":16,"context_line":"see public group types."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I4d4895bc2c589076add06dcf874f755e29b1548a"},{"line_number":19,"context_line":"Closes-Bug: #1616204"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1ac06dbe_29aeae2d","line":16,"updated":"2016-08-24 11:19:58.000000000","message":"-1: This should be another patch and have its own bug","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"771d10fba5253fbe3099905fbd9b978fd4aea452","unresolved":false,"context_lines":[{"line_number":13,"context_line":"and delete a group type."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Also fixed an issue in db API so that non-admin can only"},{"line_number":16,"context_line":"see public group types."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I4d4895bc2c589076add06dcf874f755e29b1548a"},{"line_number":19,"context_line":"Closes-Bug: #1616204"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa7ab95a_ffd25fff","line":16,"in_reply_to":"1ac06dbe_29aeae2d","updated":"2016-08-28 03:16:06.000000000","message":"Sure.  Removed from this patch.","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f252705e4c66f2b1f8975421de6897dbc8fc6603","unresolved":false,"context_lines":[{"line_number":13,"context_line":"and delete a group type."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Also fixed an issue in db API so that non-admin can only"},{"line_number":16,"context_line":"see public group types."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I4d4895bc2c589076add06dcf874f755e29b1548a"},{"line_number":19,"context_line":"Closes-Bug: #1616204"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa7ab95a_d22489c6","line":16,"in_reply_to":"fa7ab95a_ffd25fff","updated":"2016-08-29 13:09:17.000000000","message":"Thanks","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"}],"cinder/api/v3/group_specs.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c36c7f1c5269bf622dac49de9d8d656dbc6d59a","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from cinder.api import common"},{"line_number":20,"context_line":"from cinder.api.openstack import wsgi"},{"line_number":21,"context_line":"from cinder.api.v3 import group_types as group_types_api"},{"line_number":22,"context_line":"from cinder import db"},{"line_number":23,"context_line":"from cinder import exception"},{"line_number":24,"context_line":"from cinder.i18n import _"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_694386af","line":21,"updated":"2016-08-24 11:19:58.000000000","message":"-1: We shouldn\u0027t be importing methods from other api controllers, any common method should go in cinder.api.common","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"771d10fba5253fbe3099905fbd9b978fd4aea452","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from cinder.api import common"},{"line_number":20,"context_line":"from cinder.api.openstack import wsgi"},{"line_number":21,"context_line":"from cinder.api.v3 import group_types as group_types_api"},{"line_number":22,"context_line":"from cinder import db"},{"line_number":23,"context_line":"from cinder import exception"},{"line_number":24,"context_line":"from cinder.i18n import _"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_7f61ef38","line":21,"in_reply_to":"1ac06dbe_694386af","updated":"2016-08-28 03:16:06.000000000","message":"Done","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"061e42842f495f68af8fe69f7345235919e2cd34","unresolved":false,"context_lines":[{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        self._check_type(context, group_type_id)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        try:"},{"line_number":144,"context_line":"            db.group_type_specs_delete(context, group_type_id, id)"},{"line_number":145,"context_line":"        except exception.GroupTypeSpecsNotFound as error:"},{"line_number":146,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003derror.msg)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        notifier_info \u003d dict(type_id\u003dgroup_type_id, id\u003did)"},{"line_number":149,"context_line":"        notifier \u003d rpc.get_notifier(\u0027groupTypeSpecs\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_328a151c","line":146,"range":{"start_line":143,"start_character":0,"end_line":146,"end_character":63},"updated":"2016-08-29 13:11:53.000000000","message":"nit: We shouldn\u0027t be catching this exception here if we are just going to change it to a webob exception, since that\u0027s already being handled at the middleware level, we can replace all this with just:\n\n db.group_type_specs_delete(context, group_type_id, id)","commit_id":"b08435ecb02d1cc0697b87ec4dd0f402b288983b"}],"cinder/api/v3/group_types.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c36c7f1c5269bf622dac49de9d8d656dbc6d59a","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from cinder.volume import group_types"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def check_policy(context, action, target_obj\u003dNone):"},{"line_number":34,"context_line":"    target \u003d {"},{"line_number":35,"context_line":"        \u0027project_id\u0027: context.project_id,"},{"line_number":36,"context_line":"        \u0027user_id\u0027: context.user_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_29f6ae8e","line":33,"updated":"2016-08-24 11:19:58.000000000","message":"-1: I think this is not the right approach I would go with\n\nCreating a policy checker class attribute in GroupTypesController\n\n policy_checker \u003d wsgi.Controller.get_policy_checker(\u0027group\u0027)\n\nAnd then we can get the context and check the policy all in one with:\n\n context \u003d self.policy_checker(req, \u0027group_types_manage\u0027)\n\nA worse alternative would be to update cinder.api.common:validate_policy to accept target_obj and just use it passing group:group_types_manage.","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"771d10fba5253fbe3099905fbd9b978fd4aea452","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from cinder.volume import group_types"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def check_policy(context, action, target_obj\u003dNone):"},{"line_number":34,"context_line":"    target \u003d {"},{"line_number":35,"context_line":"        \u0027project_id\u0027: context.project_id,"},{"line_number":36,"context_line":"        \u0027user_id\u0027: context.user_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_5fefcbaf","line":33,"in_reply_to":"1ac06dbe_29f6ae8e","updated":"2016-08-28 03:16:06.000000000","message":"Sorry but I don\u0027t understand why this is not the right approach.  I tested this and it worked.  I\u0027m following the same approach in volume/api.py and it is used in other places too.\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L104\n\nAlso please note this is not in show and index because we do allow non-admin to see the group_type just not group_specs.","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f252705e4c66f2b1f8975421de6897dbc8fc6603","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from cinder.volume import group_types"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def check_policy(context, action, target_obj\u003dNone):"},{"line_number":34,"context_line":"    target \u003d {"},{"line_number":35,"context_line":"        \u0027project_id\u0027: context.project_id,"},{"line_number":36,"context_line":"        \u0027user_id\u0027: context.user_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_128fd1cc","line":33,"in_reply_to":"fa7ab95a_5fefcbaf","updated":"2016-08-29 13:09:17.000000000","message":"Sorry for not being more specific, I mean it\u0027s not the right approach because we are duplicating code.  But I\u0027ll follow up with a patch to fix this in our API in general.","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c36c7f1c5269bf622dac49de9d8d656dbc6d59a","unresolved":false,"context_lines":[{"line_number":2929,"context_line":""},{"line_number":2930,"context_line":"    if not context.is_admin:"},{"line_number":2931,"context_line":"        the_filter \u003d [models.GroupTypes.is_public \u003d\u003d true()]"},{"line_number":2932,"context_line":"        projects_attr \u003d getattr(models.GroupTypes, \u0027projects\u0027)"},{"line_number":2933,"context_line":"        the_filter.extend(["},{"line_number":2934,"context_line":"            projects_attr.any(project_id\u003dcontext.project_id)"},{"line_number":2935,"context_line":"        ])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_698d46d2","line":2932,"updated":"2016-08-24 11:19:58.000000000","message":"?: Why are we using getattr instead of\n\n projects_attr \u003d models.GroupTypes.projects","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"771d10fba5253fbe3099905fbd9b978fd4aea452","unresolved":false,"context_lines":[{"line_number":2929,"context_line":""},{"line_number":2930,"context_line":"    if not context.is_admin:"},{"line_number":2931,"context_line":"        the_filter \u003d [models.GroupTypes.is_public \u003d\u003d true()]"},{"line_number":2932,"context_line":"        projects_attr \u003d getattr(models.GroupTypes, \u0027projects\u0027)"},{"line_number":2933,"context_line":"        the_filter.extend(["},{"line_number":2934,"context_line":"            projects_attr.any(project_id\u003dcontext.project_id)"},{"line_number":2935,"context_line":"        ])"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_ff409fa7","line":2932,"in_reply_to":"1ac06dbe_698d46d2","updated":"2016-08-28 03:16:06.000000000","message":"Moved in another patch.","commit_id":"7c2b4c46d4e7776faf3886e3bbcf935a22a4be55"}]}