)]}'
{"patrole_tempest_plugin/tests/api/network/rbac_base.py":[{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        cls.admin_client \u003d cls.os_adm.agents_client"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def _get_agent(cls, agent_type\u003dNone):"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        agent \u003d dict()"},{"line_number":48,"context_line":"        agent[\u0027agent_type\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_c41457ea","line":45,"updated":"2017-02-01 00:57:59.000000000","message":"Is this going to be used by multiple tests?","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        cls.admin_client \u003d cls.os_adm.agents_client"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def _get_agent(cls, agent_type\u003dNone):"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        agent \u003d dict()"},{"line_number":48,"context_line":"        agent[\u0027agent_type\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_69bd2202","line":45,"in_reply_to":"3a461143_c41457ea","updated":"2017-02-01 17:32:11.000000000","message":"Yes, but this should be part of a different patchset.  I will remove it for now and re-add it when needed.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"67a5de5602e019e22959dd15498ec295626c74a1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return agent"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"class BaseRouterRbacTest(BaseNetworkRbacTest):"},{"line_number":71,"context_line":"    def _cleanup_router(self, router):"},{"line_number":72,"context_line":"        try:"},{"line_number":73,"context_line":"            self.delete_router(router)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_ce8d58d4","line":70,"range":{"start_line":70,"start_character":6,"end_line":70,"end_character":24},"updated":"2017-01-31 23:59:09.000000000","message":"ignore me if this is extended by multiple tests... but, did we agree on where to put the internal methods for the tests? did we want them in the rbac_base even if it affects 1 file, or did we want them as a part of the test_x_rbac files themselves?","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return agent"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"class BaseRouterRbacTest(BaseNetworkRbacTest):"},{"line_number":71,"context_line":"    def _cleanup_router(self, router):"},{"line_number":72,"context_line":"        try:"},{"line_number":73,"context_line":"            self.delete_router(router)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_c907f67f","line":70,"range":{"start_line":70,"start_character":6,"end_line":70,"end_character":24},"in_reply_to":"3a461143_ce8d58d4","updated":"2017-02-01 17:32:11.000000000","message":"It depends on if you think there will be more than one test set that needs the base class.  I chose to make this it\u0027s own base class because I expected there will be other tests that can extend this base class.  However, if you disagree we can definitely move these methods into the test class until the base class is needed.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"}],"patrole_tempest_plugin/tests/api/network/test_routers_rbac.py":[{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        super(RbacRouterTest, cls).resource_setup()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        # Create an external network by admin"},{"line_number":47,"context_line":"        post_body \u003d {\u0027name\u0027: data_utils.rand_name(\u0027network-admin-\u0027)}"},{"line_number":48,"context_line":"        post_body[\u0027router:external\u0027] \u003d True"},{"line_number":49,"context_line":"        body \u003d cls.networks_client.create_network(**post_body)"},{"line_number":50,"context_line":"        cls.admin_network \u003d body[\u0027network\u0027]"},{"line_number":51,"context_line":"        cls.networks.append(cls.admin_network)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_84adefea","line":49,"range":{"start_line":47,"start_character":8,"end_line":49,"end_character":62},"updated":"2017-02-01 00:57:59.000000000","message":"Is there a reason you\u0027re not using the built-in create_network function? It\u0027s possible to pass kwargs","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        super(RbacRouterTest, cls).resource_setup()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        # Create an external network by admin"},{"line_number":47,"context_line":"        post_body \u003d {\u0027name\u0027: data_utils.rand_name(\u0027network-admin-\u0027)}"},{"line_number":48,"context_line":"        post_body[\u0027router:external\u0027] \u003d True"},{"line_number":49,"context_line":"        body \u003d cls.networks_client.create_network(**post_body)"},{"line_number":50,"context_line":"        cls.admin_network \u003d body[\u0027network\u0027]"},{"line_number":51,"context_line":"        cls.networks.append(cls.admin_network)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_962b4994","line":49,"range":{"start_line":47,"start_character":8,"end_line":49,"end_character":62},"in_reply_to":"3a461143_84adefea","updated":"2017-02-01 17:32:11.000000000","message":"I will look into this and try to use the built in function.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        cls.admin_subnet \u003d cls.create_subnet(cls.admin_network)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        router_name \u003d data_utils.rand_name(\u0027router-admin\u0027)"},{"line_number":56,"context_line":"        cls.admin_router \u003d cls.create_router(router_name\u003drouter_name)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _try_delete_interface_resource(self, delete_callable, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_04ef3f1e","line":55,"updated":"2017-02-01 00:57:59.000000000","message":"create_router will automatically generate a name for you, this isn\u0027t necessary","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        cls.admin_subnet \u003d cls.create_subnet(cls.admin_network)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        router_name \u003d data_utils.rand_name(\u0027router-admin\u0027)"},{"line_number":56,"context_line":"        cls.admin_router \u003d cls.create_router(router_name\u003drouter_name)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _try_delete_interface_resource(self, delete_callable, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_d61b117f","line":55,"in_reply_to":"3a461143_04ef3f1e","updated":"2017-02-01 17:32:11.000000000","message":"Good point.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        # or BadRequest exceptions thrown for interface that was correctly"},{"line_number":63,"context_line":"        # deleted by the test."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        try:"},{"line_number":66,"context_line":"            delete_callable(*args, **kwargs)"},{"line_number":67,"context_line":"        # if resource is not found, this means it was deleted in the test"},{"line_number":68,"context_line":"        except exceptions.NotFound:"},{"line_number":69,"context_line":"            pass"},{"line_number":70,"context_line":"        except exceptions.BadRequest:"},{"line_number":71,"context_line":"            pass"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_a4bd8b03","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":16},"updated":"2017-02-01 00:57:59.000000000","message":"Can this be condensed using call_and_ignore_notfound_exc?","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        # or BadRequest exceptions thrown for interface that was correctly"},{"line_number":63,"context_line":"        # deleted by the test."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        try:"},{"line_number":66,"context_line":"            delete_callable(*args, **kwargs)"},{"line_number":67,"context_line":"        # if resource is not found, this means it was deleted in the test"},{"line_number":68,"context_line":"        except exceptions.NotFound:"},{"line_number":69,"context_line":"            pass"},{"line_number":70,"context_line":"        except exceptions.BadRequest:"},{"line_number":71,"context_line":"            pass"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_5608012e","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":16},"in_reply_to":"3a461143_a4bd8b03","updated":"2017-02-01 17:32:11.000000000","message":"Yes.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        service\u003d\"neutron\", rule\u003d\"update_router\")"},{"line_number":126,"context_line":"    @decorators.idempotent_id(\u00273d182f4e-0023-4218-9aa0-ea2b0ae0bd7a\u0027)"},{"line_number":127,"context_line":"    def test_update_router(self):"},{"line_number":128,"context_line":"        router \u003d self._create_router()"},{"line_number":129,"context_line":"        new_name \u003d router[\u0027name\u0027] + \u0027-new\u0027"},{"line_number":130,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":131,"context_line":"        self.routers_client.update_router(router[\u0027id\u0027], name\u003dnew_name)"},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_2457dbca","line":129,"range":{"start_line":128,"start_character":1,"end_line":129,"end_character":42},"updated":"2017-02-01 00:57:59.000000000","message":"Instead of creating a new router and then changing the name, can you use the existing router and change the description? That seems more efficient","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        service\u003d\"neutron\", rule\u003d\"update_router\")"},{"line_number":126,"context_line":"    @decorators.idempotent_id(\u00273d182f4e-0023-4218-9aa0-ea2b0ae0bd7a\u0027)"},{"line_number":127,"context_line":"    def test_update_router(self):"},{"line_number":128,"context_line":"        router \u003d self._create_router()"},{"line_number":129,"context_line":"        new_name \u003d router[\u0027name\u0027] + \u0027-new\u0027"},{"line_number":130,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":131,"context_line":"        self.routers_client.update_router(router[\u0027id\u0027], name\u003dnew_name)"},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_693362f7","line":129,"range":{"start_line":128,"start_character":1,"end_line":129,"end_character":42},"in_reply_to":"3a461143_2457dbca","updated":"2017-02-01 17:32:11.000000000","message":"Yes, I can do it that way.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        try:"},{"line_number":194,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        except exceptions.NotFound as e:"},{"line_number":197,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":200,"context_line":"                                 rule\u003d\"add_router_interface\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_a4f02bca","line":197,"range":{"start_line":196,"start_character":0,"end_line":197,"end_character":53},"updated":"2017-02-01 00:57:59.000000000","message":"Is this necessary?","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        try:"},{"line_number":194,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        except exceptions.NotFound as e:"},{"line_number":197,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":200,"context_line":"                                 rule\u003d\"add_router_interface\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_493dc6ba","line":197,"range":{"start_line":196,"start_character":0,"end_line":197,"end_character":53},"in_reply_to":"3a461143_a4f02bca","updated":"2017-02-01 17:32:11.000000000","message":"I believe this is one of the cases where what should be a 403 forbidden is instead returned as a 404 NotFound.  I will look into this more to confirm though.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"866a29028a019ad4b719c797dafd815ebe06c274","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    @decorators.idempotent_id(\u0027a0627778-d68d-4913-881b-e345360cca19\u0027)"},{"line_number":202,"context_line":"    def test_add_router_interfaces(self):"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        name \u003d data_utils.rand_name(\u0027router-network-\u0027)"},{"line_number":205,"context_line":"        network \u003d self.create_network(network_name\u003dname)"},{"line_number":206,"context_line":"        subnet \u003d self.create_subnet(network)"},{"line_number":207,"context_line":"        router \u003d self._create_router()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_0432dfa6","line":204,"updated":"2017-02-01 00:57:59.000000000","message":"See L55","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"67a5de5602e019e22959dd15498ec295626c74a1","unresolved":false,"context_lines":[{"line_number":219,"context_line":"    @decorators.idempotent_id(\u0027ff2593a4-2bff-4c27-97d3-dd3702b27dfb\u0027)"},{"line_number":220,"context_line":"    def test_remove_router_interfaces(self):"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        name \u003d data_utils.rand_name(\u0027router-network-\u0027)"},{"line_number":223,"context_line":"        network \u003d self.create_network(network_name\u003dname)"},{"line_number":224,"context_line":"        subnet \u003d self.create_subnet(network)"},{"line_number":225,"context_line":"        router \u003d self._create_router()"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self.routers_client.add_router_interface("},{"line_number":228,"context_line":"            router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_aef1ec96","line":225,"range":{"start_line":222,"start_character":8,"end_line":225,"end_character":37},"updated":"2017-01-31 23:59:09.000000000","message":"if these are shared by multiple tests, can this also be moved to resource_setup?","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"67a5de5602e019e22959dd15498ec295626c74a1","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        self.routers_client.add_router_interface("},{"line_number":228,"context_line":"            router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        self.addCleanup(self._try_delete_interface_resource,"},{"line_number":231,"context_line":"                        self.routers_client.remove_router_interface,"},{"line_number":232,"context_line":"                        router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_ae480ccd","line":232,"range":{"start_line":230,"start_character":8,"end_line":232,"end_character":61},"updated":"2017-01-31 23:59:09.000000000","message":"can these by included in the teardown?","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"1e1fb469aef637c69c78e2923afee07eaa2d0f9c","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        self.routers_client.add_router_interface("},{"line_number":228,"context_line":"            router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        self.addCleanup(self._try_delete_interface_resource,"},{"line_number":231,"context_line":"                        self.routers_client.remove_router_interface,"},{"line_number":232,"context_line":"                        router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_e97af276","line":232,"range":{"start_line":230,"start_character":8,"end_line":232,"end_character":61},"in_reply_to":"3a461143_ae480ccd","updated":"2017-02-01 17:32:11.000000000","message":"I don\u0027t see what that gets us that using an addCleanup doesn\u0027t.","commit_id":"05934a2017b5fdcdff1d920e392d171674b04976"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"657d1e3c1ad9c3ffceebc4671725f097a2e31cf9","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        \"\"\""},{"line_number":75,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":76,"context_line":"        external_gateway_info \u003d {\u0027network_id\u0027: self.admin_network[\u0027id\u0027],"},{"line_number":77,"context_line":"                                 \u0027enable_snat\u0027: False}"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":80,"context_line":"        create_body \u003d self.routers_client.create_router("}],"source_content_type":"text/x-python","patch_set":6,"id":"3a461143_4472d67d","line":77,"range":{"start_line":77,"start_character":48,"end_line":77,"end_character":53},"updated":"2017-02-02 19:28:37.000000000","message":"if you are enabling snat, does this need to be set to True?\n\n\nExample Create router with SNAT disabled\n\nPOST /v2.0/routers\nAccept: application/json\n{\n    \"router\":\n    {\n        \"name\": \"another_router\",\n        \"admin_state_up\": true,\n        \"external_gateway_info\": {\n            \"network_id\": \"3c5bcddd-6af9-4e6b-9c3e-c153e521cab8\",\n            \"enable_snat\": false}\n    }\n}\n\nhttps://specs.openstack.org/openstack/neutron-specs/specs/api/configurable_external_gateway_modes.html","commit_id":"bdb3cf0f0efd47c3e5a7b07cbae29cf8010ad0f8"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"2e96da16d6587212a12d6815fe268142c66aa277","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        RBAC test for the neutron create_router policy"},{"line_number":61,"context_line":"        \"\"\""},{"line_number":62,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":63,"context_line":"        self.routers_client.create_router()"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    @rbac_rule_validation.action("},{"line_number":66,"context_line":"        service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_931a3e16","line":63,"range":{"start_line":63,"start_character":8,"end_line":63,"end_character":43},"updated":"2017-02-02 21:21:19.000000000","message":"Need addCleanup.","commit_id":"19a6261fd293dd4d34416cb2fd42f620fb8f8ead"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"e5b15a7aa2f2851767ba3a326d0169fed0dd0e53","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        RBAC test for the neutron create_router policy"},{"line_number":61,"context_line":"        \"\"\""},{"line_number":62,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":63,"context_line":"        self.routers_client.create_router()"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    @rbac_rule_validation.action("},{"line_number":66,"context_line":"        service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_5e253dc0","line":63,"range":{"start_line":63,"start_character":8,"end_line":63,"end_character":43},"in_reply_to":"3a461143_931a3e16","updated":"2017-02-02 22:00:43.000000000","message":"Good catch, thanks :)","commit_id":"19a6261fd293dd4d34416cb2fd42f620fb8f8ead"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        RBAC test for the neutron create_router policy"},{"line_number":61,"context_line":"        \"\"\""},{"line_number":62,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":63,"context_line":"        create_body \u003d self.routers_client.create_router()"},{"line_number":64,"context_line":"        self.addCleanup(self.routers_client.delete_router,"},{"line_number":65,"context_line":"                        create_body[\u0027router\u0027][\u0027id\u0027])"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_591c4d77","line":63,"range":{"start_line":63,"start_character":8,"end_line":63,"end_character":19},"updated":"2017-02-06 20:31:29.000000000","message":"better is put it as -\n\"router \u003d self.routers_client.create_router()\"","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\"Create Router Fixed IPs"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        RBAC test for the neutron"},{"line_number":95,"context_line":"        create_router:external_gateway_info:external_fixed_ips policy"},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":98,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_dcd74bbd","line":95,"range":{"start_line":95,"start_character":22,"end_line":95,"end_character":43},"updated":"2017-02-06 20:31:29.000000000","message":"Applicable to all - \n\nIt\u0027s better to put API action description because user will not purpose of this colon cemented line.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"ee03b58f4c946bf431ffcbc753713b897865a502","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\"Create Router Fixed IPs"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        RBAC test for the neutron"},{"line_number":95,"context_line":"        create_router:external_gateway_info:external_fixed_ips policy"},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":98,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_ef01826b","line":95,"range":{"start_line":95,"start_character":22,"end_line":95,"end_character":43},"in_reply_to":"1a430d35_dcd74bbd","updated":"2017-02-07 16:21:40.000000000","message":"This test is created to test the specified policy.  Therefore, I feel it is best to include the exact name of the policy being tested.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"e5f555edbfe07fa4a72638a14e5160fcf63a5180","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\"Create Router Fixed IPs"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        RBAC test for the neutron"},{"line_number":95,"context_line":"        create_router:external_gateway_info:external_fixed_ips policy"},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":98,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_2a02983d","line":95,"range":{"start_line":95,"start_character":22,"end_line":95,"end_character":43},"in_reply_to":"1a430d35_ef01826b","updated":"2017-02-07 16:31:52.000000000","message":"It was only suggestion. So we can use this also.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                                 \u0027external_fixed_ips\u0027: [external_fixed_ips]}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":108,"context_line":"        create_body \u003d self.routers_client.create_router("},{"line_number":109,"context_line":"            name\u003dname, external_gateway_info\u003dexternal_gateway_info)"},{"line_number":110,"context_line":"        self.addCleanup(self.routers_client.delete_router,"},{"line_number":111,"context_line":"                        create_body[\u0027router\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_1cf9d34d","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":19},"updated":"2017-02-06 20:31:29.000000000","message":"\u0027router\u0027 instead of \u0027created_body\u0027?","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        RBAC test for the neutron get_router policy"},{"line_number":119,"context_line":"        \"\"\""},{"line_number":120,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.routers_client.show_router(self.admin_router[\u0027id\u0027])"},{"line_number":123,"context_line":"        except exceptions.NotFound as e:"},{"line_number":124,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_1ca71350","line":121,"range":{"start_line":121,"start_character":8,"end_line":121,"end_character":11},"updated":"2017-02-06 20:31:29.000000000","message":"try - except block is not needed. It is auto handled and will fail when no router_id is there. Also when no router_id is there, failure is \"client error\" not an action failure.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"ee03b58f4c946bf431ffcbc753713b897865a502","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        RBAC test for the neutron get_router policy"},{"line_number":119,"context_line":"        \"\"\""},{"line_number":120,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.routers_client.show_router(self.admin_router[\u0027id\u0027])"},{"line_number":123,"context_line":"        except exceptions.NotFound as e:"},{"line_number":124,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_8f2d5e00","line":121,"range":{"start_line":121,"start_character":8,"end_line":121,"end_character":11},"in_reply_to":"1a430d35_1ca71350","updated":"2017-02-07 16:21:40.000000000","message":"It is needed because for this api endpoint, if the role attempting to access the endpoint is not allowed, it will return 404 NotFound rather than 403 Forbidden as is expected.  We need to catch that 404 and then raise an RbacActionFailed exception so the decorator will know that the test did not fail.  This is a temporary measure until we can start fixing the incorrect errors in various services.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"e5f555edbfe07fa4a72638a14e5160fcf63a5180","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        RBAC test for the neutron get_router policy"},{"line_number":119,"context_line":"        \"\"\""},{"line_number":120,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.routers_client.show_router(self.admin_router[\u0027id\u0027])"},{"line_number":123,"context_line":"        except exceptions.NotFound as e:"},{"line_number":124,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_ca6bd4ef","line":121,"range":{"start_line":121,"start_character":8,"end_line":121,"end_character":11},"in_reply_to":"1a430d35_8f2d5e00","updated":"2017-02-07 16:31:52.000000000","message":"Looked into the decorator.\nAs another except block to handle these 404 errors is added there as per our discussion, yes in this case try except is needed.\nThanks.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        RBAC test for the neutron update_router policy"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        new_name \u003d self.admin_router[\u0027name\u0027] + \u0027-new\u0027"},{"line_number":135,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":136,"context_line":"        self.routers_client.update_router(self.admin_router[\u0027id\u0027],"},{"line_number":137,"context_line":"                                          name\u003dnew_name)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_9f24d5af","line":134,"range":{"start_line":134,"start_character":8,"end_line":134,"end_character":53},"updated":"2017-02-06 20:31:29.000000000","message":"This is not a good way to create new name. use data_utils.rand_name module.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":178,"context_line":"        self.routers_client.update_router("},{"line_number":179,"context_line":"            self.admin_router[\u0027id\u0027],"},{"line_number":180,"context_line":"            external_gateway_info\u003d{\u0027network_id\u0027: self.admin_network[\u0027id\u0027],"},{"line_number":181,"context_line":"                                   \u0027enable_snat\u0027: True})"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    @rbac_rule_validation.action("}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_1f00e5a9","line":180,"range":{"start_line":180,"start_character":49,"end_line":180,"end_character":53},"updated":"2017-02-06 20:31:29.000000000","message":"This test case is for enable_snat, if \u0027network_id\u0027 is not mandatory, it should be removed. Because it can create a dependency on network_id parameter.\n\nif it is mandatory when passing enable_snat then ignore this comment.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"c4534fa699715dbe85fab6f72d6871948afbc6ae","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":178,"context_line":"        self.routers_client.update_router("},{"line_number":179,"context_line":"            self.admin_router[\u0027id\u0027],"},{"line_number":180,"context_line":"            external_gateway_info\u003d{\u0027network_id\u0027: self.admin_network[\u0027id\u0027],"},{"line_number":181,"context_line":"                                   \u0027enable_snat\u0027: True})"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    @rbac_rule_validation.action("}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_6a9bc072","line":180,"range":{"start_line":180,"start_character":49,"end_line":180,"end_character":53},"in_reply_to":"1a430d35_1f00e5a9","updated":"2017-02-07 16:24:04.000000000","message":"It is needed.\n\nDetails: {u\u0027message\u0027: u\"Invalid input for external_gateway_info. Reason: Validation of dictionary\u0027s keys failed. Expected keys: set([\u0027network_id\u0027]) Provided keys: set([u\u0027enable_snat\u0027]).\", u\u0027type\u0027: u\u0027HTTPBadRequest\u0027, u\u0027detail\u0027: u\u0027\u0027}","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"e5f555edbfe07fa4a72638a14e5160fcf63a5180","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":178,"context_line":"        self.routers_client.update_router("},{"line_number":179,"context_line":"            self.admin_router[\u0027id\u0027],"},{"line_number":180,"context_line":"            external_gateway_info\u003d{\u0027network_id\u0027: self.admin_network[\u0027id\u0027],"},{"line_number":181,"context_line":"                                   \u0027enable_snat\u0027: True})"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    @rbac_rule_validation.action("}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_0a615c05","line":180,"range":{"start_line":180,"start_character":49,"end_line":180,"end_character":53},"in_reply_to":"1a430d35_6a9bc072","updated":"2017-02-07 16:31:52.000000000","message":"Thanks for clarification.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        \"\"\"Update Router External Gateway Info External Fixed Ips"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        RBAC test for the neutron"},{"line_number":191,"context_line":"        update_router:external_gateway_info:external_fixed_ips policy"},{"line_number":192,"context_line":"        \"\"\""},{"line_number":193,"context_line":"        # Pick an ip address within the allocation_pools range"},{"line_number":194,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_5f08ad91","line":191,"range":{"start_line":191,"start_character":8,"end_line":191,"end_character":69},"updated":"2017-02-06 20:31:29.000000000","message":"ditto","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"c9a8eb0a2964552b6127e5336c98ca7458b9aa53","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        \"\"\""},{"line_number":214,"context_line":"        router \u003d self.create_router()"},{"line_number":215,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":216,"context_line":"        try:"},{"line_number":217,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":218,"context_line":"        except exceptions.NotFound as e:"},{"line_number":219,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_df857dad","line":216,"range":{"start_line":216,"start_character":8,"end_line":216,"end_character":11},"updated":"2017-02-06 20:31:29.000000000","message":"try except block no needed.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"e5f555edbfe07fa4a72638a14e5160fcf63a5180","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        \"\"\""},{"line_number":214,"context_line":"        router \u003d self.create_router()"},{"line_number":215,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":216,"context_line":"        try:"},{"line_number":217,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":218,"context_line":"        except exceptions.NotFound as e:"},{"line_number":219,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_aa3848f0","line":216,"range":{"start_line":216,"start_character":8,"end_line":216,"end_character":11},"in_reply_to":"1a430d35_4f9e1664","updated":"2017-02-07 16:31:52.000000000","message":"Agreed.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"ee03b58f4c946bf431ffcbc753713b897865a502","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        \"\"\""},{"line_number":214,"context_line":"        router \u003d self.create_router()"},{"line_number":215,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":216,"context_line":"        try:"},{"line_number":217,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":218,"context_line":"        except exceptions.NotFound as e:"},{"line_number":219,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_4f9e1664","line":216,"range":{"start_line":216,"start_character":8,"end_line":216,"end_character":11},"in_reply_to":"1a430d35_df857dad","updated":"2017-02-07 16:21:40.000000000","message":"It is needed because for this api endpoint, if the role attempting to access the endpoint is not allowed, it will return 404 NotFound rather than 403 Forbidden as is expected.  We need to catch that 404 and then raise an RbacActionFailed exception so the decorator will know that the test did not fail.  This is a temporary measure until we can start fixing the incorrect errors in various services.","commit_id":"d370e6bbeda15f8bf100d0f181082805ea59109f"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"9ed831013bda7e0f62936b95e0e49d5963316fdf","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.routers_client.show_router(self.admin_router[\u0027id\u0027])"},{"line_number":123,"context_line":"        except exceptions.NotFound as e:"},{"line_number":124,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    @rbac_rule_validation.action("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_4227c8c9","line":123,"updated":"2017-02-07 18:01:56.000000000","message":"As this except block is intentional, it\u0027s better to put a message specifying reason to use this block.\n\nIt will help us identifying wrong code issues in other projects.","commit_id":"73af21c985941b683188db14daf11ddc9e4ade9e"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"9ed831013bda7e0f62936b95e0e49d5963316fdf","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":216,"context_line":"        try:"},{"line_number":217,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":218,"context_line":"        except exceptions.NotFound as e:"},{"line_number":219,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_22155c48","line":218,"range":{"start_line":218,"start_character":8,"end_line":218,"end_character":40},"updated":"2017-02-07 18:01:56.000000000","message":"ditto","commit_id":"73af21c985941b683188db14daf11ddc9e4ade9e"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"9ed831013bda7e0f62936b95e0e49d5963316fdf","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        try:"},{"line_number":235,"context_line":"            self.routers_client.add_router_interface("},{"line_number":236,"context_line":"                router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":237,"context_line":"        except exceptions.NotFound as e:"},{"line_number":238,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        self.addCleanup(test_utils.call_and_ignore_notfound_exc,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_620be42a","line":237,"range":{"start_line":237,"start_character":8,"end_line":237,"end_character":40},"updated":"2017-02-07 18:01:56.000000000","message":"ditto","commit_id":"73af21c985941b683188db14daf11ddc9e4ade9e"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"9ed831013bda7e0f62936b95e0e49d5963316fdf","unresolved":false,"context_lines":[{"line_number":267,"context_line":"            self.routers_client.remove_router_interface("},{"line_number":268,"context_line":"                router[\u0027id\u0027],"},{"line_number":269,"context_line":"                subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":270,"context_line":"        except exceptions.NotFound as e:"},{"line_number":271,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_c5002204","line":270,"range":{"start_line":270,"start_character":8,"end_line":270,"end_character":39},"updated":"2017-02-07 18:01:56.000000000","message":"ditto","commit_id":"73af21c985941b683188db14daf11ddc9e4ade9e"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"d56e4abdaf1f6b80d423369a39115f0e597d63fe","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        create_router:external_gateway_info:external_fixed_ips policy"},{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":100,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"},{"line_number":101,"context_line":"        ip_address \u003d str(netaddr.IPAddress(cidr) + 10)"},{"line_number":102,"context_line":"        external_fixed_ips \u003d {\u0027subnet_id\u0027: self.admin_subnet[\u0027id\u0027],"},{"line_number":103,"context_line":"                              \u0027ip_address\u0027: ip_address}"}],"source_content_type":"text/x-python","patch_set":13,"id":"1a430d35_66b90b15","line":100,"range":{"start_line":100,"start_character":38,"end_line":100,"end_character":50},"updated":"2017-02-08 14:59:43.000000000","message":"Just a suggestion (don\u0027t consider it a comment)\nI this cidr parameter should come from conf rather deriving from subnet","commit_id":"f00273d572d3e9807088ae357d1bebe6a196f831"},{"author":{"_account_id":23185,"name":"Samantha Blanco","email":"samantha.blanco@att.com","username":"sblanco"},"change_message_id":"7c6d52ee58b631d614067120229394756515d761","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                     \"This is irregular and should be fixed.\")"},{"line_number":249,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":252,"context_line":"                        self.routers_client.remove_router_interface,"},{"line_number":253,"context_line":"                        router[\u0027id\u0027],"},{"line_number":254,"context_line":"                        subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":257,"context_line":"                                 rule\u003d\"remove_router_interface\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"1a430d35_9531bf5c","line":254,"range":{"start_line":251,"start_character":8,"end_line":254,"end_character":47},"updated":"2017-02-08 14:21:48.000000000","message":"Shouldn\u0027t this be moved to the try block? If only for consistency and readability","commit_id":"f00273d572d3e9807088ae357d1bebe6a196f831"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"b5212a1b34999e71cf035ff378a81f00157d0ad5","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                     \"This is irregular and should be fixed.\")"},{"line_number":249,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":252,"context_line":"                        self.routers_client.remove_router_interface,"},{"line_number":253,"context_line":"                        router[\u0027id\u0027],"},{"line_number":254,"context_line":"                        subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":257,"context_line":"                                 rule\u003d\"remove_router_interface\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"1a430d35_50e7653f","line":254,"range":{"start_line":251,"start_character":8,"end_line":254,"end_character":47},"updated":"2017-02-08 14:27:57.000000000","message":"This should be moved to try block for sure, or try-except-else block can be used and thus addCleanup will move to \"else\" block of try-except-else.","commit_id":"f00273d572d3e9807088ae357d1bebe6a196f831"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"07f4984946b500feb6ec429b88df8651b4ec3a8f","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":100,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"},{"line_number":101,"context_line":"        ip_address \u003d str(netaddr.IPAddress(cidr) + 10)"},{"line_number":102,"context_line":"        external_fixed_ips \u003d {\u0027subnet_id\u0027: self.admin_subnet[\u0027id\u0027],"},{"line_number":103,"context_line":"                              \u0027ip_address\u0027: ip_address}"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a430d35_51d67d28","line":101,"range":{"start_line":101,"start_character":25,"end_line":101,"end_character":53},"updated":"2017-02-08 18:36:19.000000000","message":"Quick questions -\n1. What if cidr specified in the conf file for a particular oepnstack deployment is such that it allows only few IPs say \u00277\u0027. In this case above IP address will in increased beyond allocation pool.\n2. How it would be randomly choosing  IP address from the subnet allocation pool  ?\n##################################################\n# something like (just prototyping) :\nthis should go in resource setup \n    cls.admin_allocation_pool \u003d self.admin_subnet.allocation_pool\n\n# Now in test case : \ntmp_allocation_pool_list \u003d self.admin_allocation_pool\nip_address \u003d pop(tmp_allocation_pool_list) \n# we can use pop or random. pop seem much better to me.","commit_id":"232fc711ffe91bf10389f27d575187eb727f3b33"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"b86919d7ed8f4e33ea0b5e872b8710b41c261fd4","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":100,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"},{"line_number":101,"context_line":"        ip_address \u003d str(netaddr.IPAddress(cidr) + 10)"},{"line_number":102,"context_line":"        external_fixed_ips \u003d {\u0027subnet_id\u0027: self.admin_subnet[\u0027id\u0027],"},{"line_number":103,"context_line":"                              \u0027ip_address\u0027: ip_address}"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a430d35_389e2eb9","line":101,"range":{"start_line":101,"start_character":25,"end_line":101,"end_character":53},"in_reply_to":"1a430d35_51d67d28","updated":"2017-02-08 20:44:29.000000000","message":"I\u0027ll take the same approach that I took on the port tests.","commit_id":"232fc711ffe91bf10389f27d575187eb727f3b33"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"b61fe86f7be323b1ac9f305fcd96d17794b10b8b","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        name \u003d data_utils.rand_name(\u0027snat-router\u0027)"},{"line_number":100,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"},{"line_number":101,"context_line":"        ip_address \u003d str(netaddr.IPAddress(cidr) + 10)"},{"line_number":102,"context_line":"        external_fixed_ips \u003d {\u0027subnet_id\u0027: self.admin_subnet[\u0027id\u0027],"},{"line_number":103,"context_line":"                              \u0027ip_address\u0027: ip_address}"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a430d35_03cd7d30","line":101,"range":{"start_line":101,"start_character":25,"end_line":101,"end_character":53},"in_reply_to":"1a430d35_51d67d28","updated":"2017-02-09 03:19:09.000000000","message":"The following code\n\nnet \u003d netaddr.IPNetwork(CONF.network.project_network_v6_cidr)\ngateway \u003d str(netaddr.IPAddress(net.first + 2))\n\nis used in https://github.com/openstack/tempest/blob/4fe213ad544528d90e430548ebc73b04d2735e79/tempest/api/network/test_networks.py\n\nDon\u0027t know much about this, but maybe it\u0027s safer to do\nip_address \u003d str((netaddr.IPAddress(cidr)).first + 2) or something instead?","commit_id":"232fc711ffe91bf10389f27d575187eb727f3b33"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"07f4984946b500feb6ec429b88df8651b4ec3a8f","unresolved":false,"context_lines":[{"line_number":243,"context_line":"            self.routers_client.add_router_interface("},{"line_number":244,"context_line":"                router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":245,"context_line":"            self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":246,"context_line":"                        self.routers_client.remove_router_interface,"},{"line_number":247,"context_line":"                        router[\u0027id\u0027],"},{"line_number":248,"context_line":"                        subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":249,"context_line":"        except exceptions.NotFound as e:"},{"line_number":250,"context_line":"            LOG.info(\"NotFound exception caught. Exception is thrown when \""},{"line_number":251,"context_line":"                     \"role doesn\u0027t have access to the endpoint.\""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a430d35_f1e5a908","line":248,"range":{"start_line":246,"start_character":0,"end_line":248,"end_character":47},"updated":"2017-02-08 18:36:19.000000000","message":"pep8 failure is here. Kindly fix it.","commit_id":"232fc711ffe91bf10389f27d575187eb727f3b33"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"b86919d7ed8f4e33ea0b5e872b8710b41c261fd4","unresolved":false,"context_lines":[{"line_number":243,"context_line":"            self.routers_client.add_router_interface("},{"line_number":244,"context_line":"                router[\u0027id\u0027], subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":245,"context_line":"            self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":246,"context_line":"                        self.routers_client.remove_router_interface,"},{"line_number":247,"context_line":"                        router[\u0027id\u0027],"},{"line_number":248,"context_line":"                        subnet_id\u003dsubnet[\u0027id\u0027])"},{"line_number":249,"context_line":"        except exceptions.NotFound as e:"},{"line_number":250,"context_line":"            LOG.info(\"NotFound exception caught. Exception is thrown when \""},{"line_number":251,"context_line":"                     \"role doesn\u0027t have access to the endpoint.\""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a430d35_583dba8d","line":248,"range":{"start_line":246,"start_character":0,"end_line":248,"end_character":47},"in_reply_to":"1a430d35_f1e5a908","updated":"2017-02-08 20:44:29.000000000","message":"The idea behind code review is for human eyes to catch things that automated tests can\u0027t. Please restrict your comments to things that aren\u0027t already caught by our Jenkins gates.","commit_id":"232fc711ffe91bf10389f27d575187eb727f3b33"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"0a61d9d153b76c3b151a96a319f4d766feb21306","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        \"\"\""},{"line_number":202,"context_line":"        # Pick an ip address within the allocation_pools range"},{"line_number":203,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"},{"line_number":204,"context_line":"        ip_address \u003d str(netaddr.IPAddress(cidr) + 10)"},{"line_number":205,"context_line":"        external_fixed_ips \u003d {\u0027subnet_id\u0027: self.admin_subnet[\u0027id\u0027],"},{"line_number":206,"context_line":"                              \u0027ip_address\u0027: ip_address}"},{"line_number":207,"context_line":"        external_gateway_info \u003d {\u0027network_id\u0027: self.admin_network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a430d35_f9d81e55","line":204,"range":{"start_line":204,"start_character":25,"end_line":204,"end_character":53},"updated":"2017-02-09 04:26:20.000000000","message":"Somehow, this is missed :) Please fix this.","commit_id":"268e6304dfd44ada742d8bd287ab0f72cf551ca5"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"48cc53460e2b7e23f928c5c2ca1f3be4be8bc594","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        \"\"\""},{"line_number":202,"context_line":"        # Pick an ip address within the allocation_pools range"},{"line_number":203,"context_line":"        cidr \u003d netaddr.IPNetwork(self.admin_subnet[\u0027cidr\u0027])"},{"line_number":204,"context_line":"        ip_address \u003d str(netaddr.IPAddress(cidr) + 10)"},{"line_number":205,"context_line":"        external_fixed_ips \u003d {\u0027subnet_id\u0027: self.admin_subnet[\u0027id\u0027],"},{"line_number":206,"context_line":"                              \u0027ip_address\u0027: ip_address}"},{"line_number":207,"context_line":"        external_gateway_info \u003d {\u0027network_id\u0027: self.admin_network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a430d35_b9705641","line":204,"range":{"start_line":204,"start_character":25,"end_line":204,"end_character":53},"in_reply_to":"1a430d35_f9d81e55","updated":"2017-02-09 04:30:41.000000000","message":"Whoops.","commit_id":"268e6304dfd44ada742d8bd287ab0f72cf551ca5"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"367d06862169aecd6af5f8e4c5f051c228c4baca","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":210,"context_line":"        self.routers_client.update_router("},{"line_number":211,"context_line":"            self.admin_router[\u0027id\u0027],"},{"line_number":212,"context_line":"            external_gateway_info\u003dexternal_gateway_info)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":16,"id":"1a430d35_f4ae4980","line":211,"range":{"start_line":211,"start_character":12,"end_line":211,"end_character":29},"updated":"2017-02-09 17:00:54.000000000","message":"#Same comment as in port testcases\nJust a suggestion, \nAs you are creating port from base class method, created resource will be deleted at class level i.e. once all the tests in this are done.\nIn this particular test, what\u0027s happening, for created port you are adding IP address. This IP is taken randomly. \nAnd this IP will remain with this created port till end of class. \nIf another testcase in same class also uses same random choice and come with same IP address during port IP update/assignment then it can cause a problem.\nSo, better is to -\na. In such cases, create/clean ports local to testcase method or\nb. use pop instead of random, so that no overlapping IPs can come but this requires smart handling.\nFor me for such testcases, best is to take local port create/cleanup within the testcase method.","commit_id":"09588582f10c75491ea16f13987e996bfb81101a"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"ba346dc8a18079ce0263e459c8c85c5ae0200a7a","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":210,"context_line":"        self.routers_client.update_router("},{"line_number":211,"context_line":"            self.admin_router[\u0027id\u0027],"},{"line_number":212,"context_line":"            external_gateway_info\u003dexternal_gateway_info)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":16,"id":"1a430d35_be2f38a6","line":211,"range":{"start_line":211,"start_character":12,"end_line":211,"end_character":29},"in_reply_to":"1a430d35_9eeefcb2","updated":"2017-02-09 19:25:10.000000000","message":"Yes, this is probably better.","commit_id":"09588582f10c75491ea16f13987e996bfb81101a"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"5d8bfbd99c389554660eda93fefaaf6f4a11b44e","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":210,"context_line":"        self.routers_client.update_router("},{"line_number":211,"context_line":"            self.admin_router[\u0027id\u0027],"},{"line_number":212,"context_line":"            external_gateway_info\u003dexternal_gateway_info)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":16,"id":"1a430d35_9eeefcb2","line":211,"range":{"start_line":211,"start_character":12,"end_line":211,"end_character":29},"in_reply_to":"1a430d35_f4ae4980","updated":"2017-02-09 19:20:55.000000000","message":"In testing I saw no negative effects from assigning two routers to the same set of external_fixed_ips.  In this case just to be safe though, I think the best solution is to remove the external_gateway_info that references the external_fixed_ips in a cleanup.","commit_id":"09588582f10c75491ea16f13987e996bfb81101a"}]}