)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b90da57f641ec97c2d890763b559509dae9e72a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"should updates the router:external attribute to False"},{"line_number":11,"context_line":"if there is no other access_as_external rbac policy on the network."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Tempest API test patch: https://review.openstack.org/#/c/520255/"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibdbe8a88581e54250259825bbf1c77485fd09f89"},{"line_number":16,"context_line":"Closes-Bug: #1692472"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"ff82abbf_93d1e35a","line":13,"updated":"2017-11-20 14:07:43.000000000","message":"I don\u0027t think that it\u0027s necessary to point to tempest patch in commit message but that\u0027s only mine opinion :)","commit_id":"65a9bb3ce680da1bb49f8372c02c8b7c391a8ff6"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"20896ff1b8601142f9d89abbc95f2a4d284ebe6d","unresolved":false,"context_lines":[{"line_number":10,"context_line":"should updates the router:external attribute to False"},{"line_number":11,"context_line":"if there is no other access_as_external rbac policy on the network."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Tempest API test patch: https://review.openstack.org/#/c/520255/"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibdbe8a88581e54250259825bbf1c77485fd09f89"},{"line_number":16,"context_line":"Closes-Bug: #1692472"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"ff82abbf_ac108101","line":13,"in_reply_to":"ff82abbf_93d1e35a","updated":"2017-11-28 22:41:36.000000000","message":"It\u0027s fine. Probably could link to Change-Id instead but that\u0027s a matter of taste.","commit_id":"65a9bb3ce680da1bb49f8372c02c8b7c391a8ff6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"c81596cf92d17cf276474f7a96bb1e4477282a1f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Update network external attribute for RBAC change"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a network\u0027s RBAC external attribute delete, we"},{"line_number":10,"context_line":"should updates the router:external attribute to False"},{"line_number":11,"context_line":"if there is no other access_as_external rbac policy on the network."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"df87a7cf_26532e3b","line":9,"range":{"start_line":9,"start_character":39,"end_line":9,"end_character":45},"updated":"2017-12-11 12:17:24.000000000","message":"is deleted","commit_id":"8212228bd2e1c573ff7b7b155a125dc1146420de"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"ded0ff5a40c6cbe07694e44d21fd5e3d4b45608d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Update network external attribute for RBAC change"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a network\u0027s RBAC external attribute delete, we"},{"line_number":10,"context_line":"should updates the router:external attribute to False"},{"line_number":11,"context_line":"if there is no other access_as_external rbac policy on the network."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"df87a7cf_c4fb4744","line":9,"range":{"start_line":9,"start_character":39,"end_line":9,"end_character":45},"in_reply_to":"df87a7cf_26532e3b","updated":"2017-12-12 01:25:24.000000000","message":"Done","commit_id":"8212228bd2e1c573ff7b7b155a125dc1146420de"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"c81596cf92d17cf276474f7a96bb1e4477282a1f","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Update network external attribute for RBAC change"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a network\u0027s RBAC external attribute delete, we"},{"line_number":10,"context_line":"should updates the router:external attribute to False"},{"line_number":11,"context_line":"if there is no other access_as_external rbac policy on the network."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Tempest API test patch: https://review.openstack.org/#/c/520255/"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"df87a7cf_c6336213","line":10,"range":{"start_line":10,"start_character":7,"end_line":10,"end_character":14},"updated":"2017-12-11 12:17:24.000000000","message":"update","commit_id":"8212228bd2e1c573ff7b7b155a125dc1146420de"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"ded0ff5a40c6cbe07694e44d21fd5e3d4b45608d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Update network external attribute for RBAC change"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a network\u0027s RBAC external attribute delete, we"},{"line_number":10,"context_line":"should updates the router:external attribute to False"},{"line_number":11,"context_line":"if there is no other access_as_external rbac policy on the network."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Tempest API test patch: https://review.openstack.org/#/c/520255/"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"df87a7cf_e4f68b5d","line":10,"range":{"start_line":10,"start_character":7,"end_line":10,"end_character":14},"in_reply_to":"df87a7cf_c6336213","updated":"2017-12-12 01:25:24.000000000","message":"Done","commit_id":"8212228bd2e1c573ff7b7b155a125dc1146420de"}],"neutron/db/external_net_db.py":[{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"891383e7ee1a9cea03df3827e2c09cd9c96c2a4b","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            msg \u003d _(\"Only admins can manipulate policies on networks they \""},{"line_number":179,"context_line":"                    \"do not own\")"},{"line_number":180,"context_line":"            raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":181,"context_line":"        self._process_l3_update(context, net,"},{"line_number":182,"context_line":"                                {external_net.EXTERNAL: False},"},{"line_number":183,"context_line":"                                allow_all\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_af5916ff","line":180,"range":{"start_line":180,"start_character":18,"end_line":180,"end_character":36},"updated":"2017-10-19 09:52:48.000000000","message":"Shouldn\u0027t be n_exc.NotAuthorized?","commit_id":"5e2a9baf3ce2cc2f32596a4938a6d79d4be760fb"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"f83fb14f29f8920eed3290ffa8a0bf7b0e25ed0a","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            msg \u003d _(\"Only admins can manipulate policies on networks they \""},{"line_number":179,"context_line":"                    \"do not own\")"},{"line_number":180,"context_line":"            raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":181,"context_line":"        self._process_l3_update(context, net,"},{"line_number":182,"context_line":"                                {external_net.EXTERNAL: False},"},{"line_number":183,"context_line":"                                allow_all\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_b2060086","line":180,"range":{"start_line":180,"start_character":18,"end_line":180,"end_character":36},"in_reply_to":"3f4b6375_2f4886ca","updated":"2017-10-20 06:44:02.000000000","message":"+1 for Pawel\nInvalidInput is raised for creation as you may use wrong parameters, so for deletion having NotAuthorized make sense.","commit_id":"5e2a9baf3ce2cc2f32596a4938a6d79d4be760fb"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"1327dc8dc696cbfe777b6cc93ecc6ac37c7de20b","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            msg \u003d _(\"Only admins can manipulate policies on networks they \""},{"line_number":179,"context_line":"                    \"do not own\")"},{"line_number":180,"context_line":"            raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":181,"context_line":"        self._process_l3_update(context, net,"},{"line_number":182,"context_line":"                                {external_net.EXTERNAL: False},"},{"line_number":183,"context_line":"                                allow_all\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_2f4886ca","line":180,"range":{"start_line":180,"start_character":18,"end_line":180,"end_character":36},"in_reply_to":"3f4b6375_af5916ff","updated":"2017-10-19 10:09:17.000000000","message":"Yes, maybe NotAuthorized will be better, but some old codes use InvailidInput like in L164. Let\u0027s see other reviewers\u0027 opinion, thanks.","commit_id":"5e2a9baf3ce2cc2f32596a4938a6d79d4be760fb"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"f00e0fa803ac2c1fb9faf5eb269e5956b6c5cb8c","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            msg \u003d _(\"Only admins can manipulate policies on networks they \""},{"line_number":179,"context_line":"                    \"do not own\")"},{"line_number":180,"context_line":"            raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":181,"context_line":"        self._process_l3_update(context, net,"},{"line_number":182,"context_line":"                                {external_net.EXTERNAL: False},"},{"line_number":183,"context_line":"                                allow_all\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_3228d0e0","line":180,"range":{"start_line":180,"start_character":18,"end_line":180,"end_character":36},"in_reply_to":"3f4b6375_b2060086","updated":"2017-10-20 06:58:07.000000000","message":"Thanks, Ann and Pawel. I will update.","commit_id":"5e2a9baf3ce2cc2f32596a4938a6d79d4be760fb"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"76af6b48501dcb2a691c49d3cd243ac5d92a7490","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {external_net.EXTERNAL: False},"},{"line_number":181,"context_line":"                                allow_all\u003dFalse)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, (events.BEFORE_UPDATE,"},{"line_number":184,"context_line":"                                       events.BEFORE_DELETE))"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f4b6375_90c0807d","line":181,"range":{"start_line":181,"start_character":32,"end_line":181,"end_character":47},"updated":"2017-10-21 22:09:35.000000000","message":"Nit: I don\u0027t think you need this, in case you roll another patchset","commit_id":"72ad87d6998857b2641053e422f594dbde6e9166"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"50b7959731533458438a70e081506606b69f969b","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, [events.AFTER_DELETE])"},{"line_number":172,"context_line":"    def _process_ext_policy_delete(self, resource, event, trigger, context,"},{"line_number":173,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_06d20fd3","line":174,"updated":"2017-10-23 07:48:20.000000000","message":"nit: should\u0027t constant constant from https://github.com/openstack/neutron-lib/blob/master/neutron_lib/callbacks/resources.py be used instead of \"network\"","commit_id":"4c6268568c27097431b50cdd787c803d986cefcd"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"0a2405c937cb5d134038ff6ebc52a7c1cf51b419","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, [events.AFTER_DELETE])"},{"line_number":172,"context_line":"    def _process_ext_policy_delete(self, resource, event, trigger, context,"},{"line_number":173,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_c12861e3","line":174,"in_reply_to":"3f4b6375_06d20fd3","updated":"2017-10-23 08:38:02.000000000","message":"IMO, this constant is not related to the callback resource, but it\u0027s really hardcode here and other places(L157, L187).","commit_id":"4c6268568c27097431b50cdd787c803d986cefcd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"665fed5e8dec766220de8d1c9a765fd5d6950bed","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, [events.AFTER_DELETE])"},{"line_number":172,"context_line":"    def _process_ext_policy_delete(self, resource, event, trigger, context,"},{"line_number":173,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_3c1bda81","line":174,"in_reply_to":"3f4b6375_c12861e3","updated":"2017-10-23 09:32:01.000000000","message":"ok, I could be wrong, thx for explanation","commit_id":"4c6268568c27097431b50cdd787c803d986cefcd"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"c44c483e39e786b6b40b36f34992cacdc7839e82","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, [events.AFTER_DELETE])"},{"line_number":172,"context_line":"    def _process_ext_policy_delete(self, resource, event, trigger, context,"},{"line_number":173,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            raise n_exc.NotAuthorized()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f4b6375_c4d8287c","line":175,"range":{"start_line":174,"start_character":8,"end_line":175,"end_character":18},"updated":"2017-10-25 10:21:40.000000000","message":"Should consider an external network with access_as_shared attibute here. We should not update the router:external attribute while deleting rbac policy.","commit_id":"b59533beeb3ce3d50012c2f53936d66162d64ce1"},{"author":{"_account_id":7715,"name":"Hirofumi Ichihara","email":"ichihara.hirofumi@gmail.com","username":"Hirofumi-Ichihara"},"change_message_id":"43debbbb8b475a964c50aa085b5f662dd97beeb9","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {external_net.EXTERNAL: False})"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f4b6375_cda18c96","line":178,"range":{"start_line":177,"start_character":8,"end_line":178,"end_character":39},"updated":"2017-10-24 13:41:39.000000000","message":"When we hit this case, although rbac policy was deleted, network\u0027s external keeped True, right? It means that bug isn\u0027t solved.","commit_id":"b59533beeb3ce3d50012c2f53936d66162d64ce1"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"4130a65cae21c34a2453993d1676b6d96d5a643b","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {external_net.EXTERNAL: False})"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f4b6375_d5c9ff58","line":178,"range":{"start_line":177,"start_character":8,"end_line":178,"end_character":39},"in_reply_to":"3f4b6375_01440b1a","updated":"2017-10-25 01:29:35.000000000","message":"Yes, I think so. When I use a non admin user create rbac policy with access_with_external action, if forbiddens:\nForbidden: ((((rule:create_rbac_policy and rule:create_rbac_policy:target_tenant) and rule:create_rbac_policy:action) and rule:create_rbac_policy:object_type) and rule:create_rbac_policy:object_id) is disallowed by policy\nNeutron server returns request_ids: [\u0027req-ac77aa48-7749-4dcb-8a3b-bc3948d8370a\u0027]","commit_id":"b59533beeb3ce3d50012c2f53936d66162d64ce1"},{"author":{"_account_id":7715,"name":"Hirofumi Ichihara","email":"ichihara.hirofumi@gmail.com","username":"Hirofumi-Ichihara"},"change_message_id":"3856225514cca4526de0a1baea27c88b5ac75516","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {external_net.EXTERNAL: False})"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f4b6375_01440b1a","line":178,"range":{"start_line":177,"start_character":8,"end_line":178,"end_character":39},"in_reply_to":"3f4b6375_90eba7da","updated":"2017-10-24 15:16:27.000000000","message":"You mean that this case never hapen? Non admin can create rbac policy with access_as_external action, right?","commit_id":"b59533beeb3ce3d50012c2f53936d66162d64ce1"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"0982035385741c10a34f4cf3d25bcbe179416673","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":177,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":178,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {external_net.EXTERNAL: False})"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f4b6375_90eba7da","line":178,"range":{"start_line":177,"start_character":8,"end_line":178,"end_character":39},"in_reply_to":"3f4b6375_cda18c96","updated":"2017-10-24 14:08:18.000000000","message":"If not admin, we can\u0027t setup rbac policy for external network. This was verified in L161 when we creating rbac policy.","commit_id":"b59533beeb3ce3d50012c2f53936d66162d64ce1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"af87ee9e1d3e81ed5c8184cd58906ba0db1b682e","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        if (object_type \u003d\u003d \u0027network\u0027 and"},{"line_number":177,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_3d901195","line":176,"updated":"2017-10-26 08:00:27.000000000","message":"nit: this first condition will be always true here so it can be removed IMO\nYou can also merge those two \"ifs\" into one:\n    if object_type !\u003d \u0027network\u0027 or policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"98bdc5e7bc1f55fa4d135e6342223ab6d95edf1c","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":174,"context_line":"        if object_type !\u003d \u0027network\u0027:"},{"line_number":175,"context_line":"            return"},{"line_number":176,"context_line":"        if (object_type \u003d\u003d \u0027network\u0027 and"},{"line_number":177,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_fd3039a1","line":176,"in_reply_to":"3f4b6375_3d901195","updated":"2017-10-26 08:51:13.000000000","message":"Good point, thanks.","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"ae9f1af12b021f87081771491582831eb1e248c6","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":179,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":180,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":181,"context_line":"        self._process_l3_update(context, net,"},{"line_number":182,"context_line":"                                {extnet_apidef.EXTERNAL: False})"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1f485f77_9db61c3e","line":180,"range":{"start_line":179,"start_character":8,"end_line":180,"end_character":39},"updated":"2017-11-16 09:32:00.000000000","message":"I think it is better to check this in BEFORE_DELETE event. If we do this in AFTER_DELETE event, there is a case where a RBAC entry has been deleted but this method fails.","commit_id":"dfda14d7d293694100b86a1aae504127e49aace8"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"8a1eafaf6c2eedc79d42eadd4c6c50357a4fc214","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":179,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":180,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":181,"context_line":"        self._process_l3_update(context, net,"},{"line_number":182,"context_line":"                                {extnet_apidef.EXTERNAL: False})"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1f485f77_5378958e","line":180,"range":{"start_line":179,"start_character":8,"end_line":180,"end_character":39},"in_reply_to":"1f485f77_9db61c3e","updated":"2017-11-16 10:00:06.000000000","message":"OK, I will add to _validate_ext_not_in_use_by_tenant method, I think both BEFORE_UPDATE and BEFORE_DELETE need this check.","commit_id":"dfda14d7d293694100b86a1aae504127e49aace8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"bc5c01ec78575a0c7d0b9277c2dbb529d0d2c288","unresolved":false,"context_lines":[{"line_number":173,"context_line":"    def _process_ext_policy_delete(self, resource, event, trigger, context,"},{"line_number":174,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":175,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"},{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_adacd79e","line":176,"updated":"2017-11-17 20:30:46.000000000","message":"nit: isn\u0027t \"access_as_external\" here the same as constant in https://github.com/openstack/neutron/blob/8f79a92e45c84819cfc4d1cda99c9320ae78bbaf/neutron/db/rbac_db_models.py ?\nBut it is IMO not necessary to change that if You will not do any other change :)","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"f9ab7278875bf14172ec531edf7184e574217bf4","unresolved":false,"context_lines":[{"line_number":173,"context_line":"    def _process_ext_policy_delete(self, resource, event, trigger, context,"},{"line_number":174,"context_line":"                                   object_type, policy, **kwargs):"},{"line_number":175,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"},{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_dc51ff37","line":176,"in_reply_to":"1f485f77_adacd79e","updated":"2017-11-18 03:43:42.000000000","message":"@Slawek, if you\u0027re not mind, I will change in another patch, there are some similar in L159 and other places.","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d5d76a5250c4179797ff4a3c9ad843f14d636e3d","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {extnet_apidef.EXTERNAL: False})"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, (events.BEFORE_UPDATE,"},{"line_number":183,"context_line":"                                       events.BEFORE_DELETE))"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_f3674e4c","line":180,"updated":"2017-11-17 22:24:06.000000000","message":"is it correct? what if the network had two rules for two tenants? don\u0027t you reset it on first rule deletion while another is still there?","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"f9ab7278875bf14172ec531edf7184e574217bf4","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":179,"context_line":"        self._process_l3_update(context, net,"},{"line_number":180,"context_line":"                                {extnet_apidef.EXTERNAL: False})"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, (events.BEFORE_UPDATE,"},{"line_number":183,"context_line":"                                       events.BEFORE_DELETE))"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_1c53173b","line":180,"in_reply_to":"1f485f77_f3674e4c","updated":"2017-11-18 03:43:42.000000000","message":"Thanks, Ihar. You\u0027re right. I will make a query here.","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d5d76a5250c4179797ff4a3c9ad843f14d636e3d","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":191,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":192,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":193,"context_line":"        new_tenant \u003d None"},{"line_number":194,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"},{"line_number":195,"context_line":"            new_tenant \u003d kwargs[\u0027policy_update\u0027][\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_d35a1281","line":192,"updated":"2017-11-17 22:24:06.000000000","message":"could you please explain how this change is relevant to the bug","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"20896ff1b8601142f9d89abbc95f2a4d284ebe6d","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":191,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":192,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":193,"context_line":"        new_tenant \u003d None"},{"line_number":194,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"},{"line_number":195,"context_line":"            new_tenant \u003d kwargs[\u0027policy_update\u0027][\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff82abbf_8ca9fd90","line":192,"in_reply_to":"1f485f77_9c3ca7e1","updated":"2017-11-28 22:41:36.000000000","message":"Akihiro suggested that AFTER_DELETE was not the right place for the check, but it still doesn\u0027t explain how this check is related to the goal of the patch.","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"f9ab7278875bf14172ec531edf7184e574217bf4","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":191,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":192,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":193,"context_line":"        new_tenant \u003d None"},{"line_number":194,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"},{"line_number":195,"context_line":"            new_tenant \u003d kwargs[\u0027policy_update\u0027][\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_9c3ca7e1","line":192,"in_reply_to":"1f485f77_d35a1281","updated":"2017-11-18 03:43:42.000000000","message":"We need to check the privilege in BEFORE_DELETE event, also I think this is can apply to BEFORE_UPDATE event, please see Akihiro Motoki\u0027s comment in PS14.","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"3996ccd2996ff16806a50fcd4faa97f9b5a979db","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":191,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":192,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":193,"context_line":"        new_tenant \u003d None"},{"line_number":194,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"},{"line_number":195,"context_line":"            new_tenant \u003d kwargs[\u0027policy_update\u0027][\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff82abbf_cdf12e97","line":192,"in_reply_to":"ff82abbf_8ca9fd90","updated":"2017-11-29 14:21:22.000000000","message":"A non admin user can create rbac policy with access_with_external action for specified tenant_id.\nIf the network\u0027s tenant_id not match with context\u0027s tenant_id, this should raise an exception.\nSo I think it\u0027s need to check before we do update and delete operations.","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7e38a9b7d192c2f6148f649a175a994c4a62ccc9","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":191,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":192,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":193,"context_line":"        new_tenant \u003d None"},{"line_number":194,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"},{"line_number":195,"context_line":"            new_tenant \u003d kwargs[\u0027policy_update\u0027][\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff82abbf_9489ea75","line":192,"in_reply_to":"ff82abbf_cdf12e97","updated":"2017-11-29 18:48:04.000000000","message":"But isn\u0027t it the same regardless of the fix? What I imply is that maybe it\u0027s a separate bug and so it should have a separate report in LP and a separate patch in gerrit.","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b90da57f641ec97c2d890763b559509dae9e72a","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":193,"context_line":"            return"},{"line_number":194,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":195,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":196,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":197,"context_line":"        new_tenant \u003d None"},{"line_number":198,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff82abbf_b3d6674f","line":195,"updated":"2017-11-20 14:07:43.000000000","message":"nit: shoudn\u0027t You use \"project_id\" instead of tenant_id already?","commit_id":"65a9bb3ce680da1bb49f8372c02c8b7c391a8ff6"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"5a1873fbc4934f4cd7476c1c738e8b656e55aa35","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":193,"context_line":"            return"},{"line_number":194,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":195,"context_line":"        if not context.is_admin and net[\u0027tenant_id\u0027] !\u003d context.tenant_id:"},{"line_number":196,"context_line":"            raise n_exc.NotAuthorized()"},{"line_number":197,"context_line":"        new_tenant \u003d None"},{"line_number":198,"context_line":"        if event \u003d\u003d events.BEFORE_UPDATE:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff82abbf_5d41f70a","line":195,"in_reply_to":"ff82abbf_b3d6674f","updated":"2017-11-22 02:49:13.000000000","message":"project_is is same as tenant_id, I think both is ok, just keeps the old behavior.","commit_id":"65a9bb3ce680da1bb49f8372c02c8b7c391a8ff6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"35ede934d9e83f2bc14dea49a72eb1b7cbafeb3b","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"},{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_63f2969c","line":180,"range":{"start_line":178,"start_character":11,"end_line":180,"end_character":71},"updated":"2017-12-10 09:47:25.000000000","message":"I would add separate variable and use it in \"if\" condition, something like:\n\n    network_as_external \u003d context.session....count()\n    if network_as_external:\n        return\n\nIMHO something like that is much more readable.","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"08bf7a0975996f96f2a54d1d59a5e04ef221df6d","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"},{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_b7df7367","line":180,"range":{"start_line":178,"start_character":11,"end_line":180,"end_character":71},"in_reply_to":"df87a7cf_1e63e1b3","updated":"2017-12-11 06:41:47.000000000","message":"Done","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"08bf7a0975996f96f2a54d1d59a5e04ef221df6d","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"},{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_f7f17b02","line":180,"range":{"start_line":178,"start_character":11,"end_line":180,"end_character":71},"in_reply_to":"df87a7cf_63f2969c","updated":"2017-12-11 06:41:47.000000000","message":"Done","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":15309,"name":"zhaobo","email":"bzhaojyathousandy@gmail.com","username":"ZhaoBo"},"change_message_id":"1005c48f612ff0bd2fa2fc68e07b47cc5be7143e","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"},{"line_number":176,"context_line":"                policy[\u0027action\u0027] !\u003d \u0027access_as_external\u0027):"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_1e63e1b3","line":180,"range":{"start_line":178,"start_character":11,"end_line":180,"end_character":71},"in_reply_to":"df87a7cf_63f2969c","updated":"2017-12-10 11:36:11.000000000","message":"Right, also you could add some comments to describe what this condition does. I think that would be good.","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"35ede934d9e83f2bc14dea49a72eb1b7cbafeb3b","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"},{"line_number":184,"context_line":"                                {extnet_apidef.EXTERNAL: False})"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_232ecefa","line":181,"updated":"2017-12-10 09:47:25.000000000","message":"I would also add some log message here to inform operator why it is not updated.\n\nAnd second thing. Did You add test case to cover such case also?","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"08bf7a0975996f96f2a54d1d59a5e04ef221df6d","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"},{"line_number":184,"context_line":"                                {extnet_apidef.EXTERNAL: False})"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_974eef35","line":181,"in_reply_to":"df87a7cf_232ecefa","updated":"2017-12-11 06:41:47.000000000","message":"I agrees with ZhaoBo here, the deletion of rbac policy will not impact the external network.\n \nOK, is it test_delete_network_rbac_external_with_multi_rbac_policy in [1] good for you?\n\n[1] https://review.openstack.org/#/c/512484/20/neutron/tests/unit/db/test_rbac_db_mixin.py","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":15309,"name":"zhaobo","email":"bzhaojyathousandy@gmail.com","username":"ZhaoBo"},"change_message_id":"1005c48f612ff0bd2fa2fc68e07b47cc5be7143e","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"},{"line_number":184,"context_line":"                                {extnet_apidef.EXTERNAL: False})"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_de5819fb","line":181,"in_reply_to":"df87a7cf_232ecefa","updated":"2017-12-10 11:36:11.000000000","message":"look at other functions, I think it\u0027s better not log here, if you no raise some errors, hide it is OK. But it\u0027s true that the UTs no cover this logic.","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"08bf7a0975996f96f2a54d1d59a5e04ef221df6d","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if context.session.query(rbac_db.NetworkRBAC).filter("},{"line_number":179,"context_line":"            rbac_db.NetworkRBAC.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":180,"context_line":"            rbac_db.NetworkRBAC.action \u003d\u003d \u0027access_as_external\u0027).count():"},{"line_number":181,"context_line":"            return"},{"line_number":182,"context_line":"        net \u003d self.get_network(context, policy[\u0027object_id\u0027])"},{"line_number":183,"context_line":"        self._process_l3_update(context, net,"},{"line_number":184,"context_line":"                                {extnet_apidef.EXTERNAL: False})"}],"source_content_type":"text/x-python","patch_set":20,"id":"df87a7cf_d771d777","line":181,"in_reply_to":"df87a7cf_de5819fb","updated":"2017-12-11 06:41:47.000000000","message":"Please see the above comments.","commit_id":"f704a0fceeb20929a68a9a9dfe6318ec0150cea9"}],"neutron/tests/unit/db/test_rbac_db_mixin.py":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"f407dd864f2cb008e02e8de397ca9442e2b30854","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        port \u003d self.plugin.create_port(self.context, test_port)"},{"line_number":55,"context_line":"        return netrbac, port"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def test_delete_network_rbac_external(self):"},{"line_number":58,"context_line":"        with self.network() as ext_net,\\"},{"line_number":59,"context_line":"            mock.patch.object(external_net,"},{"line_number":60,"context_line":"                              \u0027_process_l3_update\u0027) as l3_update:"},{"line_number":61,"context_line":"            self.plugin.update_network("},{"line_number":62,"context_line":"                self.context, ext_net[\u0027network\u0027][\u0027id\u0027],"},{"line_number":63,"context_line":"                {\u0027network\u0027: {\u0027router:external\u0027: False}})"},{"line_number":64,"context_line":"            policy \u003d {\u0027rbac_policy\u0027:"},{"line_number":65,"context_line":"                      {\u0027tenant_id\u0027: ext_net[\u0027network\u0027][\u0027tenant_id\u0027],"},{"line_number":66,"context_line":"                       \u0027object_id\u0027: ext_net[\u0027network\u0027][\u0027id\u0027],"},{"line_number":67,"context_line":"                       \u0027object_type\u0027: \u0027network\u0027,"},{"line_number":68,"context_line":"                       \u0027action\u0027: \u0027access_as_external\u0027,"},{"line_number":69,"context_line":"                       \u0027target_tenant\u0027: \u0027*\u0027}}"},{"line_number":70,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":73,"context_line":"            self.assertEqual(3, l3_update.call_count)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def test_update_networkrbac_valid(self):"},{"line_number":76,"context_line":"        orig_target \u003d \u0027test-tenant-2\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f4b6375_b3b8302a","line":73,"range":{"start_line":57,"start_character":0,"end_line":73,"end_character":53},"updated":"2017-10-20 22:04:41.000000000","message":"Why do we need to mock if we can test the code directly? Consider this option or something similar:     \n\n    def _assert_external_net_state(self, net_id, is_external\u003dFalse):\n        req \u003d self.new_show_request(\u0027networks\u0027, net_id)\n        res \u003d self.deserialize(self.fmt, req.get_response(self.api))\n        self.assertEqual(is_external, res[\u0027network\u0027][\u0027router:external\u0027])\n\n    def test_delete_network_rbac_external(self):\n        with self.network() as ext_net:\n            net_id \u003d ext_net[\u0027network\u0027][\u0027id\u0027]\n            self._assert_external_net_state(net_id)\n            policy \u003d {\u0027rbac_policy\u0027:\n                      {\u0027tenant_id\u0027: ext_net[\u0027network\u0027][\u0027tenant_id\u0027],\n                       \u0027object_id\u0027: net_id,\n                       \u0027object_type\u0027: \u0027network\u0027,\n                       \u0027action\u0027: \u0027access_as_external\u0027,\n                       \u0027target_tenant\u0027: \u0027*\u0027}}\n            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)\n            self._assert_external_net_state(net_id, is_external\u003dTrue)\n            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])\n            self._assert_external_net_state(net_id)","commit_id":"82f40e24dcde30bf052bdd8bbda9aacc042fd98a"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"189a35937dfe3374834e5f94e160df3055ffccf0","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        port \u003d self.plugin.create_port(self.context, test_port)"},{"line_number":55,"context_line":"        return netrbac, port"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def test_delete_network_rbac_external(self):"},{"line_number":58,"context_line":"        with self.network() as ext_net,\\"},{"line_number":59,"context_line":"            mock.patch.object(external_net,"},{"line_number":60,"context_line":"                              \u0027_process_l3_update\u0027) as l3_update:"},{"line_number":61,"context_line":"            self.plugin.update_network("},{"line_number":62,"context_line":"                self.context, ext_net[\u0027network\u0027][\u0027id\u0027],"},{"line_number":63,"context_line":"                {\u0027network\u0027: {\u0027router:external\u0027: False}})"},{"line_number":64,"context_line":"            policy \u003d {\u0027rbac_policy\u0027:"},{"line_number":65,"context_line":"                      {\u0027tenant_id\u0027: ext_net[\u0027network\u0027][\u0027tenant_id\u0027],"},{"line_number":66,"context_line":"                       \u0027object_id\u0027: ext_net[\u0027network\u0027][\u0027id\u0027],"},{"line_number":67,"context_line":"                       \u0027object_type\u0027: \u0027network\u0027,"},{"line_number":68,"context_line":"                       \u0027action\u0027: \u0027access_as_external\u0027,"},{"line_number":69,"context_line":"                       \u0027target_tenant\u0027: \u0027*\u0027}}"},{"line_number":70,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":73,"context_line":"            self.assertEqual(3, l3_update.call_count)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def test_update_networkrbac_valid(self):"},{"line_number":76,"context_line":"        orig_target \u003d \u0027test-tenant-2\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f4b6375_e8f0f6cb","line":73,"range":{"start_line":57,"start_character":0,"end_line":73,"end_character":53},"in_reply_to":"3f4b6375_b3b8302a","updated":"2017-10-21 00:43:54.000000000","message":"Yes, your method seems more straightforward, thanks.","commit_id":"82f40e24dcde30bf052bdd8bbda9aacc042fd98a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"50b7959731533458438a70e081506606b69f969b","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                       \u0027action\u0027: \u0027access_as_external\u0027,"},{"line_number":70,"context_line":"                       \u0027target_tenant\u0027: \u0027*\u0027}}"},{"line_number":71,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":72,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dTrue)"},{"line_number":73,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":74,"context_line":"            self._assert_external_net_state(net_id)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_e6d493bf","line":72,"updated":"2017-10-23 07:48:20.000000000","message":"do You really need this additional method to use it only once? maybe those 3 lines could be moved here?","commit_id":"4c6268568c27097431b50cdd787c803d986cefcd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"665fed5e8dec766220de8d1c9a765fd5d6950bed","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                       \u0027action\u0027: \u0027access_as_external\u0027,"},{"line_number":70,"context_line":"                       \u0027target_tenant\u0027: \u0027*\u0027}}"},{"line_number":71,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":72,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dTrue)"},{"line_number":73,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":74,"context_line":"            self._assert_external_net_state(net_id)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_bc0eaa38","line":72,"in_reply_to":"3f4b6375_416111a9","updated":"2017-10-23 09:32:01.000000000","message":"You\u0027re right. I missed this one in L64. sorry","commit_id":"4c6268568c27097431b50cdd787c803d986cefcd"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"0a2405c937cb5d134038ff6ebc52a7c1cf51b419","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                       \u0027action\u0027: \u0027access_as_external\u0027,"},{"line_number":70,"context_line":"                       \u0027target_tenant\u0027: \u0027*\u0027}}"},{"line_number":71,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":72,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dTrue)"},{"line_number":73,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":74,"context_line":"            self._assert_external_net_state(net_id)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_416111a9","line":72,"in_reply_to":"3f4b6375_e6d493bf","updated":"2017-10-23 08:38:02.000000000","message":"Both L64 and L74 also uses this method.","commit_id":"4c6268568c27097431b50cdd787c803d986cefcd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"af87ee9e1d3e81ed5c8184cd58906ba0db1b682e","unresolved":false,"context_lines":[{"line_number":27,"context_line":"class NetworkRbacTestcase(test_plugin.NeutronDbPluginV2TestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":30,"context_line":"        super(NetworkRbacTestcase, self).setUp(plugin\u003d\u0027ml2\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _make_networkrbac(self, network, target):"},{"line_number":33,"context_line":"        policy \u003d {\u0027rbac_policy\u0027: {\u0027tenant_id\u0027: network[\u0027network\u0027][\u0027tenant_id\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_1d8b1568","line":30,"updated":"2017-10-26 08:00:27.000000000","message":"is this change related to this patch?","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"98bdc5e7bc1f55fa4d135e6342223ab6d95edf1c","unresolved":false,"context_lines":[{"line_number":27,"context_line":"class NetworkRbacTestcase(test_plugin.NeutronDbPluginV2TestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":30,"context_line":"        super(NetworkRbacTestcase, self).setUp(plugin\u003d\u0027ml2\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _make_networkrbac(self, network, target):"},{"line_number":33,"context_line":"        policy \u003d {\u0027rbac_policy\u0027: {\u0027tenant_id\u0027: network[\u0027network\u0027][\u0027tenant_id\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_7ddd69de","line":30,"in_reply_to":"3f4b6375_1d8b1568","updated":"2017-10-26 08:51:13.000000000","message":"If we\u0027re not set this, self.network() will not contains router:external attribute.","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"af87ee9e1d3e81ed5c8184cd58906ba0db1b682e","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        port \u003d self.plugin.create_port(self.context, test_port)"},{"line_number":54,"context_line":"        return netrbac, port"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _assert_external_net_state(self, net_id, is_external\u003dFalse):"},{"line_number":57,"context_line":"        req \u003d self.new_show_request(\u0027networks\u0027, net_id)"},{"line_number":58,"context_line":"        res \u003d self.deserialize(self.fmt, req.get_response(self.api))"},{"line_number":59,"context_line":"        self.assertEqual(is_external, res[\u0027network\u0027][\u0027router:external\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_fdeb990f","line":56,"updated":"2017-10-26 08:00:27.000000000","message":"IMHO this default value isn\u0027t necessary, \"is_external\" is expected value here and should be always given explicit","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"98bdc5e7bc1f55fa4d135e6342223ab6d95edf1c","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        port \u003d self.plugin.create_port(self.context, test_port)"},{"line_number":54,"context_line":"        return netrbac, port"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _assert_external_net_state(self, net_id, is_external\u003dFalse):"},{"line_number":57,"context_line":"        req \u003d self.new_show_request(\u0027networks\u0027, net_id)"},{"line_number":58,"context_line":"        res \u003d self.deserialize(self.fmt, req.get_response(self.api))"},{"line_number":59,"context_line":"        self.assertEqual(is_external, res[\u0027network\u0027][\u0027router:external\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_282fb188","line":56,"in_reply_to":"3f4b6375_fdeb990f","updated":"2017-10-26 08:51:13.000000000","message":"Sure, both is ok for me. I will update.","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"af87ee9e1d3e81ed5c8184cd58906ba0db1b682e","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":72,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dTrue)"},{"line_number":73,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":74,"context_line":"            self._assert_external_net_state(net_id)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_delete_external_network_shared_rbac(self):"},{"line_number":77,"context_line":"        with self.network() as ext_net:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_ddee9d20","line":74,"updated":"2017-10-26 08:00:27.000000000","message":"nit: I would pass here explicit \"is_external\u003dFals\" for better readablitiy of test","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"98bdc5e7bc1f55fa4d135e6342223ab6d95edf1c","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            net_rbac \u003d self.plugin.create_rbac_policy(self.context, policy)"},{"line_number":72,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dTrue)"},{"line_number":73,"context_line":"            self.plugin.delete_rbac_policy(self.context, net_rbac[\u0027id\u0027])"},{"line_number":74,"context_line":"            self._assert_external_net_state(net_id)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_delete_external_network_shared_rbac(self):"},{"line_number":77,"context_line":"        with self.network() as ext_net:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_e852b912","line":74,"in_reply_to":"3f4b6375_ddee9d20","updated":"2017-10-26 08:51:13.000000000","message":"Done","commit_id":"41529eeb84dadb64b7e350ec4854b4af8acac0a7"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d5d76a5250c4179797ff4a3c9ad843f14d636e3d","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        res \u003d self.deserialize(self.fmt, req.get_response(self.api))"},{"line_number":59,"context_line":"        self.assertEqual(is_external, res[\u0027network\u0027][\u0027router:external\u0027])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_delete_network_rbac_external(self):"},{"line_number":62,"context_line":"        with self.network() as ext_net:"},{"line_number":63,"context_line":"            net_id \u003d ext_net[\u0027network\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_b3d2f6cf","line":61,"updated":"2017-11-17 22:24:06.000000000","message":"that\u0027s fine, but I would like to see tempest api tests for the issue (note that they should be proposed for the new neutron-tempest-plugin repository).\n\nscenarios to cover:\n\n- create rule / delete rule / check that network is not external\n- create rule for tenant1 / create rule for tenant2 / delete rule for tenant1 / check that network is still external","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"f9ab7278875bf14172ec531edf7184e574217bf4","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        res \u003d self.deserialize(self.fmt, req.get_response(self.api))"},{"line_number":59,"context_line":"        self.assertEqual(is_external, res[\u0027network\u0027][\u0027router:external\u0027])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_delete_network_rbac_external(self):"},{"line_number":62,"context_line":"        with self.network() as ext_net:"},{"line_number":63,"context_line":"            net_id \u003d ext_net[\u0027network\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"            self._assert_external_net_state(net_id, is_external\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f485f77_dc1fbf69","line":61,"in_reply_to":"1f485f77_b3d2f6cf","updated":"2017-11-18 03:43:42.000000000","message":"Sure, there is a patch land in neutron-tempest-plugin:\nhttps://review.openstack.org/#/c/520255/","commit_id":"5516d54eec55597bc30c2a7cb38445d9b06acfd9"}]}