)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"1429c5f6ab9eb8a89fcb723833e3c1d1bb76544d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Use Router OVO in external_net_db"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Router OVO is created in path [1]."},{"line_number":10,"context_line":"This patch uses Router object in external_net_db"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1]https://review.openstack.org/#/c/516961/"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ff82abbf_3d5561fa","line":9,"range":{"start_line":9,"start_character":25,"end_line":9,"end_character":29},"updated":"2017-11-29 21:15:26.000000000","message":"patch","commit_id":"9902f67638f9e9febd2fb98f0ff61043ea264a33"}],"neutron/db/external_net_db.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0c708336f316b9ab234f196e6bdb75881988e3be","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":189,"context_line":"        router \u003d l3_obj.Router.get_object("},{"line_number":190,"context_line":"            context,"},{"line_number":191,"context_line":"            l3_obj.Router.gw_port_id.in_(ports)"},{"line_number":192,"context_line":"        )"},{"line_number":193,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":194,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_4f48075b","line":191,"updated":"2017-11-25 11:14:19.000000000","message":"are You sure that UUIDField() object has got \"in_()\" method to call on it?","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"e381a0cc049666988648cf155b706e9bf6d58a7a","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":189,"context_line":"        router \u003d l3_obj.Router.get_object("},{"line_number":190,"context_line":"            context,"},{"line_number":191,"context_line":"            l3_obj.Router.gw_port_id.in_(ports)"},{"line_number":192,"context_line":"        )"},{"line_number":193,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":194,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_30c4d8f4","line":191,"in_reply_to":"ff82abbf_4f48075b","updated":"2017-11-28 11:05:51.000000000","message":"Thanks for your comment. I\u0027m working on it to find an equivalent method for \"in\" operator","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0c708336f316b9ab234f196e6bdb75881988e3be","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":194,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":195,"context_line":"            router \u003d router.filter("},{"line_number":196,"context_line":"                l3_obj.Router.tenant_id \u003d\u003d policy[\u0027target_tenant\u0027])"},{"line_number":197,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":198,"context_line":"            # router lookup because they will have access either way"},{"line_number":199,"context_line":"            if context.session.query(rbac_db.NetworkRBAC).filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_6f45cb42","line":196,"updated":"2017-11-25 11:14:19.000000000","message":"IMO we should use \"project_id\" in all new code currently","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"e381a0cc049666988648cf155b706e9bf6d58a7a","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":194,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":195,"context_line":"            router \u003d router.filter("},{"line_number":196,"context_line":"                l3_obj.Router.tenant_id \u003d\u003d policy[\u0027target_tenant\u0027])"},{"line_number":197,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":198,"context_line":"            # router lookup because they will have access either way"},{"line_number":199,"context_line":"            if context.session.query(rbac_db.NetworkRBAC).filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_d0b6cc8a","line":196,"in_reply_to":"ff82abbf_6f45cb42","updated":"2017-11-28 11:05:51.000000000","message":"Done","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0c708336f316b9ab234f196e6bdb75881988e3be","unresolved":false,"context_lines":[{"line_number":217,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":218,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":219,"context_line":"            router \u003d router.filter("},{"line_number":220,"context_line":"                ~l3_obj.Router.tenant_id.in_(tenants_with_entries))"},{"line_number":221,"context_line":"            if new_tenant:"},{"line_number":222,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":223,"context_line":"                # interfaces that belong to the new target."}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_0f78bf8d","line":220,"updated":"2017-11-25 11:14:19.000000000","message":"see my comments above","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0c708336f316b9ab234f196e6bdb75881988e3be","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":223,"context_line":"                # interfaces that belong to the new target."},{"line_number":224,"context_line":"                router \u003d router.filter("},{"line_number":225,"context_line":"                    l3_obj.Router.tenant_id !\u003d new_tenant)"},{"line_number":226,"context_line":"        if router.count():"},{"line_number":227,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":228,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_2f7d837e","line":225,"updated":"2017-11-25 11:14:19.000000000","message":"IMO should be Router.project_id (I know that it\u0027s the same currently)","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"e381a0cc049666988648cf155b706e9bf6d58a7a","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":223,"context_line":"                # interfaces that belong to the new target."},{"line_number":224,"context_line":"                router \u003d router.filter("},{"line_number":225,"context_line":"                    l3_obj.Router.tenant_id !\u003d new_tenant)"},{"line_number":226,"context_line":"        if router.count():"},{"line_number":227,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":228,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_b0af8828","line":225,"in_reply_to":"ff82abbf_2f7d837e","updated":"2017-11-28 11:05:51.000000000","message":"Done","commit_id":"46bb75e41e87d535c7f54a5d0b0a63f3267334fe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c713b40a208846e82375b13f7cfeabe29b37eecd","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":187,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":188,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":189,"context_line":"        router \u003d l3_obj.Router.get_object.filter("},{"line_number":190,"context_line":"            context,"},{"line_number":191,"context_line":"            l3_obj.Router.gw_port_id.in_(ports)"},{"line_number":192,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_dd8a3562","line":189,"range":{"start_line":189,"start_character":32,"end_line":189,"end_character":41},"updated":"2017-11-29 21:01:50.000000000","message":"eh, what does get_object.filter mean?.. It should be:\n\n\nrouters \u003d l3_obj.Router.get_objects(context, gw_port_id\u003dports)\n\nAlso since line 195 applies an additional filter, you may want to first construct complete filter set, then pass all keyword arguments into get_objects to avoid the need to filter by project_id in python instead of db layer.","commit_id":"9902f67638f9e9febd2fb98f0ff61043ea264a33"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e457e42bcc2f27699de318506a8a7f4f16a1d7ef","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":189,"context_line":"        router \u003d l3_obj.Router.get_object.filter("},{"line_number":190,"context_line":"            context,"},{"line_number":191,"context_line":"            l3_obj.Router.gw_port_id.in_(ports)"},{"line_number":192,"context_line":"        )"},{"line_number":193,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":194,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_9bcda3b8","line":191,"updated":"2017-11-29 13:21:28.000000000","message":"I\u0027m still almost 100% sure that it will not work. I think that there is exactly same method in OVO \"world\" like in sqlalchemy.","commit_id":"9902f67638f9e9febd2fb98f0ff61043ea264a33"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c713b40a208846e82375b13f7cfeabe29b37eecd","unresolved":false,"context_lines":[{"line_number":217,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":218,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":219,"context_line":"            router \u003d router.filter("},{"line_number":220,"context_line":"                ~l3_obj.Router.project_id.in_(tenants_with_entries))"},{"line_number":221,"context_line":"            if new_tenant:"},{"line_number":222,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":223,"context_line":"                # interfaces that belong to the new target."}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_9deeed1f","line":220,"updated":"2017-11-29 21:01:50.000000000","message":".filter should be changed to correct kwargs passed into get_objects","commit_id":"9902f67638f9e9febd2fb98f0ff61043ea264a33"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8efa1d3cc9ed38ed766b8c5bd7c722bf569c02e6","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":219,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":220,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":221,"context_line":"            router \u003d ~l3_obj.Router.get_objects("},{"line_number":222,"context_line":"                context,"},{"line_number":223,"context_line":"                project_id\u003dtenants_with_entries"},{"line_number":224,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_f09ce26b","line":221,"range":{"start_line":221,"start_character":21,"end_line":221,"end_character":25},"updated":"2017-11-30 19:24:20.000000000","message":"This tilda (~) thing is something wrong. I think the fact that the patch hasn\u0027t broken unit tests for the code suggest that test coverage is really bad here. Because of that, I fee like we should first add test coverage for this code in a separate patch, only then touch the code with OVO migration. Otherwise I don\u0027t feel safe to merge the migration change.","commit_id":"5008ee6b20f83ac75518513e744de05aa46036d3"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"0e95d51906534c58939fdf0b74f5da0a0a2d5acd","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":219,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":220,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":221,"context_line":"            router \u003d ~l3_obj.Router.get_objects("},{"line_number":222,"context_line":"                context,"},{"line_number":223,"context_line":"                project_id\u003dtenants_with_entries"},{"line_number":224,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":5,"id":"df87a7cf_87b63dfa","line":221,"range":{"start_line":221,"start_character":21,"end_line":221,"end_character":25},"in_reply_to":"ff82abbf_f09ce26b","updated":"2017-12-05 13:56:50.000000000","message":"Good day Ihar,\nAccording to your suggestion, I rebased my patch upon [1].\nIt does trigger the code in my case, so I think we can rely on that for testing migration patch.\n- However I ran into other failed cases when testing that UT for \"router\" (line 189) for \"test_delete_network_rbac_external_with_multi_rbac_policy\" and \"test_delete_network_rbac_external_with_multi_rbac_policy\"\n\n- And I still haven\u0027t found away for rewriting tilde \"~\" operator (line 221). \n\nDo you have any suggestion on those problems? I\u0027m kinda stuck here.\nThank you for your help!\n[1]https://review.openstack.org/#/c/512484/","commit_id":"5008ee6b20f83ac75518513e744de05aa46036d3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"74b4f699e299dc07cd3beaa4d07b4e5c46be6461","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        filters \u003d {"},{"line_number":204,"context_line":"            \u0027gw_port_id\u0027: ports"},{"line_number":205,"context_line":"        }"},{"line_number":206,"context_line":"        router \u003d l3_obj.Router.get_objects(context, **filters)"},{"line_number":207,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":208,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":209,"context_line":"            filters[\u0027project_id\u0027] \u003d policy[\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"df87a7cf_dfba8e6c","line":206,"updated":"2017-12-07 14:20:34.000000000","message":"this is wasteful in case target_tenant !\u003d *. Move the call after line 208","commit_id":"f4a03a2352f811d0b3f30549f82a59bd4c7f9770"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"74b4f699e299dc07cd3beaa4d07b4e5c46be6461","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":230,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":231,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":232,"context_line":"            filters \u003d {"},{"line_number":233,"context_line":"                \u0027project_id\u0027: tenants_with_entries"},{"line_number":234,"context_line":"            }"},{"line_number":235,"context_line":"            router \u003d l3_obj.Router.get_objects(context, **filters)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df87a7cf_9f8726b6","line":232,"updated":"2017-12-07 14:20:34.000000000","message":"we should apply both tenant_id and gw_port_id filters (latter is in line 204 of old version).","commit_id":"f4a03a2352f811d0b3f30549f82a59bd4c7f9770"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"74b4f699e299dc07cd3beaa4d07b4e5c46be6461","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            filters \u003d {"},{"line_number":233,"context_line":"                \u0027project_id\u0027: tenants_with_entries"},{"line_number":234,"context_line":"            }"},{"line_number":235,"context_line":"            router \u003d l3_obj.Router.get_objects(context, **filters)"},{"line_number":236,"context_line":"            if not new_tenant:"},{"line_number":237,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":238,"context_line":"                # interfaces that belong to the new target."}],"source_content_type":"text/x-python","patch_set":7,"id":"df87a7cf_bf59ca8e","line":235,"updated":"2017-12-07 14:20:34.000000000","message":"-\u003e routers","commit_id":"f4a03a2352f811d0b3f30549f82a59bd4c7f9770"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e3ad5dd8e7952591ae63a6cc5bf9124e50686d4d","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_target_tenant("},{"line_number":209,"context_line":"                context, ports, policy[\u0027target_tenant\u0027])"},{"line_number":210,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":211,"context_line":"            # router lookup because they will have access either way"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_8b5bc852","line":208,"range":{"start_line":208,"start_character":12,"end_line":208,"end_character":19},"updated":"2017-12-14 09:40:05.000000000","message":"where is this used? it is not clear what you are doing here","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d5df6000b224618c642fc68d3ad1c1e3e14cee9c","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_target_tenant("},{"line_number":209,"context_line":"                context, ports, policy[\u0027target_tenant\u0027])"},{"line_number":210,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":211,"context_line":"            # router lookup because they will have access either way"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_a5f9144e","line":208,"range":{"start_line":208,"start_character":12,"end_line":208,"end_character":19},"in_reply_to":"df87a7cf_6509ccdb","updated":"2017-12-14 14:30:39.000000000","message":"BTW when you move to get_objects, you should place it after line 216 so that we don\u0027t fetch anything before routers are needed.","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"d8de0fd61d8a05db8ccb77904161aa62ecacb0a3","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_target_tenant("},{"line_number":209,"context_line":"                context, ports, policy[\u0027target_tenant\u0027])"},{"line_number":210,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":211,"context_line":"            # router lookup because they will have access either way"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_cb3f8fb3","line":208,"range":{"start_line":208,"start_character":12,"end_line":208,"end_character":19},"in_reply_to":"df87a7cf_8b5bc852","updated":"2017-12-14 09:45:16.000000000","message":"Hi garyk, \"routers\" is used in L241 in old code and L234 in new code","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"935122ed80dd55b1f59dbdb0292ad8967f90cbbe","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_target_tenant("},{"line_number":209,"context_line":"                context, ports, policy[\u0027target_tenant\u0027])"},{"line_number":210,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":211,"context_line":"            # router lookup because they will have access either way"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_d6c63d17","line":208,"range":{"start_line":208,"start_character":12,"end_line":208,"end_character":19},"in_reply_to":"df87a7cf_a5f9144e","updated":"2017-12-15 08:32:11.000000000","message":"Done. Btw when researching, I think the \"count()\" method is efficient enough as we don\u0027t need to get full obj. So I use it for this case.","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"94617e54e1d1fde621e06957cddab461a738978f","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_target_tenant("},{"line_number":209,"context_line":"                context, ports, policy[\u0027target_tenant\u0027])"},{"line_number":210,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":211,"context_line":"            # router lookup because they will have access either way"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_6509ccdb","line":208,"range":{"start_line":208,"start_character":12,"end_line":208,"end_character":19},"in_reply_to":"df87a7cf_cb3f8fb3","updated":"2017-12-14 14:12:15.000000000","message":"You shouldn\u0027t need any of those new OVO methods. Neither they should return SQLAlchemy queries.\n\nI believe get_objects is just fine for the job, you just need to construct filter dict in two steps (line 206 and line 210), and then pass it into get_objects as **filters","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"94617e54e1d1fde621e06957cddab461a738978f","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":230,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":231,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":232,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_tenants_with_entries("},{"line_number":233,"context_line":"                context, ports, tenants_with_entries, new_tenant)"},{"line_number":234,"context_line":"        if routers.count():"},{"line_number":235,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_050ec0e4","line":232,"updated":"2017-12-14 14:12:15.000000000","message":"See above","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d5df6000b224618c642fc68d3ad1c1e3e14cee9c","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":230,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":231,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":232,"context_line":"            routers \u003d l3_obj.Router.get_router_filter_tenants_with_entries("},{"line_number":233,"context_line":"                context, ports, tenants_with_entries, new_tenant)"},{"line_number":234,"context_line":"        if routers.count():"},{"line_number":235,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_e5addc31","line":232,"in_reply_to":"df87a7cf_050ec0e4","updated":"2017-12-14 14:30:39.000000000","message":"OK maybe implementing all those NOTs is not easy with OVO. We can have a separate method to fetch/count routers by tenants (count_not_tenants?) and maybe leave a TODO there suggesting to implement NOT semantics in get_object(s)","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"58965f512687d1067ec23d9af938f298f025fa2d","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"},{"line_number":207,"context_line":"            \u0027gw_port_id\u0027: ports.all()"},{"line_number":208,"context_line":"        }"},{"line_number":209,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":210,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"df87a7cf_07454c15","line":207,"updated":"2017-12-15 11:08:52.000000000","message":"what is the reason to move all() from L205 here?","commit_id":"5c08f28ef009d19d1a11943cff3bb23d457c06db"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"05cc02bcdb8329efe3794d8547723fe4848b6d96","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"},{"line_number":207,"context_line":"            \u0027gw_port_id\u0027: ports.all()"},{"line_number":208,"context_line":"        }"},{"line_number":209,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":210,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"df87a7cf_cb8966ca","line":207,"in_reply_to":"df87a7cf_07454c15","updated":"2017-12-19 03:17:39.000000000","message":"Hi Slawek, because in L236 we still use \"ports\" as db model object. So we only need pass convert ports in to a list/dict for this case and leave it intact for other cases.","commit_id":"5c08f28ef009d19d1a11943cff3bb23d457c06db"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"59b257c54383b2d2802a0a1e329f39155d77fd1e","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"},{"line_number":207,"context_line":"            \u0027gw_port_id\u0027: ports.all()"},{"line_number":208,"context_line":"        }"},{"line_number":209,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":210,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bf8cb3f7_78eac9ac","line":207,"in_reply_to":"df87a7cf_cb8966ca","updated":"2017-12-23 15:47:38.000000000","message":"ok, that makes sense :)","commit_id":"5c08f28ef009d19d1a11943cff3bb23d457c06db"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"8456518829a01ddb8085ab00e04218f4272b43f6","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            if new_tenant \u003d\u003d policy[\u0027target_tenant\u0027]:"},{"line_number":201,"context_line":"                # nothing to validate if the tenant didn\u0027t change"},{"line_number":202,"context_line":"                return"},{"line_number":203,"context_line":"        ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_8781684d","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":13},"updated":"2017-12-24 16:03:58.000000000","message":"This is actually not a port list. How about renaming this to \u0027gw_ports_query\u0027?","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"b9a3c7852eed3aed18de47341d99003fb5e48ad9","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            if new_tenant \u003d\u003d policy[\u0027target_tenant\u0027]:"},{"line_number":201,"context_line":"                # nothing to validate if the tenant didn\u0027t change"},{"line_number":202,"context_line":"                return"},{"line_number":203,"context_line":"        ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_0ac6aa9b","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":13},"in_reply_to":"bf8cb3f7_8781684d","updated":"2017-12-27 08:36:00.000000000","message":"Done","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"8456518829a01ddb8085ab00e04218f4272b43f6","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"},{"line_number":207,"context_line":"            \u0027gw_port_id\u0027: ports.all()"},{"line_number":208,"context_line":"        }"},{"line_number":209,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":210,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":211,"context_line":"            filters[\u0027project_id\u0027] \u003d policy[\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_c7ec502b","line":208,"range":{"start_line":206,"start_character":8,"end_line":208,"end_character":9},"updated":"2017-12-24 16:03:58.000000000","message":"This is used only inside \"if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027\", Can you move it inside the if-clause?","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"b9a3c7852eed3aed18de47341d99003fb5e48ad9","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        filters \u003d {"},{"line_number":207,"context_line":"            \u0027gw_port_id\u0027: ports.all()"},{"line_number":208,"context_line":"        }"},{"line_number":209,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":210,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":211,"context_line":"            filters[\u0027project_id\u0027] \u003d policy[\u0027target_tenant\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_cabf62f9","line":208,"range":{"start_line":206,"start_character":8,"end_line":208,"end_character":9},"in_reply_to":"bf8cb3f7_c7ec502b","updated":"2017-12-27 08:36:00.000000000","message":"Done","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"8456518829a01ddb8085ab00e04218f4272b43f6","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":217,"context_line":"                    rbac.target_tenant \u003d\u003d \u0027*\u0027).count():"},{"line_number":218,"context_line":"                return"},{"line_number":219,"context_line":"            routers \u003d l3_obj.Router.count(context, **filters)"},{"line_number":220,"context_line":"        else:"},{"line_number":221,"context_line":"            # deleting the wildcard is okay as long as the tenants with"},{"line_number":222,"context_line":"            # attached routers have their own entries and the network is"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_0cfedfad","line":219,"range":{"start_line":219,"start_character":12,"end_line":219,"end_character":19},"updated":"2017-12-24 16:03:58.000000000","message":"num_routers is less confusing variable name. Could you update the name?","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"b9a3c7852eed3aed18de47341d99003fb5e48ad9","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":217,"context_line":"                    rbac.target_tenant \u003d\u003d \u0027*\u0027).count():"},{"line_number":218,"context_line":"                return"},{"line_number":219,"context_line":"            routers \u003d l3_obj.Router.count(context, **filters)"},{"line_number":220,"context_line":"        else:"},{"line_number":221,"context_line":"            # deleting the wildcard is okay as long as the tenants with"},{"line_number":222,"context_line":"            # attached routers have their own entries and the network is"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_eaba6608","line":219,"range":{"start_line":219,"start_character":12,"end_line":219,"end_character":19},"in_reply_to":"bf8cb3f7_0cfedfad","updated":"2017-12-27 08:36:00.000000000","message":"Done","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"7e9d59a4abfba319cc4977906bc7c3649a36d2b6","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":234,"context_line":"            router \u003d router.filter("},{"line_number":235,"context_line":"                ~l3_models.Router.tenant_id.in_(tenants_with_entries))"},{"line_number":236,"context_line":"            if new_tenant:"},{"line_number":237,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":238,"context_line":"                # interfaces that belong to the new target."},{"line_number":239,"context_line":"                router \u003d router.filter("},{"line_number":240,"context_line":"                    l3_models.Router.tenant_id !\u003d new_tenant)"},{"line_number":241,"context_line":"        if router.count():"},{"line_number":242,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":243,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_8c043819","side":"PARENT","line":240,"range":{"start_line":236,"start_character":11,"end_line":240,"end_character":61},"updated":"2018-01-03 09:52:11.000000000","message":"why can you drop this logic?","commit_id":"e3ca20fb57c30e218a23dc6ea7098cb2234d2981"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":234,"context_line":"            router \u003d router.filter("},{"line_number":235,"context_line":"                ~l3_models.Router.tenant_id.in_(tenants_with_entries))"},{"line_number":236,"context_line":"            if new_tenant:"},{"line_number":237,"context_line":"                # if this is an update we also need to ignore any router"},{"line_number":238,"context_line":"                # interfaces that belong to the new target."},{"line_number":239,"context_line":"                router \u003d router.filter("},{"line_number":240,"context_line":"                    l3_models.Router.tenant_id !\u003d new_tenant)"},{"line_number":241,"context_line":"        if router.count():"},{"line_number":242,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":243,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_83e719e5","side":"PARENT","line":240,"range":{"start_line":236,"start_character":11,"end_line":240,"end_character":61},"in_reply_to":"9f91af0f_8c043819","updated":"2018-01-03 22:08:59.000000000","message":"Yeah, I think we should just include new_tenant into tenants_with_entries that is passed into get_router_count_not_owned_by_tenants.","commit_id":"e3ca20fb57c30e218a23dc6ea7098cb2234d2981"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027])"},{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            filters \u003d {"},{"line_number":209,"context_line":"                \u0027gw_port_id\u0027: gw_ports_query.all()"},{"line_number":210,"context_line":"            }"},{"line_number":211,"context_line":"            filters[\u0027project_id\u0027] \u003d policy[\u0027target_tenant\u0027]"},{"line_number":212,"context_line":"            # if there is a wildcard entry we can safely proceed without the"},{"line_number":213,"context_line":"            # router lookup because they will have access either way"},{"line_number":214,"context_line":"            if context.session.query(rbac_db.NetworkRBAC).filter("}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_2373ad4f","line":211,"range":{"start_line":208,"start_character":12,"end_line":211,"end_character":59},"updated":"2018-01-03 22:08:59.000000000","message":"filters \u003d {\n    \u0027gw_port_id\u0027: ...,\n     \u0027project_id\u0027: ...,\n}\n\nor you may completely avoid constructing the dict and just pass arguments directly into .count():\n\nnum_routers \u003d ...count(context, gw_port_id\u003dports, project_id\u003d....)","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":217,"context_line":"                    rbac.target_tenant \u003d\u003d \u0027*\u0027).count():"},{"line_number":218,"context_line":"                return"},{"line_number":219,"context_line":"            num_routers \u003d l3_obj.Router.count(context, **filters)"},{"line_number":220,"context_line":"        else:"},{"line_number":221,"context_line":"            # deleting the wildcard is okay as long as the tenants with"},{"line_number":222,"context_line":"            # attached routers have their own entries and the network is"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_03f46985","line":219,"range":{"start_line":219,"start_character":12,"end_line":219,"end_character":24},"updated":"2018-01-03 22:08:59.000000000","message":"I don\u0027t think you care about the number of matches, as long as at least one matches. In this case you better use objects_exist API.","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":233,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":234,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":235,"context_line":"            num_routers \u003d l3_obj.Router.get_router_count_not_owned_by_tenants("},{"line_number":236,"context_line":"                context, gw_ports_query, tenants_with_entries)"},{"line_number":237,"context_line":"        if num_routers !\u003d 0:"},{"line_number":238,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_8335d948","line":235,"range":{"start_line":235,"start_character":51,"end_line":235,"end_character":58},"updated":"2018-01-03 22:08:59.000000000","message":"do we really care about the number, or the fact of existence would be enough? if so, return a boolean and rename the method into e.g. routers_not_owned_by_tenants_exist","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":234,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":235,"context_line":"            num_routers \u003d l3_obj.Router.get_router_count_not_owned_by_tenants("},{"line_number":236,"context_line":"                context, gw_ports_query, tenants_with_entries)"},{"line_number":237,"context_line":"        if num_routers !\u003d 0:"},{"line_number":238,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":239,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_a387bd0f","line":236,"range":{"start_line":236,"start_character":25,"end_line":236,"end_character":39},"updated":"2018-01-03 22:08:59.000000000","message":"please pass a list/set of ports not a query here. We should avoid passing sqlalchemy constructs across OVO API boundaries.","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2278e5b5b0d7b1e4203ef7dcb03016330410070b","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            filters \u003d {"},{"line_number":209,"context_line":"                \u0027gw_port_id\u0027: gw_ports.all(),"},{"line_number":210,"context_line":"                \u0027project_id\u0027: policy[\u0027target_tenant\u0027]"},{"line_number":211,"context_line":"            }"},{"line_number":212,"context_line":"            # if there is a wildcard entry we can safely proceed without the"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_15980dbe","line":209,"range":{"start_line":209,"start_character":38,"end_line":209,"end_character":44},"updated":"2018-01-04 14:38:11.000000000","message":"can you call .all in line 205 so that gw_ports is already a ready list of port IDs? Then you don\u0027t need to call it here and (implicitly) in the OVO method. I think it would isolate us a bit more from sqlalchemy specifics since then we won\u0027t pass a query into OVO method but a legit list of IDs.","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"282e0919785e364b6a634a12a9fd6f4e6bb142db","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":207,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"},{"line_number":208,"context_line":"            filters \u003d {"},{"line_number":209,"context_line":"                \u0027gw_port_id\u0027: gw_ports.all(),"},{"line_number":210,"context_line":"                \u0027project_id\u0027: policy[\u0027target_tenant\u0027]"},{"line_number":211,"context_line":"            }"},{"line_number":212,"context_line":"            # if there is a wildcard entry we can safely proceed without the"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_a7b0e58b","line":209,"range":{"start_line":209,"start_character":38,"end_line":209,"end_character":44},"in_reply_to":"9f91af0f_15980dbe","updated":"2018-01-08 06:12:39.000000000","message":"Done","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2278e5b5b0d7b1e4203ef7dcb03016330410070b","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":234,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":235,"context_line":"            router_exist \u003d l3_obj.Router.check_routers_not_owned_by_tenants("},{"line_number":236,"context_line":"                context, gw_ports, tenants_with_entries, new_tenant)"},{"line_number":237,"context_line":"        if router_exist:"},{"line_number":238,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":239,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_d5e62545","line":236,"range":{"start_line":236,"start_character":56,"end_line":236,"end_character":68},"updated":"2018-01-04 14:38:11.000000000","message":"do we really need to pass it separately? can\u0027t we just meld it into tenants_with_entries before passing? I think for both the tenants_with_entries and new_tenant, we seek the same behavior (\"routers that are not owned by any of those tenants\"), so it should work no?","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"282e0919785e364b6a634a12a9fd6f4e6bb142db","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":234,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027))"},{"line_number":235,"context_line":"            router_exist \u003d l3_obj.Router.check_routers_not_owned_by_tenants("},{"line_number":236,"context_line":"                context, gw_ports, tenants_with_entries, new_tenant)"},{"line_number":237,"context_line":"        if router_exist:"},{"line_number":238,"context_line":"            msg \u003d _(\"There are routers attached to this network that \""},{"line_number":239,"context_line":"                    \"depend on this policy for access.\")"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_47a9d921","line":236,"range":{"start_line":236,"start_character":56,"end_line":236,"end_character":68},"in_reply_to":"9f91af0f_d5e62545","updated":"2018-01-08 06:12:39.000000000","message":"Done","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"36b9e8542de1f126bf8436ca10aeb8d41fbdde9a","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                return"},{"line_number":203,"context_line":"        gw_ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027]).all()"},{"line_number":206,"context_line":"        gw_ports \u003d [gw_port[0] for gw_port in gw_ports]"},{"line_number":207,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":208,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_e7d65750","line":205,"range":{"start_line":205,"start_character":43,"end_line":205,"end_character":49},"updated":"2018-01-11 14:14:26.000000000","message":"I think you don\u0027t need all() if you later iterate","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"36b9e8542de1f126bf8436ca10aeb8d41fbdde9a","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                context.session.query(rbac.target_tenant)."},{"line_number":233,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":234,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":235,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027)).all()"},{"line_number":236,"context_line":"            tenants_with_entries \u003d [tenants_with_entry[0]"},{"line_number":237,"context_line":"                                    for tenants_with_entry"},{"line_number":238,"context_line":"                                    in tenants_with_entries]"}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_87cbd3a3","line":235,"range":{"start_line":235,"start_character":45,"end_line":235,"end_character":51},"updated":"2018-01-11 14:14:26.000000000","message":"ditto","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3ccf06f48b6d3203d25469598a70da2092d9ad04","unresolved":false,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    @registry.receives(\u0027rbac-policy\u0027, (events.BEFORE_UPDATE,"},{"line_number":190,"context_line":"                                       events.BEFORE_DELETE))"},{"line_number":191,"context_line":"    def _validate_ext_not_in_use_by_tenant(self, resource, event, trigger,"},{"line_number":192,"context_line":"                                           context, object_type, policy,"},{"line_number":193,"context_line":"                                           **kwargs):"},{"line_number":194,"context_line":"        if (object_type !\u003d \u0027network\u0027 or"}],"source_content_type":"text/x-python","patch_set":26,"id":"5f93b717_70a9b1dd","line":191,"updated":"2018-01-24 23:28:00.000000000","message":"I see you added some test for object change, but I don\u0027t see any tests for this method. I would like to see them added before we consider merging this patch, since history showed that obvious issues with the patch were not revealed by gate, so it\u0027s concerning.\n\nIdeally you will add those tests as a separate parent patch so that we can split concerns and review tests and this code change separately on their own merit.","commit_id":"2571297dcfdd36158b66aa7789fc1d32f1897bed"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3ccf06f48b6d3203d25469598a70da2092d9ad04","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                return"},{"line_number":203,"context_line":"        gw_ports \u003d context.session.query(models_v2.Port.id).filter_by("},{"line_number":204,"context_line":"            device_owner\u003dDEVICE_OWNER_ROUTER_GW,"},{"line_number":205,"context_line":"            network_id\u003dpolicy[\u0027object_id\u0027]).all()"},{"line_number":206,"context_line":"        gw_ports \u003d [gw_port[0] for gw_port in gw_ports]"},{"line_number":207,"context_line":"        rbac \u003d rbac_db.NetworkRBAC"},{"line_number":208,"context_line":"        if policy[\u0027target_tenant\u0027] !\u003d \u0027*\u0027:"}],"source_content_type":"text/x-python","patch_set":26,"id":"5f93b717_10970d93","line":205,"range":{"start_line":205,"start_character":43,"end_line":205,"end_character":49},"updated":"2018-01-24 23:28:00.000000000","message":"I was suggesting it before, so repeating here: since you iterate over gw_ports in line 206, this .all() call is not needed, you can allow __iter__ to iterate through the query results","commit_id":"2571297dcfdd36158b66aa7789fc1d32f1897bed"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3ccf06f48b6d3203d25469598a70da2092d9ad04","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                context.session.query(rbac.target_tenant)."},{"line_number":233,"context_line":"                filter(rbac.object_id \u003d\u003d policy[\u0027object_id\u0027],"},{"line_number":234,"context_line":"                       rbac.action \u003d\u003d \u0027access_as_external\u0027,"},{"line_number":235,"context_line":"                       rbac.target_tenant !\u003d \u0027*\u0027)).all()"},{"line_number":236,"context_line":"            projects_with_entries \u003d [projects_with_entry[0]"},{"line_number":237,"context_line":"                                     for projects_with_entry"},{"line_number":238,"context_line":"                                     in projects_with_entries]"}],"source_content_type":"text/x-python","patch_set":26,"id":"5f93b717_b0edf926","line":235,"range":{"start_line":235,"start_character":50,"end_line":235,"end_character":56},"updated":"2018-01-24 23:28:00.000000000","message":"same here, I don\u0027t think .all() is needed, it should work without it too.","commit_id":"2571297dcfdd36158b66aa7789fc1d32f1897bed"}],"neutron/objects/router.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"feb10d305edd9702557bb5acf23cc2f8ebd89f56","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    @classmethod"},{"line_number":210,"context_line":"    def get_router_filter_target_tenant(cls, context, ports, target_tenant):"},{"line_number":211,"context_line":"        router \u003d context.session.query(l3.Router).filter("},{"line_number":212,"context_line":"            l3.Router.gw_port_id.in_(ports))"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        router \u003d router.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_3ad20336","line":211,"range":{"start_line":211,"start_character":8,"end_line":211,"end_character":14},"updated":"2017-12-14 13:30:13.000000000","message":"I would personally name it \"query\" as it is in many other places","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"94617e54e1d1fde621e06957cddab461a738978f","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    @classmethod"},{"line_number":210,"context_line":"    def get_router_filter_target_tenant(cls, context, ports, target_tenant):"},{"line_number":211,"context_line":"        router \u003d context.session.query(l3.Router).filter("},{"line_number":212,"context_line":"            l3.Router.gw_port_id.in_(ports))"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        router \u003d router.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_e5f4fcd1","line":211,"range":{"start_line":211,"start_character":8,"end_line":211,"end_character":14},"in_reply_to":"df87a7cf_3ad20336","updated":"2017-12-14 14:12:15.000000000","message":"Those methods are not needed; and yes, we shouldn\u0027t return queries either way.","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"935122ed80dd55b1f59dbdb0292ad8967f90cbbe","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    @classmethod"},{"line_number":210,"context_line":"    def get_router_filter_target_tenant(cls, context, ports, target_tenant):"},{"line_number":211,"context_line":"        router \u003d context.session.query(l3.Router).filter("},{"line_number":212,"context_line":"            l3.Router.gw_port_id.in_(ports))"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        router \u003d router.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_162105fc","line":211,"range":{"start_line":211,"start_character":8,"end_line":211,"end_character":14},"in_reply_to":"df87a7cf_e5f4fcd1","updated":"2017-12-15 08:32:11.000000000","message":"Agreed. Done","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"feb10d305edd9702557bb5acf23cc2f8ebd89f56","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        router \u003d router.filter("},{"line_number":215,"context_line":"            l3.Router.tenant_id \u003d\u003d target_tenant)"},{"line_number":216,"context_line":"        return router"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @classmethod"},{"line_number":219,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_dae2f701","line":216,"updated":"2017-12-14 13:30:13.000000000","message":"You returns here db model objects. Shouldn\u0027t You create OVO objects for each such router and returns list of OVO objects?","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"935122ed80dd55b1f59dbdb0292ad8967f90cbbe","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        router \u003d router.filter("},{"line_number":215,"context_line":"            l3.Router.tenant_id \u003d\u003d target_tenant)"},{"line_number":216,"context_line":"        return router"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @classmethod"},{"line_number":219,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_d6787df9","line":216,"in_reply_to":"df87a7cf_dae2f701","updated":"2017-12-15 08:32:11.000000000","message":"Done","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d5df6000b224618c642fc68d3ad1c1e3e14cee9c","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return router"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @classmethod"},{"line_number":219,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"},{"line_number":220,"context_line":"                                               tenants_with_entries,"},{"line_number":221,"context_line":"                                               new_tenant):"},{"line_number":222,"context_line":"        router \u003d context.session.query(l3.Router).filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_a5a7d44f","line":219,"updated":"2017-12-14 14:30:39.000000000","message":"OK, maybe this method is needed, though it shouldn\u0027t return a query. Seems like a number should be enough for consumer. Or list of OVO routers.","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"935122ed80dd55b1f59dbdb0292ad8967f90cbbe","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return router"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @classmethod"},{"line_number":219,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"},{"line_number":220,"context_line":"                                               tenants_with_entries,"},{"line_number":221,"context_line":"                                               new_tenant):"},{"line_number":222,"context_line":"        router \u003d context.session.query(l3.Router).filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_566c6d35","line":219,"in_reply_to":"df87a7cf_a5a7d44f","updated":"2017-12-15 08:32:11.000000000","message":"Thanks for your comment, Ihar. I\u0027ve implemented return as your suggestion.","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"feb10d305edd9702557bb5acf23cc2f8ebd89f56","unresolved":false,"context_lines":[{"line_number":219,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"},{"line_number":220,"context_line":"                                               tenants_with_entries,"},{"line_number":221,"context_line":"                                               new_tenant):"},{"line_number":222,"context_line":"        router \u003d context.session.query(l3.Router).filter("},{"line_number":223,"context_line":"            l3.Router.gw_port_id.in_(ports))"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        router \u003d router.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_7aef2be9","line":222,"range":{"start_line":222,"start_character":8,"end_line":222,"end_character":14},"updated":"2017-12-14 13:30:13.000000000","message":"maybe \"query\"?","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"feb10d305edd9702557bb5acf23cc2f8ebd89f56","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        if new_tenant:"},{"line_number":228,"context_line":"            router \u003d router.filter("},{"line_number":229,"context_line":"                l3.Router.tenant_id !\u003d new_tenant)"},{"line_number":230,"context_line":"        return router"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_9aec6ff4","line":230,"updated":"2017-12-14 13:30:13.000000000","message":"You returns here db model objects. Shouldn\u0027t You create OVO objects for each such router and returns list of OVO objects?","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"935122ed80dd55b1f59dbdb0292ad8967f90cbbe","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        if new_tenant:"},{"line_number":228,"context_line":"            router \u003d router.filter("},{"line_number":229,"context_line":"                l3.Router.tenant_id !\u003d new_tenant)"},{"line_number":230,"context_line":"        return router"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_361c49c0","line":230,"in_reply_to":"df87a7cf_9aec6ff4","updated":"2017-12-15 08:32:11.000000000","message":"Done","commit_id":"df5a8713d20bd092b9ae302aa9485ff4429b7e54"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"58965f512687d1067ec23d9af938f298f025fa2d","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    # TODO(hungpv) We may want to implement NOT semantics in get_objects(s)"},{"line_number":212,"context_line":"    @classmethod"},{"line_number":213,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"},{"line_number":214,"context_line":"                                               tenants_with_entries,"}],"source_content_type":"text/x-python","patch_set":11,"id":"df87a7cf_0c4a5254","line":211,"updated":"2017-12-15 11:08:52.000000000","message":"nit: I would personally move this comment to inside method, so L216","commit_id":"896855c4795cf79bf30437ac8fb0430acdd4f014"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"58965f512687d1067ec23d9af938f298f025fa2d","unresolved":false,"context_lines":[{"line_number":212,"context_line":"    @classmethod"},{"line_number":213,"context_line":"    def get_router_filter_tenants_with_entries(cls, context, ports,"},{"line_number":214,"context_line":"                                               tenants_with_entries,"},{"line_number":215,"context_line":"                                               new_tenant):"},{"line_number":216,"context_line":"        query \u003d context.session.query(l3.Router).filter("},{"line_number":217,"context_line":"            l3.Router.gw_port_id.in_(ports))"},{"line_number":218,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"df87a7cf_4cae1ae8","line":215,"updated":"2017-12-15 11:08:52.000000000","message":"maybe it\u0027s only me but I don\u0027t know exactly by reading name of method what it will return. Can You maybe try to rephrase it somehow or at least add some comment to explain it","commit_id":"896855c4795cf79bf30437ac8fb0430acdd4f014"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"8456518829a01ddb8085ab00e04218f4272b43f6","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def get_router_not_count_by_tenants(cls, context, ports,"},{"line_number":213,"context_line":"                                        tenants_with_entries,"},{"line_number":214,"context_line":"                                        new_tenant):"},{"line_number":215,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_objects(s)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf8cb3f7_47aa80d2","line":212,"range":{"start_line":212,"start_character":8,"end_line":212,"end_character":39},"updated":"2017-12-24 16:03:58.000000000","message":"The method name sounds unclear. In addition, could you add a docstring?\n\nThis method returns router_count, so at least \"get_router_count_.....\" is a better name.","commit_id":"cd8080ce9897c5a6c7a9bdd102cdf84df8455864"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5977247015b11a8364330f630ad8f3a1385bacca","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def get_router_count_not_by_tenant_entries(cls, context, ports,"},{"line_number":213,"context_line":"                                               tenants_with_entries,"},{"line_number":214,"context_line":"                                               new_tenant):"},{"line_number":215,"context_line":"        \"\"\"This method returns the count of routers of tenants which don\u0027t have"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_baa38e4f","line":212,"range":{"start_line":212,"start_character":8,"end_line":212,"end_character":46},"updated":"2017-12-30 17:04:37.000000000","message":"I am still confused with the name of the method.\nThis method counts the number of routers which do not belong to \"tenants_with_entries\" and \"new_tenant\".\n\nThe naming of \"tenants_entries\" makes sense in external_net_db.py because it comes from RBAC entries.\nHowever, this is now a part of router object. From this point of view, \"tenant_entries\" sounds less sense to me.\n\nHow about \"get_router_count_not_owned_by_tenants\"?\n\n(I don\u0027t think it is a best method name, but I think it is better than now)","commit_id":"878ff5c521508adc27da81cece5dbb3b1a712f8c"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5977247015b11a8364330f630ad8f3a1385bacca","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def get_router_count_not_by_tenant_entries(cls, context, ports,"},{"line_number":213,"context_line":"                                               tenants_with_entries,"},{"line_number":214,"context_line":"                                               new_tenant):"},{"line_number":215,"context_line":"        \"\"\"This method returns the count of routers of tenants which don\u0027t have"},{"line_number":216,"context_line":"          their own entries"},{"line_number":217,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_5a8842c5","line":214,"range":{"start_line":213,"start_character":47,"end_line":214,"end_character":57},"updated":"2017-12-30 17:04:37.000000000","message":"Looking at the query filter below, this method count the number of routers which do not belong to tenants_with_entries and new_tenant. \n\nWhy do we need to have two arguments? Is there any reason you cannot merge these two arguments into one?\n\nThe only difference I see is \"tenant_with_entries\" can be sqlalchemy query and \"new_tenant\" cannot, but I am not sure this is a valid reason to have two similar arguments.\n\nIn addition, \"teannts_with_entries\" and \"new_tenant\" do not sound meaningful names as method arguments. They were meaningful in external_net_db.py but I don\u0027t think they are meaningful names here.","commit_id":"878ff5c521508adc27da81cece5dbb3b1a712f8c"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def get_router_count_not_owned_by_tenants(cls, context, ports,"},{"line_number":213,"context_line":"                                              tenants_owned):"},{"line_number":214,"context_line":"        \"\"\"This method counts the number of routers which do not belong to"},{"line_number":215,"context_line":"        tenants"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_231dedbc","line":212,"range":{"start_line":212,"start_character":60,"end_line":212,"end_character":65},"updated":"2018-01-03 22:08:59.000000000","message":"THIS argument, on contrast, may need some explanation. Either somehow change the name of the argument [gw_ports?] so that it\u0027s self explanatory as to its goal, or explain what it means in docstring.","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def get_router_count_not_owned_by_tenants(cls, context, ports,"},{"line_number":213,"context_line":"                                              tenants_owned):"},{"line_number":214,"context_line":"        \"\"\"This method counts the number of routers which do not belong to"},{"line_number":215,"context_line":"        tenants"},{"line_number":216,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_830e398b","line":213,"range":{"start_line":213,"start_character":46,"end_line":213,"end_character":57},"updated":"2018-01-03 22:08:59.000000000","message":"rename into \u0027tenants\u0027 since the method name already alludes to the meaning of the argument","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def get_router_count_not_owned_by_tenants(cls, context, ports,"},{"line_number":213,"context_line":"                                              tenants_owned):"},{"line_number":214,"context_line":"        \"\"\"This method counts the number of routers which do not belong to"},{"line_number":215,"context_line":"        tenants"},{"line_number":216,"context_line":"        \"\"\""},{"line_number":217,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_objects(s)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_0349c9ce","line":214,"updated":"2018-01-03 22:08:59.000000000","message":"...but if we don\u0027t really care about the number but existence only then the method and the docstring will need a refinement.","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        \"\"\"This method counts the number of routers which do not belong to"},{"line_number":215,"context_line":"        tenants"},{"line_number":216,"context_line":"        \"\"\""},{"line_number":217,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_objects(s)"},{"line_number":218,"context_line":"        query \u003d context.session.query(l3.Router).filter("},{"line_number":219,"context_line":"            l3.Router.gw_port_id.in_(ports))"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_c379a1fc","line":217,"range":{"start_line":217,"start_character":75,"end_line":217,"end_character":76},"updated":"2018-01-03 22:08:59.000000000","message":"spurious \"s\"","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"7e9d59a4abfba319cc4977906bc7c3649a36d2b6","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        query \u003d query.filter("},{"line_number":222,"context_line":"            ~l3.Router.tenant_id.in_(tenants_owned))"},{"line_number":223,"context_line":"        if tenants_owned:"},{"line_number":224,"context_line":"            query \u003d query.filter("},{"line_number":225,"context_line":"                l3.Router.tenant_id !\u003d tenants_owned)"},{"line_number":226,"context_line":"        return query.count()"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_4f04e215","line":225,"range":{"start_line":223,"start_character":8,"end_line":225,"end_character":53},"updated":"2018-01-03 09:52:11.000000000","message":"tenants_owned is used both in L.222 and L.225. What does this mean?","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1931b053e37a3a41b43ebac652eaf889e88d22ec","unresolved":false,"context_lines":[{"line_number":223,"context_line":"        if tenants_owned:"},{"line_number":224,"context_line":"            query \u003d query.filter("},{"line_number":225,"context_line":"                l3.Router.tenant_id !\u003d tenants_owned)"},{"line_number":226,"context_line":"        return query.count()"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_6368d520","line":226,"updated":"2018-01-03 22:08:59.000000000","message":"since the caller probably doesn\u0027t care about the number, return bool(query.count())","commit_id":"3078a083f33ee7dc4475ce3809cfd1ad866e8787"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2278e5b5b0d7b1e4203ef7dcb03016330410070b","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports,"},{"line_number":213,"context_line":"                                           tenants, new_tenant):"},{"line_number":214,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenant"},{"line_number":215,"context_line":"        exist or not"},{"line_number":216,"context_line":"        \"\"\""},{"line_number":217,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_object(s)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_95e97d1b","line":214,"range":{"start_line":214,"start_character":72,"end_line":214,"end_character":78},"updated":"2018-01-04 14:38:11.000000000","message":"tenants?","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"282e0919785e364b6a634a12a9fd6f4e6bb142db","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports,"},{"line_number":213,"context_line":"                                           tenants, new_tenant):"},{"line_number":214,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenant"},{"line_number":215,"context_line":"        exist or not"},{"line_number":216,"context_line":"        \"\"\""},{"line_number":217,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_object(s)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_276a95ed","line":214,"range":{"start_line":214,"start_character":72,"end_line":214,"end_character":78},"in_reply_to":"9f91af0f_95e97d1b","updated":"2018-01-08 06:12:39.000000000","message":"Done","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2278e5b5b0d7b1e4203ef7dcb03016330410070b","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        query \u003d query.filter("},{"line_number":222,"context_line":"            ~l3.Router.tenant_id.in_(tenants))"},{"line_number":223,"context_line":"        if new_tenant:"},{"line_number":224,"context_line":"            query \u003d query.filter("},{"line_number":225,"context_line":"                l3.Router.tenant_id !\u003d new_tenant)"},{"line_number":226,"context_line":"        return bool(query.count())"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_b5d5a145","line":225,"range":{"start_line":223,"start_character":8,"end_line":225,"end_character":50},"updated":"2018-01-04 14:38:11.000000000","message":"I am not clear why we need to handle it separately and not just append it to \u0027tenants\u0027 since !\u003d is same as ~, right?","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"282e0919785e364b6a634a12a9fd6f4e6bb142db","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        query \u003d query.filter("},{"line_number":222,"context_line":"            ~l3.Router.tenant_id.in_(tenants))"},{"line_number":223,"context_line":"        if new_tenant:"},{"line_number":224,"context_line":"            query \u003d query.filter("},{"line_number":225,"context_line":"                l3.Router.tenant_id !\u003d new_tenant)"},{"line_number":226,"context_line":"        return bool(query.count())"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_877841c4","line":225,"range":{"start_line":223,"start_character":8,"end_line":225,"end_character":50},"in_reply_to":"9f91af0f_b5d5a145","updated":"2018-01-08 06:12:39.000000000","message":"Agreed. Please check the newest patchset and help me review it.","commit_id":"76b0fc0f70dd32fff1744075804884e9383eabc5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b8eb38200fbd04ecb529076c7d2bd97dba2d6c7","unresolved":false,"context_lines":[{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        for router in routers:"},{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router[\u0027project_id\u0027]:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9f91af0f_996082d1","line":224,"range":{"start_line":224,"start_character":29,"end_line":224,"end_character":49},"updated":"2018-01-09 08:51:58.000000000","message":"nit: I know that it works like that but maybe better would be to access object attributes as objects: so use router.project_id. What You think about that?","commit_id":"3bebc57ab5fed4e817573af57992d45824986b2d"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"d35701244882d1ffce891346eb3ba3110b00f534","unresolved":false,"context_lines":[{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        for router in routers:"},{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router[\u0027project_id\u0027]:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f96bb07_61823acb","line":224,"range":{"start_line":224,"start_character":29,"end_line":224,"end_character":49},"in_reply_to":"9f91af0f_996082d1","updated":"2018-01-11 11:27:08.000000000","message":"Thanks for your comment, I agree. That\u0027d avoid unexpected result.","commit_id":"3bebc57ab5fed4e817573af57992d45824986b2d"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b8eb38200fbd04ecb529076c7d2bd97dba2d6c7","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports, tenants):"},{"line_number":213,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenants"},{"line_number":214,"context_line":"        exist or not"},{"line_number":215,"context_line":"        \"\"\""},{"line_number":216,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_object(s)"},{"line_number":217,"context_line":"        filters \u003d {"},{"line_number":218,"context_line":"            \u0027gw_port_id\u0027: gw_ports,"},{"line_number":219,"context_line":"        }"},{"line_number":220,"context_line":"        routers \u003d Router.get_objects(context, **filters)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        for router in routers:"},{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router[\u0027project_id\u0027]:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f91af0f_b95d4616","line":226,"range":{"start_line":212,"start_character":0,"end_line":226,"end_character":48},"updated":"2018-01-09 08:51:58.000000000","message":"I think that simple UT for this method should be added","commit_id":"3bebc57ab5fed4e817573af57992d45824986b2d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ece67007dad4dab998a5c77a926256ee17292552","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports, tenants):"},{"line_number":213,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenants"},{"line_number":214,"context_line":"        exist or not"},{"line_number":215,"context_line":"        \"\"\""},{"line_number":216,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_object(s)"},{"line_number":217,"context_line":"        filters \u003d {"},{"line_number":218,"context_line":"            \u0027gw_port_id\u0027: gw_ports,"},{"line_number":219,"context_line":"        }"},{"line_number":220,"context_line":"        routers \u003d Router.get_objects(context, **filters)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        for router in routers:"},{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router[\u0027project_id\u0027]:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f96bb07_27bc3f04","line":226,"range":{"start_line":212,"start_character":0,"end_line":226,"end_character":48},"in_reply_to":"7f96bb07_f335c1f8","updated":"2018-01-11 14:15:22.000000000","message":"I actually think it\u0027s urgent since the code is clearly buggy.","commit_id":"3bebc57ab5fed4e817573af57992d45824986b2d"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"d35701244882d1ffce891346eb3ba3110b00f534","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports, tenants):"},{"line_number":213,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenants"},{"line_number":214,"context_line":"        exist or not"},{"line_number":215,"context_line":"        \"\"\""},{"line_number":216,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_object(s)"},{"line_number":217,"context_line":"        filters \u003d {"},{"line_number":218,"context_line":"            \u0027gw_port_id\u0027: gw_ports,"},{"line_number":219,"context_line":"        }"},{"line_number":220,"context_line":"        routers \u003d Router.get_objects(context, **filters)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        for router in routers:"},{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router[\u0027project_id\u0027]:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f96bb07_f335c1f8","line":226,"range":{"start_line":212,"start_character":0,"end_line":226,"end_character":48},"in_reply_to":"9f91af0f_b95d4616","updated":"2018-01-11 11:27:08.000000000","message":"I think it\u0027s no urgent right now. I\u0027ll add it after finish l3_db and remaining files for router.","commit_id":"3bebc57ab5fed4e817573af57992d45824986b2d"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"b8fca0c547208b51e0ec68888f7375afade74f30","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        \u0027flavor_id\u0027: common_types.UUIDField(nullable\u003dTrue),"},{"line_number":207,"context_line":"    }"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports, tenants):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_070263b5","line":209,"range":{"start_line":209,"start_character":38,"end_line":209,"end_character":50},"updated":"2018-01-11 14:22:10.000000000","message":"why is \u0027gw_port_id\u0027 not updatable?","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"4c7ca3c802552e0343b7c4224a1a2453e8cbe344","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    fields_no_update \u003d [\u0027project_id\u0027, \u0027gw_port_id\u0027]"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports, tenants):"},{"line_number":213,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenants"},{"line_number":214,"context_line":"        exist or not"},{"line_number":215,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_e732d7a7","line":212,"range":{"start_line":212,"start_character":35,"end_line":212,"end_character":42},"updated":"2018-01-11 14:22:32.000000000","message":"Let\u0027s use new terminology here: projects not tenants (also the argument name)","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"4c7ca3c802552e0343b7c4224a1a2453e8cbe344","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @classmethod"},{"line_number":212,"context_line":"    def check_routers_not_owned_by_tenants(cls, context, gw_ports, tenants):"},{"line_number":213,"context_line":"        \"\"\"This method is to check whether routers that aren\u0027t owned by tenants"},{"line_number":214,"context_line":"        exist or not"},{"line_number":215,"context_line":"        \"\"\""},{"line_number":216,"context_line":"        # TODO(hungpv) We may want to implement NOT semantics in get_object(s)"}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_a72ccf47","line":213,"range":{"start_line":213,"start_character":71,"end_line":213,"end_character":79},"updated":"2018-01-11 14:22:32.000000000","message":"-\u003e projects","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"fecf52efe1366c09562e6396baa40fe662d22fd3","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        }"},{"line_number":220,"context_line":"        routers \u003d Router.get_objects(context, **filters)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        for router in routers:"},{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router.project_id:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_278e7f78","line":224,"range":{"start_line":222,"start_character":8,"end_line":224,"end_character":47},"updated":"2018-01-11 14:13:57.000000000","message":"I don\u0027t think we should filter in python. The previous version with query was better in that all work was done in db.","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"fecf52efe1366c09562e6396baa40fe662d22fd3","unresolved":false,"context_lines":[{"line_number":223,"context_line":"            for tenant in tenants:"},{"line_number":224,"context_line":"                if tenant !\u003d router.project_id:"},{"line_number":225,"context_line":"                    return True"},{"line_number":226,"context_line":"        return bool(len(routers) - len(tenants))"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"@base.NeutronObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":21,"id":"7f96bb07_67d9e781","line":226,"range":{"start_line":226,"start_character":15,"end_line":226,"end_character":48},"updated":"2018-01-11 14:13:57.000000000","message":"this is something totally unclear: so if there are no routers, you get negative value meaning True? I think the previous version was mostly fine.","commit_id":"34af57a3f81e47d96a0eb6ae2b731c32b3b32f6b"}],"neutron/tests/unit/objects/test_router.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"f7033194451a43796cce088501c96596c2706cac","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        project_no_match \u003d uuidutils.generate_uuid()"},{"line_number":89,"context_line":"        new_project_no_match \u003d uuidutils.generate_uuid()"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Check router match with gw_port BUT projects"},{"line_number":92,"context_line":"        router_exist \u003d router.Router.check_routers_not_owned_by_projects("},{"line_number":93,"context_line":"            self.context,"},{"line_number":94,"context_line":"            [gw_port],"}],"source_content_type":"text/x-python","patch_set":25,"id":"7f96bb07_f07cb373","line":91,"range":{"start_line":91,"start_character":42,"end_line":91,"end_character":54},"updated":"2018-01-22 15:15:41.000000000","message":"shouldn\u0027t be \"BUT no projects\"?\nI know that it is nitty nit but IMHO it is worth to make it clear now to not confuse someone in the future :)","commit_id":"9f4b2d47009219bb627cf063e1a075897f0854f6"},{"author":{"_account_id":26072,"name":"Van Hung Pham","email":"vanhung.pham@outlook.com","username":"hungpv"},"change_message_id":"ab6dbd97d1264bf5163c80bf376505761d0f21ee","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        project_no_match \u003d uuidutils.generate_uuid()"},{"line_number":89,"context_line":"        new_project_no_match \u003d uuidutils.generate_uuid()"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Check router match with gw_port BUT projects"},{"line_number":92,"context_line":"        router_exist \u003d router.Router.check_routers_not_owned_by_projects("},{"line_number":93,"context_line":"            self.context,"},{"line_number":94,"context_line":"            [gw_port],"}],"source_content_type":"text/x-python","patch_set":25,"id":"7f96bb07_8c9e6805","line":91,"range":{"start_line":91,"start_character":42,"end_line":91,"end_character":54},"in_reply_to":"7f96bb07_f07cb373","updated":"2018-01-23 02:05:49.000000000","message":"Done","commit_id":"9f4b2d47009219bb627cf063e1a075897f0854f6"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"52ed7b99597b5d1640b084659ceba5b4eaae2573","unresolved":false,"context_lines":[{"line_number":15,"context_line":"from neutron.objects import router"},{"line_number":16,"context_line":"from neutron.tests.unit.objects import test_base as obj_test_base"},{"line_number":17,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":18,"context_line":"from oslo_utils import uuidutils"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class RouterRouteIfaceObjectTestCase("}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa0c359_a24b1329","line":18,"updated":"2018-02-08 04:10:42.000000000","message":"oslo_utils is third-party imports. This needs to be placed before neutron imports.","commit_id":"5486066acff685902437fc60931489a7089d0853"}]}