)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a0dc9b5f32142338da737d2478e7b57b513d71dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ff5fa2f3_8cbd8c0d","updated":"2022-03-15 14:51:43.000000000","message":"code looks good, just one improvement option left in there. \n\nOn the other hand, perhaps it is worth to update octavia DB too, so that the subnet_id is included in the member information upon member creation without subnet. Let\u0027s see what Octavia experts think about it","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"c201cd9c8895d17a6037f03d668b2c20c7057260","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2cdca7ba_800edbd5","updated":"2022-03-15 14:57:12.000000000","message":"seems the update_loadbalancer_status octavia_lib (https://github.com/openstack/octavia-lib/blob/master/octavia_lib/api/drivers/driver_lib.py#L105) call only cares about id, provisioning_status and operating_status, so dismiss the comment about updating the octavia DB","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"4ef4141e252fd32226d96c05f05e623701568467","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7dbef2d7_ef5263df","in_reply_to":"ff5fa2f3_8cbd8c0d","updated":"2022-03-15 15:18:55.000000000","message":"I tried including it in response but looks like Octavia doesn\u0027t matter about it also the code confirms the test https://github.com/openstack/octavia-lib/blob/master/octavia_lib/api/drivers/driver_lib.py#L105","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0b1a9f74a7bcc87836eaff278abfb31e8e607808","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"203b5bc8_d8017984","updated":"2022-03-16 08:25:19.000000000","message":"Looks good, just one extra request, can you add a functional test to cover this case?","commit_id":"6198ab97c1cb6cd871520bb150594cd1dd4ea515"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"299deb4e9d1350d1ef18987e60c773103b9c7240","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f771e780_5fcae3a6","in_reply_to":"203b5bc8_d8017984","updated":"2022-03-16 11:06:48.000000000","message":"Sure, I just add some additional checks to existing test related to operations over member","commit_id":"6198ab97c1cb6cd871520bb150594cd1dd4ea515"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9eae58f876bf8fa5b4bb7cbd8ea13c2d839d76a5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d99a440c_376939b5","updated":"2022-03-25 08:53:48.000000000","message":"Looks good for me, thx","commit_id":"82eabfda244de2de54c8417f201c67129b875199"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"e1829cc2457dcb2293c9eb0f7e2fa27021462442","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f6a068f8_e3882c69","updated":"2022-03-18 07:06:03.000000000","message":"recheck","commit_id":"82eabfda244de2de54c8417f201c67129b875199"}],"ovn_octavia_provider/driver.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a0dc9b5f32142338da737d2478e7b57b513d71dc","unresolved":true,"context_lines":[{"line_number":340,"context_line":"                    member.address and self._ip_version_differs(member)):"},{"line_number":341,"context_line":"                skipped_members.append(member.member_id)"},{"line_number":342,"context_line":"                continue"},{"line_number":343,"context_line":"            # NOTE(mjozefcz): We need to have subnet_id information."},{"line_number":344,"context_line":"            # NOTE(froyo): if subnet_id not provided, lets try to get it"},{"line_number":345,"context_line":"            # from the member pool_id"},{"line_number":346,"context_line":"            subnet_id \u003d member.subnet_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"528bcf49_948d51ab","line":343,"range":{"start_line":343,"start_character":0,"end_line":343,"end_character":68},"updated":"2022-03-15 14:51:43.000000000","message":"move it to line 351 now, right?","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"4ef4141e252fd32226d96c05f05e623701568467","unresolved":false,"context_lines":[{"line_number":340,"context_line":"                    member.address and self._ip_version_differs(member)):"},{"line_number":341,"context_line":"                skipped_members.append(member.member_id)"},{"line_number":342,"context_line":"                continue"},{"line_number":343,"context_line":"            # NOTE(mjozefcz): We need to have subnet_id information."},{"line_number":344,"context_line":"            # NOTE(froyo): if subnet_id not provided, lets try to get it"},{"line_number":345,"context_line":"            # from the member pool_id"},{"line_number":346,"context_line":"            subnet_id \u003d member.subnet_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"8736eaff_a9ac2206","line":343,"range":{"start_line":343,"start_character":0,"end_line":343,"end_character":68},"in_reply_to":"528bcf49_948d51ab","updated":"2022-03-15 15:18:55.000000000","message":"Done","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a0dc9b5f32142338da737d2478e7b57b513d71dc","unresolved":true,"context_lines":[{"line_number":346,"context_line":"            subnet_id \u003d member.subnet_id"},{"line_number":347,"context_line":"            if (isinstance(subnet_id, o_datamodels.UnsetType) or"},{"line_number":348,"context_line":"                    not subnet_id):"},{"line_number":349,"context_line":"                member.subnet_id \u003d self._ovn_helper._get_subnet_from_pool("},{"line_number":350,"context_line":"                    member.pool_id)"},{"line_number":351,"context_line":"                if not member.subnet_id:"},{"line_number":352,"context_line":"                    msg \u003d _(\u0027Subnet is required, or Loadbalancer associated \u0027"},{"line_number":353,"context_line":"                            \u0027with Pool must have a subnet, for Member \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"335329ad_de718624","line":350,"range":{"start_line":349,"start_character":1,"end_line":350,"end_character":35},"updated":"2022-03-15 14:51:43.000000000","message":"perhaps the get_subnet_from_pool can be moved out of the loop starting at 338, since the subnet is the one of the VIP, which will be the same for all the members that do not have subnet_id","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"4ef4141e252fd32226d96c05f05e623701568467","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            subnet_id \u003d member.subnet_id"},{"line_number":347,"context_line":"            if (isinstance(subnet_id, o_datamodels.UnsetType) or"},{"line_number":348,"context_line":"                    not subnet_id):"},{"line_number":349,"context_line":"                member.subnet_id \u003d self._ovn_helper._get_subnet_from_pool("},{"line_number":350,"context_line":"                    member.pool_id)"},{"line_number":351,"context_line":"                if not member.subnet_id:"},{"line_number":352,"context_line":"                    msg \u003d _(\u0027Subnet is required, or Loadbalancer associated \u0027"},{"line_number":353,"context_line":"                            \u0027with Pool must have a subnet, for Member \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"fb30c419_25db364b","line":350,"range":{"start_line":349,"start_character":1,"end_line":350,"end_character":35},"in_reply_to":"335329ad_de718624","updated":"2022-03-15 15:18:55.000000000","message":"Done","commit_id":"f0327afa424c2fee9c9b46f66a060c8b29ceeee1"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"1e9accf3e561f9500b29dc35cabac83e75413691","unresolved":true,"context_lines":[{"line_number":278,"context_line":"        # the member is deleted, Octavia send the object without subnet_id."},{"line_number":279,"context_line":"        subnet_id \u003d member.subnet_id"},{"line_number":280,"context_line":"        if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):"},{"line_number":281,"context_line":"            member.subnet_id \u003d self._ovn_helper._get_subnet_from_pool("},{"line_number":282,"context_line":"                member.pool_id)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        request_info \u003d {\u0027id\u0027: member.member_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"b7871db2_332174e0","line":281,"range":{"start_line":281,"start_character":11,"end_line":281,"end_character":28},"updated":"2022-03-16 07:53:35.000000000","message":"to keep it consistent with member_create, assign it here to subnet_id, and use subnet_id (instead of member.subnet_id) at lines 288 and 298","commit_id":"113f4c31e05e05111432bdbff2a6ee5be60ee48b"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e5816c85d41b488369e7897e6081b5fb394d6f8a","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # the member is deleted, Octavia send the object without subnet_id."},{"line_number":279,"context_line":"        subnet_id \u003d member.subnet_id"},{"line_number":280,"context_line":"        if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):"},{"line_number":281,"context_line":"            member.subnet_id \u003d self._ovn_helper._get_subnet_from_pool("},{"line_number":282,"context_line":"                member.pool_id)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        request_info \u003d {\u0027id\u0027: member.member_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"0eeefd39_258a9e87","line":281,"range":{"start_line":281,"start_character":11,"end_line":281,"end_character":28},"in_reply_to":"b7871db2_332174e0","updated":"2022-03-16 08:09:00.000000000","message":"Done","commit_id":"113f4c31e05e05111432bdbff2a6ee5be60ee48b"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"1e9accf3e561f9500b29dc35cabac83e75413691","unresolved":true,"context_lines":[{"line_number":306,"context_line":"        # subnet_id, in that case the driver search it according to the"},{"line_number":307,"context_line":"        # pool_id, but it is not propagated to Octavia. In this case, if"},{"line_number":308,"context_line":"        # the member is updated, Octavia send the object without subnet_id."},{"line_number":309,"context_line":"        subnet_id \u003d old_member.subnet_id"},{"line_number":310,"context_line":"        if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):"},{"line_number":311,"context_line":"            old_member.subnet_id \u003d self._ovn_helper._get_subnet_from_pool("},{"line_number":312,"context_line":"                old_member.pool_id)"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        # Validate monitoring options if present"},{"line_number":315,"context_line":"        self._check_member_monitor_options(new_member)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7811dd34_13c2636a","line":312,"range":{"start_line":309,"start_character":1,"end_line":312,"end_character":35},"updated":"2022-03-16 07:53:35.000000000","message":"ditto","commit_id":"113f4c31e05e05111432bdbff2a6ee5be60ee48b"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e5816c85d41b488369e7897e6081b5fb394d6f8a","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        # subnet_id, in that case the driver search it according to the"},{"line_number":307,"context_line":"        # pool_id, but it is not propagated to Octavia. In this case, if"},{"line_number":308,"context_line":"        # the member is updated, Octavia send the object without subnet_id."},{"line_number":309,"context_line":"        subnet_id \u003d old_member.subnet_id"},{"line_number":310,"context_line":"        if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):"},{"line_number":311,"context_line":"            old_member.subnet_id \u003d self._ovn_helper._get_subnet_from_pool("},{"line_number":312,"context_line":"                old_member.pool_id)"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        # Validate monitoring options if present"},{"line_number":315,"context_line":"        self._check_member_monitor_options(new_member)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7105a704_c740d3b0","line":312,"range":{"start_line":309,"start_character":1,"end_line":312,"end_character":35},"in_reply_to":"7811dd34_13c2636a","updated":"2022-03-16 08:09:00.000000000","message":"Done","commit_id":"113f4c31e05e05111432bdbff2a6ee5be60ee48b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"04bac6b8b848c7ca6b898ad20468f68aff6aada7","unresolved":true,"context_lines":[{"line_number":351,"context_line":"                    if pool_subnet_id"},{"line_number":352,"context_line":"                    else self._ovn_helper._get_subnet_from_pool(pool_id))"},{"line_number":353,"context_line":"                # NOTE(mjozefcz): We need to have subnet_id information."},{"line_number":354,"context_line":"                if not pool_subnet_id:"},{"line_number":355,"context_line":"                    msg \u003d _(\u0027Subnet is required, or Loadbalancer associated \u0027"},{"line_number":356,"context_line":"                            \u0027with Pool must have a subnet, for Member \u0027"},{"line_number":357,"context_line":"                            \u0027creation with OVN Provider Driver\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"8826af5a_6b07c79e","line":354,"range":{"start_line":354,"start_character":16,"end_line":354,"end_character":38},"updated":"2022-03-25 08:39:39.000000000","message":"Is that possible? If a member is created from a pool, the pool must always have (if I\u0027m not wrong) a subnet_id.\n\nI\u0027m not against this check and the exception, I\u0027m just asking to know better the code.","commit_id":"82eabfda244de2de54c8417f201c67129b875199"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"4eacd37e3ca0abe1e0815868909155492f6586c6","unresolved":true,"context_lines":[{"line_number":351,"context_line":"                    if pool_subnet_id"},{"line_number":352,"context_line":"                    else self._ovn_helper._get_subnet_from_pool(pool_id))"},{"line_number":353,"context_line":"                # NOTE(mjozefcz): We need to have subnet_id information."},{"line_number":354,"context_line":"                if not pool_subnet_id:"},{"line_number":355,"context_line":"                    msg \u003d _(\u0027Subnet is required, or Loadbalancer associated \u0027"},{"line_number":356,"context_line":"                            \u0027with Pool must have a subnet, for Member \u0027"},{"line_number":357,"context_line":"                            \u0027creation with OVN Provider Driver\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"427b6b29_4059bfb0","line":354,"range":{"start_line":354,"start_character":16,"end_line":354,"end_character":38},"in_reply_to":"8826af5a_6b07c79e","updated":"2022-03-25 08:44:33.000000000","message":"Just in case that Octavia api return a None, when pool is requested in function _get_subnet_from_pool(...).","commit_id":"82eabfda244de2de54c8417f201c67129b875199"}],"ovn_octavia_provider/tests/functional/test_driver.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"31b36010def4aa175a0dff1f9d065b74f020543c","unresolved":true,"context_lines":[{"line_number":208,"context_line":"        self._update_member_and_validate("},{"line_number":209,"context_line":"            lb_data, pool_TCP_id, \"10.0.0.10\", remove_subnet_id\u003dTrue)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        # Test creating a Member without subnet but with pool"},{"line_number":212,"context_line":"        self._create_member_and_validate("},{"line_number":213,"context_line":"            lb_data, pool_TCP_id, None,"},{"line_number":214,"context_line":"            lb_data[\u0027vip_net_info\u0027][0], \u002710.0.0.11\u0027,"},{"line_number":215,"context_line":"            expected_subnet\u003dlb_data[\u0027vip_net_info\u0027][1])"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Deleting the pool should also delete the members."},{"line_number":218,"context_line":"        self._delete_pool_and_validate(lb_data, \"p_TCP\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"6b90be07_e7f85bf8","line":215,"range":{"start_line":211,"start_character":0,"end_line":215,"end_character":55},"updated":"2022-03-16 13:01:19.000000000","message":"this is already validated in 195-198, right? Perhaps move the comment back to line 194, and then add the \"testing deleting a member without a subnet\"comment to line 199","commit_id":"82eabfda244de2de54c8417f201c67129b875199"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"681f7662a69582d27a19dde1ea98039e649afc28","unresolved":true,"context_lines":[{"line_number":208,"context_line":"        self._update_member_and_validate("},{"line_number":209,"context_line":"            lb_data, pool_TCP_id, \"10.0.0.10\", remove_subnet_id\u003dTrue)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        # Test creating a Member without subnet but with pool"},{"line_number":212,"context_line":"        self._create_member_and_validate("},{"line_number":213,"context_line":"            lb_data, pool_TCP_id, None,"},{"line_number":214,"context_line":"            lb_data[\u0027vip_net_info\u0027][0], \u002710.0.0.11\u0027,"},{"line_number":215,"context_line":"            expected_subnet\u003dlb_data[\u0027vip_net_info\u0027][1])"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Deleting the pool should also delete the members."},{"line_number":218,"context_line":"        self._delete_pool_and_validate(lb_data, \"p_TCP\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"bbf6da6a_8d9f410d","line":215,"range":{"start_line":211,"start_character":0,"end_line":215,"end_character":55},"in_reply_to":"6b90be07_e7f85bf8","updated":"2022-03-16 16:04:23.000000000","message":"right! In fact also in 204-207,  but I move just to add create+delete and create+update to not affect to the original behaviour of test that create a new member and trust on delete pool to confirm also the delete of members.","commit_id":"82eabfda244de2de54c8417f201c67129b875199"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"e1829cc2457dcb2293c9eb0f7e2fa27021462442","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        self._update_member_and_validate("},{"line_number":209,"context_line":"            lb_data, pool_TCP_id, \"10.0.0.10\", remove_subnet_id\u003dTrue)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        # Test creating a Member without subnet but with pool"},{"line_number":212,"context_line":"        self._create_member_and_validate("},{"line_number":213,"context_line":"            lb_data, pool_TCP_id, None,"},{"line_number":214,"context_line":"            lb_data[\u0027vip_net_info\u0027][0], \u002710.0.0.11\u0027,"},{"line_number":215,"context_line":"            expected_subnet\u003dlb_data[\u0027vip_net_info\u0027][1])"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Deleting the pool should also delete the members."},{"line_number":218,"context_line":"        self._delete_pool_and_validate(lb_data, \"p_TCP\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"965d5ce7_804f4422","line":215,"range":{"start_line":211,"start_character":0,"end_line":215,"end_character":55},"in_reply_to":"bbf6da6a_8d9f410d","updated":"2022-03-18 07:06:03.000000000","message":"Done","commit_id":"82eabfda244de2de54c8417f201c67129b875199"}]}