)]}'
{"neutron/db/db_base_plugin_common.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e539b87af6b351472327dd688449d14bc59b3b78","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            res[\"fixed_ips\"] \u003d ["},{"line_number":220,"context_line":"                {\u0027subnet_id\u0027: ip[\"subnet_id\"],"},{"line_number":221,"context_line":"                 \u0027ip_address\u0027: str("},{"line_number":222,"context_line":"                     ip[\"ip_address\"])} for ip in port[\"fixed_ips\"]]"},{"line_number":223,"context_line":"        # Call auxiliary extend functions, if any"},{"line_number":224,"context_line":"        if process_extensions:"},{"line_number":225,"context_line":"            port_data \u003d port"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_2197c0ee","line":222,"updated":"2019-08-02 12:37:06.000000000","message":"how big improvement it brings? Is it really necessary?","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"275fa64518fdb5b317f35f5ccfadd0c391f099e9","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            res[\"fixed_ips\"] \u003d ["},{"line_number":220,"context_line":"                {\u0027subnet_id\u0027: ip[\"subnet_id\"],"},{"line_number":221,"context_line":"                 \u0027ip_address\u0027: str("},{"line_number":222,"context_line":"                     ip[\"ip_address\"])} for ip in port[\"fixed_ips\"]]"},{"line_number":223,"context_line":"        # Call auxiliary extend functions, if any"},{"line_number":224,"context_line":"        if process_extensions:"},{"line_number":225,"context_line":"            port_data \u003d port"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_3b16cb57","line":222,"in_reply_to":"7faddb67_2197c0ee","updated":"2019-08-07 13:53:29.000000000","message":"There will be a lazy load from IPAllocation table, and also there will be an order by for this ip_address when scan the entire table. It is really insufficient.","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"}],"neutron/db/dvr_mac_db.py":[{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"e9da485ba66e326f9742498e46e192d416f28d1b","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":157,"context_line":"        query \u003d query.filter("},{"line_number":158,"context_line":"            models_v2.Port.id \u003d\u003d ml2_models.PortBinding.port_id,"},{"line_number":159,"context_line":"            models_v2.Port.id \u003d\u003d models_v2.IPAllocation.port_id,"},{"line_number":160,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host,"},{"line_number":161,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet)"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_64a16df8","line":159,"updated":"2019-07-31 14:31:35.000000000","message":"IPAllocation.port_id is neither a primary key nor an indexed field, and I don\u0027t see any foreign key relationship between it and Port.id.  I think we can use some strategies here to optimize querying based on this relationship.","commit_id":"f34b4281cdf214482842fba4ab73e54f20aac170"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"2948dab453ede4bb99e8cb7adaab0e0a656c527b","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":157,"context_line":"        query \u003d query.filter("},{"line_number":158,"context_line":"            models_v2.Port.id \u003d\u003d ml2_models.PortBinding.port_id,"},{"line_number":159,"context_line":"            models_v2.Port.id \u003d\u003d models_v2.IPAllocation.port_id,"},{"line_number":160,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host,"},{"line_number":161,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet)"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9aede4b8","line":159,"in_reply_to":"7faddb67_64a16df8","updated":"2019-07-31 16:04:36.000000000","message":"It looks to me like there is a foreign key relationship between Port.id and IPAllocation.port_id:\n\nhttps://github.com/openstack/neutron/blob/master/neutron/db/models_v2.py#L49\n\nI don\u0027t see an issue with this.","commit_id":"f34b4281cdf214482842fba4ab73e54f20aac170"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"cf550d4d250547e073046057732c8c511ff223bb","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":157,"context_line":"        query \u003d query.filter("},{"line_number":158,"context_line":"            models_v2.Port.id \u003d\u003d ml2_models.PortBinding.port_id,"},{"line_number":159,"context_line":"            models_v2.Port.id \u003d\u003d models_v2.IPAllocation.port_id,"},{"line_number":160,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host,"},{"line_number":161,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet)"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9405b0c4","line":159,"in_reply_to":"7faddb67_9aede4b8","updated":"2019-08-01 00:38:41.000000000","message":"Index is good. But any DB change is not backportable for stable branches.","commit_id":"f34b4281cdf214482842fba4ab73e54f20aac170"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"e9da485ba66e326f9742498e46e192d416f28d1b","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host,"},{"line_number":161,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        owner_filter \u003d or_("},{"line_number":164,"context_line":"            models_v2.Port.device_owner.startswith("},{"line_number":165,"context_line":"                constants.DEVICE_OWNER_COMPUTE_PREFIX),"},{"line_number":166,"context_line":"            models_v2.Port.device_owner.in_("},{"line_number":167,"context_line":"                utils.get_other_dvr_serviced_device_owners()))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        ports_query \u003d query.filter(owner_filter)"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_041f595b","line":167,"range":{"start_line":163,"start_character":0,"end_line":167,"end_character":62},"updated":"2019-07-31 14:31:35.000000000","message":"There is an index on the combination of the device owner and the network_id, but I don\u0027t see an index specific to just the device_owner field.  I wonder if an index there would help this comparison.","commit_id":"f34b4281cdf214482842fba4ab73e54f20aac170"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"2948dab453ede4bb99e8cb7adaab0e0a656c527b","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host,"},{"line_number":161,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        owner_filter \u003d or_("},{"line_number":164,"context_line":"            models_v2.Port.device_owner.startswith("},{"line_number":165,"context_line":"                constants.DEVICE_OWNER_COMPUTE_PREFIX),"},{"line_number":166,"context_line":"            models_v2.Port.device_owner.in_("},{"line_number":167,"context_line":"                utils.get_other_dvr_serviced_device_owners()))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        ports_query \u003d query.filter(owner_filter)"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9a42c4aa","line":167,"range":{"start_line":163,"start_character":0,"end_line":167,"end_character":62},"in_reply_to":"7faddb67_041f595b","updated":"2019-07-31 16:04:36.000000000","message":"That\u0027s a good point, we do execute a number of queries that filter on device_owner. I would not be surprised to see that indexing that field helps us out in a number of different places.","commit_id":"f34b4281cdf214482842fba4ab73e54f20aac170"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"cf550d4d250547e073046057732c8c511ff223bb","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host,"},{"line_number":161,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        owner_filter \u003d or_("},{"line_number":164,"context_line":"            models_v2.Port.device_owner.startswith("},{"line_number":165,"context_line":"                constants.DEVICE_OWNER_COMPUTE_PREFIX),"},{"line_number":166,"context_line":"            models_v2.Port.device_owner.in_("},{"line_number":167,"context_line":"                utils.get_other_dvr_serviced_device_owners()))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        ports_query \u003d query.filter(owner_filter)"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_b40a2ccf","line":167,"range":{"start_line":163,"start_character":0,"end_line":167,"end_character":62},"in_reply_to":"7faddb67_9a42c4aa","updated":"2019-08-01 00:38:41.000000000","message":"Index is good. But any DB change is not backportable for stable branches.","commit_id":"f34b4281cdf214482842fba4ab73e54f20aac170"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e539b87af6b351472327dd688449d14bc59b3b78","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":180,"context_line":"        return ports"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def _get_gateway_ports_query(self, context, subnet, ip_addresses):"},{"line_number":183,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":184,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":185,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_c1002c37","line":182,"range":{"start_line":182,"start_character":13,"end_line":182,"end_character":20},"updated":"2019-08-02 12:37:06.000000000","message":"how this method is going to return always gateway ports? You don\u0027t do any filtering on device owner or something like that. This method currently is returning all ports which belongs to given subnet and with given fixed ip addresses.","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a29f827ee473099727ccb9e5f28e830caf495f59","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":180,"context_line":"        return ports"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def _get_gateway_ports_query(self, context, subnet, ip_addresses):"},{"line_number":183,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":184,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":185,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_32863068","line":182,"range":{"start_line":182,"start_character":13,"end_line":182,"end_character":20},"in_reply_to":"7faddb67_c1002c37","updated":"2019-08-02 15:12:23.000000000","message":"\u0027ip_addresses\u0027 are gateway IPs line 205-209. So it should be the gateway ports.","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"059453dff0e183d5946922b286a1756401706303","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":180,"context_line":"        return ports"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def _get_gateway_ports_query(self, context, subnet, ip_addresses):"},{"line_number":183,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":184,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":185,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_f2fdf86d","line":182,"range":{"start_line":182,"start_character":13,"end_line":182,"end_character":20},"in_reply_to":"7faddb67_c1002c37","updated":"2019-08-02 15:17:34.000000000","message":"Agreed. There are many ways to do this, here is an example from neutron-dynamic-routing for how filtering can be done to filter for router gateway ports https://github.com/openstack/neutron-dynamic-routing/blob/master/neutron_dynamic_routing/db/bgp_db.py#L960\n\nI tend to consult the routerports table for this sort of thing rather than the ports table, it will end up being smaller. We can then join it to the ports table for more info if needed. That will likely hit indexes so that at scale this query is still efficient.","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"275fa64518fdb5b317f35f5ccfadd0c391f099e9","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":180,"context_line":"        return ports"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def _get_gateway_ports_query(self, context, subnet, ip_addresses):"},{"line_number":183,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":184,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":185,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_edd95684","line":182,"range":{"start_line":182,"start_character":13,"end_line":182,"end_character":20},"in_reply_to":"7faddb67_f2fdf86d","updated":"2019-08-07 13:53:29.000000000","message":"Renamed: _get_gateway_ports_by_subnet_and_ip","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e539b87af6b351472327dd688449d14bc59b3b78","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        query \u003d query.filter("},{"line_number":186,"context_line":"            models_v2.Port.id \u003d\u003d models_v2.IPAllocation.port_id,"},{"line_number":187,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet,"},{"line_number":188,"context_line":"            models_v2.IPAllocation.ip_address.in_(ip_addresses))"},{"line_number":189,"context_line":"        return query.all()"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    @log_helpers.log_method_call"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_a1dc10e4","line":188,"range":{"start_line":188,"start_character":12,"end_line":188,"end_character":64},"updated":"2019-08-02 12:37:06.000000000","message":"this is only difference between this method and get_dvr_ports_by_subnet() from neutron/db/l3_dvr_db.py\n\nMaybe it could be somehow merged and reused in 2 places?","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"275fa64518fdb5b317f35f5ccfadd0c391f099e9","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        query \u003d query.filter("},{"line_number":186,"context_line":"            models_v2.Port.id \u003d\u003d models_v2.IPAllocation.port_id,"},{"line_number":187,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet,"},{"line_number":188,"context_line":"            models_v2.IPAllocation.ip_address.in_(ip_addresses))"},{"line_number":189,"context_line":"        return query.all()"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    @log_helpers.log_method_call"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_6de6c64d","line":188,"range":{"start_line":188,"start_character":12,"end_line":188,"end_character":64},"in_reply_to":"7faddb67_a1dc10e4","updated":"2019-08-07 13:53:29.000000000","message":"Done","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"ec6efa40573432e5285444855e2b4734dfd0d20f","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_4ab81310","line":194,"updated":"2019-08-14 18:42:20.000000000","message":"I think option 2 makes more sense as well.","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"50a43afea5ef8c1c1a0c6b07a0c5d009fd6488de","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_8301546d","line":194,"range":{"start_line":194,"start_character":13,"end_line":194,"end_character":21},"updated":"2019-08-11 19:40:43.000000000","message":"nit: it still can return any port by subnet and IP address. It\u0027s Your assumption when You are using this method that You will get from it gateway ports.","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d462aa3ed76c921406bd46520dbdd7259c2ce549","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_27015bb0","line":194,"range":{"start_line":194,"start_character":13,"end_line":194,"end_character":21},"in_reply_to":"7faddb67_0c457770","updated":"2019-08-14 08:57:43.000000000","message":"personally I would go with option 2 propesed by You :)","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"531fe1d9208063e7f2948924060175e7beeb5b77","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_ad551352","line":194,"range":{"start_line":194,"start_character":13,"end_line":194,"end_character":21},"in_reply_to":"7faddb67_27015bb0","updated":"2019-08-15 14:41:21.000000000","message":"Done","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7618cdc766e52078ffd9f9b5706dcf5deb684044","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_cef1b06c","line":194,"range":{"start_line":194,"start_character":13,"end_line":194,"end_character":21},"in_reply_to":"7faddb67_8301546d","updated":"2019-08-12 09:04:27.000000000","message":"It is used only once by such dvr related method. And the original return value is named as \"internal_gateway_ports\" line 219.","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6c8c076fb7bb4903c0185057ad53039aff72b2f9","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_0c457770","line":194,"range":{"start_line":194,"start_character":13,"end_line":194,"end_character":21},"in_reply_to":"7faddb67_98e0c928","updated":"2019-08-13 13:22:05.000000000","message":"This is an internal helper function for this DB Mixin class only. But, fine, I have two chices now:\n1. rename the param to \"gateway_ip_addresses\"\n2. remove this function since it has only few lines. get_ports_query_by_subnet_and_ip can be directly called in line 219.","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"243e0f9d1db97c44d0d19ccad4f5e3a7d681d437","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                   \u0027ports\u0027: ports})"},{"line_number":192,"context_line":"        return ports"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_gateway_ports_by_subnet_and_ip(self, context,"},{"line_number":195,"context_line":"                                            subnet, ip_addresses):"},{"line_number":196,"context_line":"        query \u003d get_ports_query_by_subnet_and_ip("},{"line_number":197,"context_line":"            context, subnet, ip_addresses)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_98e0c928","line":194,"range":{"start_line":194,"start_character":13,"end_line":194,"end_character":21},"in_reply_to":"7faddb67_cef1b06c","updated":"2019-08-13 08:40:11.000000000","message":"Yes, and I\u0027m fine with L219 where You are calling some function and You know that by passing proper IP addresses to it You will get gateway ports. But if someone else will come to this code in the future and will want to get list of gateway ports also he may not be aware that he will get it ONLY if he will pass proper IP addresses to it.\nBecause of that this method should be IMO named like \"_get_ports_by_subnet_and_ip\".","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"}],"neutron/db/l3_dvr_db.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e539b87af6b351472327dd688449d14bc59b3b78","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1244,"context_line":"        return False"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"    def get_dvr_ports_by_subnet(self, context, subnet_id):"},{"line_number":1247,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":1248,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":1249,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_611ff8b6","line":1246,"range":{"start_line":1246,"start_character":12,"end_line":1246,"end_character":15},"updated":"2019-08-02 12:37:06.000000000","message":"to be strict this method will return all ports which belong to the subnet, not only \"dvr\" ports (whatever it would mean :))","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"275fa64518fdb5b317f35f5ccfadd0c391f099e9","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1244,"context_line":"        return False"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"    def get_dvr_ports_by_subnet(self, context, subnet_id):"},{"line_number":1247,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":1248,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":1249,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_8dd46249","line":1246,"range":{"start_line":1246,"start_character":12,"end_line":1246,"end_character":15},"in_reply_to":"7faddb67_125d3478","updated":"2019-08-07 13:53:29.000000000","message":"Renamed: get_dvr_subnet_ports","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7618cdc766e52078ffd9f9b5706dcf5deb684044","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1244,"context_line":"        return False"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"    def get_dvr_ports_by_subnet(self, context, subnet_id):"},{"line_number":1247,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":1248,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":1249,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_4eab801d","line":1246,"range":{"start_line":1246,"start_character":12,"end_line":1246,"end_character":15},"in_reply_to":"7faddb67_23f8a07a","updated":"2019-08-12 09:04:27.000000000","message":"The original query only involved such \u0027ports\u0027 and \u0027ipalloction\u0027 table, I do not want to extend current query logical. And again, it is not for ports which belong to a distributed router, it queries all those ports in the subnet which connected to a distributed router.","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"059453dff0e183d5946922b286a1756401706303","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1244,"context_line":"        return False"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"    def get_dvr_ports_by_subnet(self, context, subnet_id):"},{"line_number":1247,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":1248,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":1249,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_125d3478","line":1246,"range":{"start_line":1246,"start_character":12,"end_line":1246,"end_character":15},"in_reply_to":"7faddb67_611ff8b6","updated":"2019-08-02 15:17:34.000000000","message":"Agreed. If you want to track down \"DVR\" ports, it may be better to look for l3_db.RouterPorts and filter by \"port_type\". It will be a smaller table. Something to consider.","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"50a43afea5ef8c1c1a0c6b07a0c5d009fd6488de","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1244,"context_line":"        return False"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"    def get_dvr_ports_by_subnet(self, context, subnet_id):"},{"line_number":1247,"context_line":"        query \u003d context.session.query(models_v2.Port)"},{"line_number":1248,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":1249,"context_line":"        query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_23f8a07a","line":1246,"range":{"start_line":1246,"start_character":12,"end_line":1246,"end_character":15},"in_reply_to":"7faddb67_8dd46249","updated":"2019-08-11 19:40:43.000000000","message":"Why You didn\u0027t this like Ryan suggested? IMO it could be better solution","commit_id":"fedffdc85f50e820793c00847ab58af8f9c1ef37"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"6ae26fd403069041feaa30082d1bd3a337f9648f","unresolved":false,"context_lines":[{"line_number":1246,"context_line":""},{"line_number":1247,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1248,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1249,"context_line":"        plugin \u003d directory.get_plugin()"},{"line_number":1250,"context_line":"        ports \u003d ["},{"line_number":1251,"context_line":"            plugin._make_port_dict(port, process_extensions\u003dFalse)"},{"line_number":1252,"context_line":"            for port in query.all()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_e30f252d","line":1249,"updated":"2019-08-07 14:59:09.000000000","message":"Do you need to do this?  Isn\u0027t it equivalent to:\n\n    self.l3plugin._core_plugin","commit_id":"409f9108024877ce6599e63b6a6a5071da81a903"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"66858e43a1972ee9834677a38b41f93f305e76c9","unresolved":false,"context_lines":[{"line_number":1246,"context_line":""},{"line_number":1247,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1248,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1249,"context_line":"        plugin \u003d directory.get_plugin()"},{"line_number":1250,"context_line":"        ports \u003d ["},{"line_number":1251,"context_line":"            plugin._make_port_dict(port, process_extensions\u003dFalse)"},{"line_number":1252,"context_line":"            for port in query.all()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_c671d746","line":1249,"in_reply_to":"7faddb67_e30f252d","updated":"2019-08-07 15:58:57.000000000","message":"Done","commit_id":"409f9108024877ce6599e63b6a6a5071da81a903"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"6ae26fd403069041feaa30082d1bd3a337f9648f","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"            plugin._make_port_dict(port, process_extensions\u003dFalse)"},{"line_number":1252,"context_line":"            for port in query.all()"},{"line_number":1253,"context_line":"        ]"},{"line_number":1254,"context_line":"        return ports"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"def is_distributed_router(router):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_d07af1b8","line":1254,"updated":"2019-08-07 14:59:09.000000000","message":"nit: since we don\u0027t use \u0027ports\u0027 otherwise, this can just be\n\nreturn [...]","commit_id":"409f9108024877ce6599e63b6a6a5071da81a903"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"66858e43a1972ee9834677a38b41f93f305e76c9","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"            plugin._make_port_dict(port, process_extensions\u003dFalse)"},{"line_number":1252,"context_line":"            for port in query.all()"},{"line_number":1253,"context_line":"        ]"},{"line_number":1254,"context_line":"        return ports"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"def is_distributed_router(router):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_a674db37","line":1254,"in_reply_to":"7faddb67_d07af1b8","updated":"2019-08-07 15:58:57.000000000","message":"Done","commit_id":"409f9108024877ce6599e63b6a6a5071da81a903"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"50a43afea5ef8c1c1a0c6b07a0c5d009fd6488de","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1246,"context_line":"        return False"},{"line_number":1247,"context_line":""},{"line_number":1248,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1249,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1250,"context_line":"        return ["},{"line_number":1251,"context_line":"            self.l3plugin._core_plugin._make_port_dict("}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_43f35ca0","line":1248,"range":{"start_line":1248,"start_character":12,"end_line":1248,"end_character":15},"updated":"2019-08-11 19:40:43.000000000","message":"stricly speaking this method still gets all ports which belongs to the given subnet, not only \"dvr\" ports","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6c8c076fb7bb4903c0185057ad53039aff72b2f9","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1246,"context_line":"        return False"},{"line_number":1247,"context_line":""},{"line_number":1248,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1249,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1250,"context_line":"        return ["},{"line_number":1251,"context_line":"            self.l3plugin._core_plugin._make_port_dict("}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_acf78359","line":1248,"range":{"start_line":1248,"start_character":12,"end_line":1248,"end_character":15},"in_reply_to":"7faddb67_385f55e6","updated":"2019-08-13 13:22:05.000000000","message":"This is a method for DVR related, so I\u0027d like to add some marking for us to notice such point.","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7618cdc766e52078ffd9f9b5706dcf5deb684044","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1246,"context_line":"        return False"},{"line_number":1247,"context_line":""},{"line_number":1248,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1249,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1250,"context_line":"        return ["},{"line_number":1251,"context_line":"            self.l3plugin._core_plugin._make_port_dict("}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_eee26ca0","line":1248,"range":{"start_line":1248,"start_character":12,"end_line":1248,"end_character":15},"in_reply_to":"7faddb67_43f35ca0","updated":"2019-08-12 09:04:27.000000000","message":"The ports are \"in this subnet which is connected to a distributed router.\" They are needed by the agent. Maybe I need a more clear name \"get_ports_under_dvr_connected_subnet\" ?","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ebcf76628805eab08e58af28ee7d5055fe67af7b","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1246,"context_line":"        return False"},{"line_number":1247,"context_line":""},{"line_number":1248,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1249,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1250,"context_line":"        return ["},{"line_number":1251,"context_line":"            self.l3plugin._core_plugin._make_port_dict("}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_4d9a5f14","line":1248,"range":{"start_line":1248,"start_character":12,"end_line":1248,"end_character":15},"in_reply_to":"7faddb67_871b6f1a","updated":"2019-08-15 14:43:07.000000000","message":"Renamed.","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d462aa3ed76c921406bd46520dbdd7259c2ce549","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1246,"context_line":"        return False"},{"line_number":1247,"context_line":""},{"line_number":1248,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1249,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1250,"context_line":"        return ["},{"line_number":1251,"context_line":"            self.l3plugin._core_plugin._make_port_dict("}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_871b6f1a","line":1248,"range":{"start_line":1248,"start_character":12,"end_line":1248,"end_character":15},"in_reply_to":"7faddb67_acf78359","updated":"2019-08-14 08:57:43.000000000","message":"IMHO this is already marked by name of class to which this method belongs. But I will not block this patch only because of this :)","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"243e0f9d1db97c44d0d19ccad4f5e3a7d681d437","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"                self.get_router(context.elevated(), router_id))"},{"line_number":1246,"context_line":"        return False"},{"line_number":1247,"context_line":""},{"line_number":1248,"context_line":"    def get_dvr_subnet_ports(self, context, subnet_id):"},{"line_number":1249,"context_line":"        query \u003d dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)"},{"line_number":1250,"context_line":"        return ["},{"line_number":1251,"context_line":"            self.l3plugin._core_plugin._make_port_dict("}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_385f55e6","line":1248,"range":{"start_line":1248,"start_character":12,"end_line":1248,"end_character":15},"in_reply_to":"7faddb67_eee26ca0","updated":"2019-08-13 08:40:11.000000000","message":"why not just \"get_subnet_ports\"?","commit_id":"4df4e02846adf4ffbdfe08c2b5bad5af922be8a1"}]}