)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8147d33c1abb666f9ffd33e8279ad4fe8c40bee2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b2a55155_18a2053b","updated":"2023-05-09 12:05:05.000000000","message":"recheck fullstack","commit_id":"d5a453a82d187b22d3f24650eb11e4c1fc573fb2"}],"neutron/common/ovn/utils.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e1fd6840f0ebcac349ef272c91576dce614b28d9","unresolved":true,"context_lines":[{"line_number":869,"context_line":"    :param subnets: (list of dict) subnet dictionaries"},{"line_number":870,"context_line":"    :param fixed_ips: (list of dict) fixed IPs of several subnets (usually"},{"line_number":871,"context_line":"                      belonging to a network but not mandatory)"},{"line_number":872,"context_line":"    :param ml2_plugin: (``Ml2Plugin``) ML2 plugin instance"},{"line_number":873,"context_line":"    :return: (tuple of 2 strings) IPv4 and IPv6 address scope IDs"},{"line_number":874,"context_line":"    \"\"\""},{"line_number":875,"context_line":"    address4_scope_id, address6_scope_id \u003d \u0027\u0027, \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"7bfaa078_b33e13c7","line":872,"updated":"2023-05-30 15:09:16.000000000","message":"That looks a bit strange to pass an instance of ml2plugin","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7a82c71de9539e4ac0202a8e980e52c7b1ac8e8c","unresolved":false,"context_lines":[{"line_number":869,"context_line":"    :param subnets: (list of dict) subnet dictionaries"},{"line_number":870,"context_line":"    :param fixed_ips: (list of dict) fixed IPs of several subnets (usually"},{"line_number":871,"context_line":"                      belonging to a network but not mandatory)"},{"line_number":872,"context_line":"    :param ml2_plugin: (``Ml2Plugin``) ML2 plugin instance"},{"line_number":873,"context_line":"    :return: (tuple of 2 strings) IPv4 and IPv6 address scope IDs"},{"line_number":874,"context_line":"    \"\"\""},{"line_number":875,"context_line":"    address4_scope_id, address6_scope_id \u003d \u0027\u0027, \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"9cf96693_80a14bb8","line":872,"in_reply_to":"7bfaa078_b33e13c7","updated":"2023-05-31 15:54:21.000000000","message":"This is done in many places in Neutron, almost any method that receives \"plugin\" as input parameter.","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8949955722fe7a990223c8616fb652fe45614a4f","unresolved":false,"context_lines":[{"line_number":869,"context_line":"    :param subnets: (list of dict) subnet dictionaries"},{"line_number":870,"context_line":"    :param fixed_ips: (list of dict) fixed IPs of several subnets (usually"},{"line_number":871,"context_line":"                      belonging to a network but not mandatory)"},{"line_number":872,"context_line":"    :param ml2_plugin: (``Ml2Plugin``) ML2 plugin instance"},{"line_number":873,"context_line":"    :return: (tuple of 2 strings) IPv4 and IPv6 address scope IDs"},{"line_number":874,"context_line":"    \"\"\""},{"line_number":875,"context_line":"    address4_scope_id, address6_scope_id \u003d \u0027\u0027, \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"8f13fdb6_27165563","line":872,"in_reply_to":"9cf96693_80a14bb8","updated":"2023-06-01 07:36:45.000000000","message":"Yes sure it was just a remark, as this method is in a module called \"utils\" and we have to pass it instance of a class. \n\nUsually that means the class should handle the method","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e1fd6840f0ebcac349ef272c91576dce614b28d9","unresolved":true,"context_lines":[{"line_number":890,"context_line":"                if subnet_pool[\u0027ip_version\u0027] \u003d\u003d const.IP_VERSION_4:"},{"line_number":891,"context_line":"                    address4_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":892,"context_line":"                else:"},{"line_number":893,"context_line":"                    address6_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":894,"context_line":"        except n_exc.SubnetPoolNotFound:"},{"line_number":895,"context_line":"            # swallow the exception and just continue if the"},{"line_number":896,"context_line":"            # lookup failed"}],"source_content_type":"text/x-python","patch_set":4,"id":"74813d00_74b0643e","line":893,"updated":"2023-05-30 15:09:16.000000000","message":"In case that we have both address4_scope_id and address6_scope_id should not we stop the loop and return?\n\n```\nif address4_scope_id and address6_scope_id :\n   return ...\n```","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9c26c0330717d90e57fd7f2ea15a724f23d16d0b","unresolved":false,"context_lines":[{"line_number":890,"context_line":"                if subnet_pool[\u0027ip_version\u0027] \u003d\u003d const.IP_VERSION_4:"},{"line_number":891,"context_line":"                    address4_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":892,"context_line":"                else:"},{"line_number":893,"context_line":"                    address6_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":894,"context_line":"        except n_exc.SubnetPoolNotFound:"},{"line_number":895,"context_line":"            # swallow the exception and just continue if the"},{"line_number":896,"context_line":"            # lookup failed"}],"source_content_type":"text/x-python","patch_set":4,"id":"8fa7179d_ddf55cbf","line":893,"in_reply_to":"13c0a395_5e9e37d1","updated":"2023-06-01 13:55:45.000000000","message":"ACK It was well me who did not get the whole point. Thanks.","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7a82c71de9539e4ac0202a8e980e52c7b1ac8e8c","unresolved":false,"context_lines":[{"line_number":890,"context_line":"                if subnet_pool[\u0027ip_version\u0027] \u003d\u003d const.IP_VERSION_4:"},{"line_number":891,"context_line":"                    address4_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":892,"context_line":"                else:"},{"line_number":893,"context_line":"                    address6_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":894,"context_line":"        except n_exc.SubnetPoolNotFound:"},{"line_number":895,"context_line":"            # swallow the exception and just continue if the"},{"line_number":896,"context_line":"            # lookup failed"}],"source_content_type":"text/x-python","patch_set":4,"id":"82884dab_c1bcbc6c","line":893,"in_reply_to":"74813d00_74b0643e","updated":"2023-05-31 15:54:21.000000000","message":"An address scope can have one IP version only.\n\nAlso this is the code I\u0027m migrating: https://github.com/openstack/neutron/blob/3a1cdebde17821837972097f3466e21d1140a33d/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L332-L341","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8949955722fe7a990223c8616fb652fe45614a4f","unresolved":true,"context_lines":[{"line_number":890,"context_line":"                if subnet_pool[\u0027ip_version\u0027] \u003d\u003d const.IP_VERSION_4:"},{"line_number":891,"context_line":"                    address4_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":892,"context_line":"                else:"},{"line_number":893,"context_line":"                    address6_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":894,"context_line":"        except n_exc.SubnetPoolNotFound:"},{"line_number":895,"context_line":"            # swallow the exception and just continue if the"},{"line_number":896,"context_line":"            # lookup failed"}],"source_content_type":"text/x-python","patch_set":4,"id":"bb47e08e_3776c4ed","line":893,"in_reply_to":"82884dab_c1bcbc6c","updated":"2023-06-01 07:36:45.000000000","message":"I\u0027m probably missing something But in the case we have both we can return without the needs of other iterations, no?\n\n(I\u0027m not saying we should remove the last return, And in an other side I can understand POV saying that I don\u0027t want to double the returns)","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"53899e0031e7a51ab48e4514e5c6c61cd5ed76fc","unresolved":false,"context_lines":[{"line_number":890,"context_line":"                if subnet_pool[\u0027ip_version\u0027] \u003d\u003d const.IP_VERSION_4:"},{"line_number":891,"context_line":"                    address4_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":892,"context_line":"                else:"},{"line_number":893,"context_line":"                    address6_scope_id \u003d subnet_pool[\u0027address_scope_id\u0027]"},{"line_number":894,"context_line":"        except n_exc.SubnetPoolNotFound:"},{"line_number":895,"context_line":"            # swallow the exception and just continue if the"},{"line_number":896,"context_line":"            # lookup failed"}],"source_content_type":"text/x-python","patch_set":4,"id":"13c0a395_5e9e37d1","line":893,"in_reply_to":"bb47e08e_3776c4ed","updated":"2023-06-01 09:04:57.000000000","message":"It is not possible: a subnetpool has one single address scope ID [1] and that refers to an IPv4 or IPv6 address scope depending on the IP version [2]. So no, we can\u0027t have both.\n\n[1]https://github.com/openstack/neutron/blob/0de6a4d620f1cb780c6a3635e10406b0db97762a/neutron/db/models_v2.py#L302\n[2]https://github.com/openstack/neutron/blob/0de6a4d620f1cb780c6a3635e10406b0db97762a/neutron/db/models_v2.py#L284","commit_id":"a22b1dedc2fdc522e3079e2e7668c69f5246d784"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"63731be47d62b712fe84c5ed998332726f44471a","unresolved":true,"context_lines":[{"line_number":321,"context_line":"                            sub"},{"line_number":322,"context_line":"                            for sub in subnets"},{"line_number":323,"context_line":"                            if sub[\"id\"] \u003d\u003d ip[\"subnet_id\"]"},{"line_number":324,"context_line":"                        ][0]"},{"line_number":325,"context_line":"                    except IndexError:"},{"line_number":326,"context_line":"                        LOG.debug(\u0027Subnet not found for ip address %s\u0027,"},{"line_number":327,"context_line":"                                  ip_addr)"}],"source_content_type":"text/x-python","patch_set":2,"id":"43b59d39_f2b81fb7","line":324,"updated":"2023-05-08 13:59:15.000000000","message":"nit: Just something I noticed here is that in the new code you added, you create a dict:\n\nsubnets_by_id \u003d {subnet[\u0027id\u0027]: subnet for subnet in subnets}\n\nYou could do that here as well before this loop as well to not iterate this list every loop.","commit_id":"ba6778be7e98cbf6154f44ee2cdd2d8cfaddb25d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7688d8ff6a7579a8e14ff4bf288d22095eec63f2","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                            sub"},{"line_number":322,"context_line":"                            for sub in subnets"},{"line_number":323,"context_line":"                            if sub[\"id\"] \u003d\u003d ip[\"subnet_id\"]"},{"line_number":324,"context_line":"                        ][0]"},{"line_number":325,"context_line":"                    except IndexError:"},{"line_number":326,"context_line":"                        LOG.debug(\u0027Subnet not found for ip address %s\u0027,"},{"line_number":327,"context_line":"                                  ip_addr)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3711acc3_76bf0652","line":324,"in_reply_to":"43b59d39_f2b81fb7","updated":"2023-05-11 15:30:59.000000000","message":"This code could be improved later. I prefer to reduce the scope of the changes to the minimum size because I\u0027ll need to backport this change. The number of subnets is reduced (usually 1), this won\u0027t take too much.","commit_id":"ba6778be7e98cbf6154f44ee2cdd2d8cfaddb25d"}]}