)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"c82ff3e554adce796bed24ba9b9e2e7b1e2a612c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3105dbed_7cb34cff","updated":"2024-04-22 07:40:19.000000000","message":"recheck get new data","commit_id":"3ceb7ac0345d5d36c50b83f581d579cfb932f662"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"3a49196703b615cc77297688e91ffeb80387f687","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"bc27ba42_2cff016e","updated":"2024-04-16 06:43:34.000000000","message":"recheck unrelated\nE: Failed to fetch http://mirror.gra1.ovh.opendev.org/ubuntu/dists/jammy-updates/universe/binary-amd64/Packages.gz  File has unexpected size (1580248 !\u003d 1579526). Mirror sync in progress? [IP: 217.182.142.242 80]","commit_id":"3ceb7ac0345d5d36c50b83f581d579cfb932f662"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"be79b7a6b1ca47ca0876fe09a3b668efa63651e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"dd001edc_8cf48b4a","updated":"2024-10-23 18:08:19.000000000","message":"A few comment to be addressed here.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"}],"octavia/amphorae/backends/utils/keepalivedlvs_query.py":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"be79b7a6b1ca47ca0876fe09a3b668efa63651e5","unresolved":true,"context_lines":[{"line_number":72,"context_line":"        ip_obj \u003d ipaddress.ip_address(listener_ip.strip(\u0027[]\u0027))"},{"line_number":73,"context_line":"        output \u003d read_kernel_file(ns_name, KERNEL_LVS_PATH).split(\u0027\\n\u0027)"},{"line_number":74,"context_line":"        if ip_obj.version \u003d\u003d 4:"},{"line_number":75,"context_line":"            ip_to_hex_format \u003d f\"{ip_obj._ip:08X}\"  # type: ignore"},{"line_number":76,"context_line":"        else:"},{"line_number":77,"context_line":"            ip_to_hex_format \u003d r\u0027\\[\u0027 + ip_obj.exploded + r\u0027\\]\u0027"},{"line_number":78,"context_line":"        port_hex_format \u003d f\"{int(listener_port):04X}\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9986a688_fa694c4a","line":75,"range":{"start_line":75,"start_character":52,"end_line":75,"end_character":66},"updated":"2024-10-23 18:08:19.000000000","message":"I hope we don\u0027t end up with a lot of these throughout the code.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"39b78ead172ee1b9a29ec74835ba8ef14de74c67","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        ip_obj \u003d ipaddress.ip_address(listener_ip.strip(\u0027[]\u0027))"},{"line_number":73,"context_line":"        output \u003d read_kernel_file(ns_name, KERNEL_LVS_PATH).split(\u0027\\n\u0027)"},{"line_number":74,"context_line":"        if ip_obj.version \u003d\u003d 4:"},{"line_number":75,"context_line":"            ip_to_hex_format \u003d f\"{ip_obj._ip:08X}\"  # type: ignore"},{"line_number":76,"context_line":"        else:"},{"line_number":77,"context_line":"            ip_to_hex_format \u003d r\u0027\\[\u0027 + ip_obj.exploded + r\u0027\\]\u0027"},{"line_number":78,"context_line":"        port_hex_format \u003d f\"{int(listener_port):04X}\""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f8a95ac_35a22ca0","line":75,"range":{"start_line":75,"start_character":52,"end_line":75,"end_character":66},"in_reply_to":"9986a688_fa694c4a","updated":"2024-10-28 11:14:58.000000000","message":"I try to keep them at a minimum. I only use them if it would be really difficult to fix the issue otherwise.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"be79b7a6b1ca47ca0876fe09a3b668efa63651e5","unresolved":true,"context_lines":[{"line_number":142,"context_line":"    #                                       {\u0027id\u0027: member-id-2,"},{"line_number":143,"context_line":"    #                                        \u0027ipport\u0027: ipport}],"},{"line_number":144,"context_line":"    #                            \u0027HealthMonitor\u0027: {\u0027id\u0027: healthmonitor-id}}"},{"line_number":145,"context_line":"    resource_ipport_mapping: tp.Dict[str, tp.Any] \u003d {}"},{"line_number":146,"context_line":"    with open(util.keepalived_lvs_cfg_path(listener_id),"},{"line_number":147,"context_line":"              encoding\u003d\u0027utf-8\u0027) as f:"},{"line_number":148,"context_line":"        cfg \u003d f.read()"}],"source_content_type":"text/x-python","patch_set":10,"id":"9cab6db2_30596c4a","line":145,"range":{"start_line":145,"start_character":42,"end_line":145,"end_character":48},"updated":"2024-10-23 18:08:19.000000000","message":"This seems like a slippery slope.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"39b78ead172ee1b9a29ec74835ba8ef14de74c67","unresolved":true,"context_lines":[{"line_number":142,"context_line":"    #                                       {\u0027id\u0027: member-id-2,"},{"line_number":143,"context_line":"    #                                        \u0027ipport\u0027: ipport}],"},{"line_number":144,"context_line":"    #                            \u0027HealthMonitor\u0027: {\u0027id\u0027: healthmonitor-id}}"},{"line_number":145,"context_line":"    resource_ipport_mapping: tp.Dict[str, tp.Any] \u003d {}"},{"line_number":146,"context_line":"    with open(util.keepalived_lvs_cfg_path(listener_id),"},{"line_number":147,"context_line":"              encoding\u003d\u0027utf-8\u0027) as f:"},{"line_number":148,"context_line":"        cfg \u003d f.read()"}],"source_content_type":"text/x-python","patch_set":10,"id":"ae171adb_aba4c7ae","line":145,"range":{"start_line":145,"start_character":42,"end_line":145,"end_character":48},"in_reply_to":"9cab6db2_30596c4a","updated":"2024-10-28 11:14:58.000000000","message":"It\u0027s not perfect, but also not terrible. In this particular case the values can be either of type dict or list, so the precise annotation would be `tp.Dict[str, list | dict]`. When you do that, however, code checkers will check if the code works with both types. That means they will flag it when you use `.append()` or `resource_ipport_mapping[\u0027Listener\u0027][\u0027ipports\u0027]` because they only work with one specific type and not with the other. To fix that, you have to declare the exact type before you use those methods/syntaxes. I updated the code to do that in this example. By using `tp.Any` type checkers are less strict and that is okay sometimes.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"be79b7a6b1ca47ca0876fe09a3b668efa63651e5","unresolved":true,"context_lines":[{"line_number":187,"context_line":"        resource_type_ids \u003d CONFIG_COMMENT_REGEX.findall(cfg)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        for resource_type, resource_id in resource_type_ids:"},{"line_number":190,"context_line":"            value: tp.Union[dict, list] \u003d {\u0027id\u0027: resource_id}"},{"line_number":191,"context_line":"            if resource_type \u003d\u003d \u0027Member\u0027:"},{"line_number":192,"context_line":"                resource_type \u003d f\u0027{resource_type}s\u0027"},{"line_number":193,"context_line":"                if resource_type not in resource_ipport_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"72dcdea0_29c5c18f","line":190,"updated":"2024-10-23 18:08:19.000000000","message":"This seems incorrect to me. how can resource_id be a list here?\nAlso, isn\u0027t this a dict type?","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"39b78ead172ee1b9a29ec74835ba8ef14de74c67","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        resource_type_ids \u003d CONFIG_COMMENT_REGEX.findall(cfg)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        for resource_type, resource_id in resource_type_ids:"},{"line_number":190,"context_line":"            value: tp.Union[dict, list] \u003d {\u0027id\u0027: resource_id}"},{"line_number":191,"context_line":"            if resource_type \u003d\u003d \u0027Member\u0027:"},{"line_number":192,"context_line":"                resource_type \u003d f\u0027{resource_type}s\u0027"},{"line_number":193,"context_line":"                if resource_type not in resource_ipport_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"de4eb81d_d7f57bdb","line":190,"in_reply_to":"72dcdea0_29c5c18f","updated":"2024-10-28 11:14:58.000000000","message":"You misread. `value` is annotated to be either of type dict or list. In this line it is initialized as a dict, but in line 194 it becomes a list.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"}],"octavia/amphorae/drivers/keepalived/vrrp_rest_driver.py":[{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"82f857934865cff140f501d87617db3dfefa1d59","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        self.clients: dict"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @abc.abstractmethod"},{"line_number":36,"context_line":"    def _populate_amphora_api_version(self, amphora,"},{"line_number":37,"context_line":"                                      timeout_dict\u003dNone,"},{"line_number":38,"context_line":"                                      raise_retry_exception\u003dFalse):"},{"line_number":39,"context_line":"        \"\"\"Populate the amphora object with the api_version\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"055d92f6_bae17778","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":37},"updated":"2024-04-29 15:57:40.000000000","message":"this is a bit weird because it defines an abstract method that is not implemented by KeepalivedAmphoraDriverMixin nor its children, but in AmphoraLoadBalancerDriver.\nSo it works because HaproxyAmphoraLoadBalancerDriver inherits from AmphoraLoadBalancerDriver and KeepalivedAmphoraDriverMixin\n\nI\u0027m wondering if those 2 classes could inherit from a Generic class that would only define _populate_amphora_api_version as an abstract method.","commit_id":"40ce99d06f10ecd673e8143ef3802f4c02a9e2de"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"2e2e5a3d0dab2613b3c7774567cf1159ce8ca8dd","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        self.clients: dict"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @abc.abstractmethod"},{"line_number":36,"context_line":"    def _populate_amphora_api_version(self, amphora,"},{"line_number":37,"context_line":"                                      timeout_dict\u003dNone,"},{"line_number":38,"context_line":"                                      raise_retry_exception\u003dFalse):"},{"line_number":39,"context_line":"        \"\"\"Populate the amphora object with the api_version\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"83783055_42e1a3c1","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":37},"in_reply_to":"055d92f6_bae17778","updated":"2024-08-21 12:50:09.000000000","message":"It\u0027s not weird. That method must not be implemented in the base class (`KeepalivedAmphoraDriverMixin`), but in all the subclasses/children. That\u0027s how the `abc.abstractmethod` decorator works.\n\nYou say \"that is not implemented by KeepalivedAmphoraDriverMixin nor its children\". It must be implemented by its subclasses/children. The only subclass I could find is `HaproxyAmphoraLoadBalancerDriver`. `AmphoraLoadBalancerDriver` is not a subclass/child and hence does not need to implement the method. Honestly, I don\u0027t see why another, generic class would be needed.","commit_id":"40ce99d06f10ecd673e8143ef3802f4c02a9e2de"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"39b78ead172ee1b9a29ec74835ba8ef14de74c67","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self.clients: dict"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @abc.abstractmethod"},{"line_number":36,"context_line":"    def _populate_amphora_api_version(self, amphora,"},{"line_number":37,"context_line":"                                      timeout_dict\u003dNone,"},{"line_number":38,"context_line":"                                      raise_retry_exception\u003dFalse):"},{"line_number":39,"context_line":"        \"\"\"Populate the amphora object with the api_version\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"ef6228ac_2880a67d","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":37},"in_reply_to":"11f95db1_710ce2e6","updated":"2024-10-28 11:14:58.000000000","message":"The reason it is here is because this method is used by *this* class line 62. Moving it into another class (that is not a base class of this) will not work.","commit_id":"40ce99d06f10ecd673e8143ef3802f4c02a9e2de"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"be79b7a6b1ca47ca0876fe09a3b668efa63651e5","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        self.clients: dict"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @abc.abstractmethod"},{"line_number":36,"context_line":"    def _populate_amphora_api_version(self, amphora,"},{"line_number":37,"context_line":"                                      timeout_dict\u003dNone,"},{"line_number":38,"context_line":"                                      raise_retry_exception\u003dFalse):"},{"line_number":39,"context_line":"        \"\"\"Populate the amphora object with the api_version\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"11f95db1_710ce2e6","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":37},"in_reply_to":"83783055_42e1a3c1","updated":"2024-10-23 18:08:19.000000000","message":"This should move to class AmphoraLoadBalancerDriver in driver_base.py in my opinion.","commit_id":"40ce99d06f10ecd673e8143ef3802f4c02a9e2de"}],"octavia/tests/unit/amphorae/drivers/keepalived/test_vrrp_rest_driver.py":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"be79b7a6b1ca47ca0876fe09a3b668efa63651e5","unresolved":true,"context_lines":[{"line_number":25,"context_line":"API_VERSION \u003d \u00271.0\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class KeepalivedAmpDriverMixinImpl("},{"line_number":29,"context_line":"        vrrp_rest_driver.KeepalivedAmphoraDriverMixin):"},{"line_number":30,"context_line":"    \"\"\"The base class is abstract\"\"\""},{"line_number":31,"context_line":"    def _populate_amphora_api_version(self, amphora,"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a1e8f60_f4af870a","line":28,"updated":"2024-10-23 18:08:19.000000000","message":"This should not be needed.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"39b78ead172ee1b9a29ec74835ba8ef14de74c67","unresolved":true,"context_lines":[{"line_number":25,"context_line":"API_VERSION \u003d \u00271.0\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class KeepalivedAmpDriverMixinImpl("},{"line_number":29,"context_line":"        vrrp_rest_driver.KeepalivedAmphoraDriverMixin):"},{"line_number":30,"context_line":"    \"\"\"The base class is abstract\"\"\""},{"line_number":31,"context_line":"    def _populate_amphora_api_version(self, amphora,"}],"source_content_type":"text/x-python","patch_set":10,"id":"b9bcf5df_45ce3604","line":28,"in_reply_to":"5a1e8f60_f4af870a","updated":"2024-10-28 11:14:58.000000000","message":"It is needed because `_populate_amphora_api_version` is declared an abstract method and the test cannot instantiate the base class directly. If you think that is wrong I will remove the `abc.abstractmethod` decorator from `_populate_amphora_api_version` and it should work without this class here.","commit_id":"1aed14add667a6e073724548b727740ca4268b8f"}]}