)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support to PF OVN LBs for NB Driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch add support to the OVN LBs created when a port forwarding"},{"line_number":10,"context_line":"(PF) is configured over a FIP for the NB driver."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #2049415"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"2a3bf8bd_c3f33048","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":14},"updated":"2024-01-16 18:41:15.000000000","message":"adds","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support to PF OVN LBs for NB Driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch add support to the OVN LBs created when a port forwarding"},{"line_number":10,"context_line":"(PF) is configured over a FIP for the NB driver."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #2049415"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"177bc83e_98d4003f","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":14},"in_reply_to":"2a3bf8bd_c3f33048","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0f9938f5_319b2b32","updated":"2024-01-15 12:34:17.000000000","message":"perhaps worth to create a launchpad bug for this and add it here","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bdf7aa72_08b7897a","updated":"2024-01-17 08:32:10.000000000","message":"thx folks for the detailed review!","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"5eedadfa84fd13552b0a792e7408f6d3e8446f86","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f199ea92_c3cb8e91","updated":"2024-01-25 14:45:46.000000000","message":"Great addition! \n\nWhile testing the code on my env, i noticed two arguments were not the correct ones. I\u0027ve highlighted them in the other comments.","commit_id":"37e5b251fd3b721b3081c55c4957bf05ef8a0adf"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"1081b65d0f0d39dc767b666fe212740dedfb06f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"96e4326d_0cd4dcd9","in_reply_to":"f199ea92_c3cb8e91","updated":"2024-01-25 16:00:53.000000000","message":"good catch!!","commit_id":"37e5b251fd3b721b3081c55c4957bf05ef8a0adf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"586444eafcec28dd00fda10d3da0d646e4b33131","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ea7510e4_61d2da0d","updated":"2024-01-25 16:55:13.000000000","message":"Great progress! Some comments, mainly it would be good to avoid checking event type in the _run() method as the event is already known before the notifier calls it.","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"cfb21088aad316df1fa65098ebe44f4c2be0724a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a93347c8_a8a5c056","updated":"2024-01-25 16:20:15.000000000","message":"recheck openstack-tox-functional-with-sudo looks unrelated","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"}],"ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":848,"context_line":""},{"line_number":849,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"},{"line_number":850,"context_line":"    def expose_ovn_pf_lb_fip(self, lb):"},{"line_number":851,"context_line":"        self._expose_ovn_pf_lb_fip(lb)"},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    def _expose_ovn_pf_lb_fip(self, lb):"},{"line_number":854,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":855,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":856,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ec8ac7d_069c047a","line":853,"range":{"start_line":851,"start_character":0,"end_line":853,"end_character":40},"updated":"2024-01-16 18:41:15.000000000","message":"Why is this needed? Can\u0027t the method contain the code since it\u0027s already guarded with the decorator?","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":true,"context_lines":[{"line_number":848,"context_line":""},{"line_number":849,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"},{"line_number":850,"context_line":"    def expose_ovn_pf_lb_fip(self, lb):"},{"line_number":851,"context_line":"        self._expose_ovn_pf_lb_fip(lb)"},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    def _expose_ovn_pf_lb_fip(self, lb):"},{"line_number":854,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":855,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":856,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":4,"id":"55a615ae_f92e6c3a","line":853,"range":{"start_line":851,"start_character":0,"end_line":853,"end_character":40},"in_reply_to":"1ec8ac7d_069c047a","updated":"2024-01-17 08:32:10.000000000","message":"To maintain coherence with the rest of the existing code, I agree with comment for a follow up patch to apply it to the rest of the methods that follow the same structure (after confirm that those methods are always called through the decorator one).","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":848,"context_line":""},{"line_number":849,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"},{"line_number":850,"context_line":"    def expose_ovn_pf_lb_fip(self, lb):"},{"line_number":851,"context_line":"        self._expose_ovn_pf_lb_fip(lb)"},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    def _expose_ovn_pf_lb_fip(self, lb):"},{"line_number":854,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":855,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":856,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":4,"id":"cd44416c_3bfb0b43","line":853,"range":{"start_line":851,"start_character":0,"end_line":853,"end_character":40},"in_reply_to":"1ec8ac7d_069c047a","updated":"2024-01-17 08:22:24.000000000","message":"no, because this method is called also in the sync method that runs regularly (reconcile style) and also has the sync, it will get blocked","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":872,"context_line":"        self._withdraw_ovn_pf_lb_fip(lb)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"    def _withdraw_ovn_pf_lb_fip(self, lb):"},{"line_number":875,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":876,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":877,"context_line":"            break"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        router \u003d lb.external_ids.get("},{"line_number":880,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY, \u0027\u0027).replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":881,"context_line":"        if not router:"},{"line_number":882,"context_line":"            return"},{"line_number":883,"context_line":"        cr_lrp_info \u003d self.ovn_local_cr_lrps.get(router)"},{"line_number":884,"context_line":"        if not cr_lrp_info:"},{"line_number":885,"context_line":"            return"},{"line_number":886,"context_line":"        net, bridge_device, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":887,"context_line":"            cr_lrp_info[\u0027provider_switch\u0027])"},{"line_number":888,"context_line":"        self._withdraw_provider_port([fip], net, bridge_device, bridge_vlan)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7b0356aa_e356470f","line":887,"range":{"start_line":875,"start_character":0,"end_line":887,"end_character":43},"updated":"2024-01-16 18:41:15.000000000","message":"This looks like an exact copy\u0026paste of L854-L887","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":872,"context_line":"        self._withdraw_ovn_pf_lb_fip(lb)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"    def _withdraw_ovn_pf_lb_fip(self, lb):"},{"line_number":875,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":876,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":877,"context_line":"            break"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        router \u003d lb.external_ids.get("},{"line_number":880,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY, \u0027\u0027).replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":881,"context_line":"        if not router:"},{"line_number":882,"context_line":"            return"},{"line_number":883,"context_line":"        cr_lrp_info \u003d self.ovn_local_cr_lrps.get(router)"},{"line_number":884,"context_line":"        if not cr_lrp_info:"},{"line_number":885,"context_line":"            return"},{"line_number":886,"context_line":"        net, bridge_device, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":887,"context_line":"            cr_lrp_info[\u0027provider_switch\u0027])"},{"line_number":888,"context_line":"        self._withdraw_provider_port([fip], net, bridge_device, bridge_vlan)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"d437ce6d_84c4b9e0","line":887,"range":{"start_line":875,"start_character":0,"end_line":887,"end_character":43},"in_reply_to":"7b0356aa_e356470f","updated":"2024-01-17 08:32:10.000000000","message":"Done","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"3a3f14bf824b2c9a7cfd6e2cef25d6246c0cf571","unresolved":true,"context_lines":[{"line_number":334,"context_line":"    def _expose_lbs(self, router_list):"},{"line_number":335,"context_line":"        lbs \u003d self.nb_idl.get_active_local_lbs(router_list)"},{"line_number":336,"context_line":"        for lb in lbs:"},{"line_number":337,"context_line":"            if lb.name.startswith(constants.OVN_LB_PF_NAME_PREFIX):"},{"line_number":338,"context_line":"                self._manage_ovn_pf_lb_fip(lb)"},{"line_number":339,"context_line":"            else:"},{"line_number":340,"context_line":"                self._expose_ovn_lb_vip(lb)"}],"source_content_type":"text/x-python","patch_set":5,"id":"09bdfd43_75a53b2f","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":67},"updated":"2024-01-18 07:30:13.000000000","message":"nit: perhaps this could be moved to an aux function to make it clearer that it is about deciding if it is a PF load balancer or an ovn-octavia loadbalancer, e.g.:\n_is_pf_lb or get_lb_type (returning either port_forwarding or load_balancer)","commit_id":"385f050986e9987fa670426b52a4512ad0381362"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"979b5a391da765451b204c746392ee391979c590","unresolved":false,"context_lines":[{"line_number":334,"context_line":"    def _expose_lbs(self, router_list):"},{"line_number":335,"context_line":"        lbs \u003d self.nb_idl.get_active_local_lbs(router_list)"},{"line_number":336,"context_line":"        for lb in lbs:"},{"line_number":337,"context_line":"            if lb.name.startswith(constants.OVN_LB_PF_NAME_PREFIX):"},{"line_number":338,"context_line":"                self._manage_ovn_pf_lb_fip(lb)"},{"line_number":339,"context_line":"            else:"},{"line_number":340,"context_line":"                self._expose_ovn_lb_vip(lb)"}],"source_content_type":"text/x-python","patch_set":5,"id":"73098315_e272f06b","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":67},"in_reply_to":"09bdfd43_75a53b2f","updated":"2024-01-19 10:22:53.000000000","message":"Done","commit_id":"385f050986e9987fa670426b52a4512ad0381362"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"3a3f14bf824b2c9a7cfd6e2cef25d6246c0cf571","unresolved":true,"context_lines":[{"line_number":342,"context_line":"                if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY):"},{"line_number":343,"context_line":"                    self._expose_ovn_lb_fip(lb)"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    def _withdraw_lbs(self, router_list):"},{"line_number":346,"context_line":"        lbs \u003d self.nb_idl.get_active_local_lbs(router_list)"},{"line_number":347,"context_line":"        for lb in lbs:"},{"line_number":348,"context_line":"            self._withdraw_ovn_lb_vip(lb)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1439ef8f_21f6f974","line":345,"range":{"start_line":345,"start_character":0,"end_line":345,"end_character":41},"updated":"2024-01-18 07:30:13.000000000","message":"this also need to be updated to withdraw the PFs","commit_id":"385f050986e9987fa670426b52a4512ad0381362"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"979b5a391da765451b204c746392ee391979c590","unresolved":false,"context_lines":[{"line_number":342,"context_line":"                if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY):"},{"line_number":343,"context_line":"                    self._expose_ovn_lb_fip(lb)"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    def _withdraw_lbs(self, router_list):"},{"line_number":346,"context_line":"        lbs \u003d self.nb_idl.get_active_local_lbs(router_list)"},{"line_number":347,"context_line":"        for lb in lbs:"},{"line_number":348,"context_line":"            self._withdraw_ovn_lb_vip(lb)"}],"source_content_type":"text/x-python","patch_set":5,"id":"6a482c23_3df25dd6","line":345,"range":{"start_line":345,"start_character":0,"end_line":345,"end_character":41},"in_reply_to":"1439ef8f_21f6f974","updated":"2024-01-19 10:22:53.000000000","message":"Ok, to avoid any leftover in case ChassisRedirect or subnet detached","commit_id":"385f050986e9987fa670426b52a4512ad0381362"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"5eedadfa84fd13552b0a792e7408f6d3e8446f86","unresolved":true,"context_lines":[{"line_number":872,"context_line":"            cr_lrp_info[\u0027provider_switch\u0027])"},{"line_number":873,"context_line":"        if withdraw:"},{"line_number":874,"context_line":"            self._withdraw_provider_port("},{"line_number":875,"context_line":"                [fip], net, bridge_device, bridge_vlan)"},{"line_number":876,"context_line":"        else:"},{"line_number":877,"context_line":"            self._expose_provider_port("},{"line_number":878,"context_line":"                [fip], None, net, bridge_device, bridge_vlan, net)"}],"source_content_type":"text/x-python","patch_set":6,"id":"175aeffc_69f25699","line":875,"updated":"2024-01-25 14:45:46.000000000","message":"This `net` argument needs to be `cr_lrp_info[\u0027provider_switch\u0027]`","commit_id":"37e5b251fd3b721b3081c55c4957bf05ef8a0adf"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"1081b65d0f0d39dc767b666fe212740dedfb06f4","unresolved":false,"context_lines":[{"line_number":872,"context_line":"            cr_lrp_info[\u0027provider_switch\u0027])"},{"line_number":873,"context_line":"        if withdraw:"},{"line_number":874,"context_line":"            self._withdraw_provider_port("},{"line_number":875,"context_line":"                [fip], net, bridge_device, bridge_vlan)"},{"line_number":876,"context_line":"        else:"},{"line_number":877,"context_line":"            self._expose_provider_port("},{"line_number":878,"context_line":"                [fip], None, net, bridge_device, bridge_vlan, net)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e9442efd_84a8a38b","line":875,"in_reply_to":"175aeffc_69f25699","updated":"2024-01-25 16:00:53.000000000","message":"Done","commit_id":"37e5b251fd3b721b3081c55c4957bf05ef8a0adf"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"5eedadfa84fd13552b0a792e7408f6d3e8446f86","unresolved":true,"context_lines":[{"line_number":875,"context_line":"                [fip], net, bridge_device, bridge_vlan)"},{"line_number":876,"context_line":"        else:"},{"line_number":877,"context_line":"            self._expose_provider_port("},{"line_number":878,"context_line":"                [fip], None, net, bridge_device, bridge_vlan, net)"},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"},{"line_number":881,"context_line":"    def withdraw_ovn_pf_lb_fip(self, lb):"}],"source_content_type":"text/x-python","patch_set":6,"id":"a5b181b2_a5532a77","line":878,"updated":"2024-01-25 14:45:46.000000000","message":"The first `net` argument needs to be `cr_lrp_info[\u0027provider_switch\u0027]` as well.","commit_id":"37e5b251fd3b721b3081c55c4957bf05ef8a0adf"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"1081b65d0f0d39dc767b666fe212740dedfb06f4","unresolved":false,"context_lines":[{"line_number":875,"context_line":"                [fip], net, bridge_device, bridge_vlan)"},{"line_number":876,"context_line":"        else:"},{"line_number":877,"context_line":"            self._expose_provider_port("},{"line_number":878,"context_line":"                [fip], None, net, bridge_device, bridge_vlan, net)"},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"    @lockutils.synchronized(\u0027nbbgp\u0027)"},{"line_number":881,"context_line":"    def withdraw_ovn_pf_lb_fip(self, lb):"}],"source_content_type":"text/x-python","patch_set":6,"id":"dc27a2ca_ed0c073e","line":878,"in_reply_to":"a5b181b2_a5532a77","updated":"2024-01-25 16:00:53.000000000","message":"Done","commit_id":"37e5b251fd3b721b3081c55c4957bf05ef8a0adf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"586444eafcec28dd00fda10d3da0d646e4b33131","unresolved":true,"context_lines":[{"line_number":853,"context_line":"        self._manage_ovn_pf_lb_fip(lb)"},{"line_number":854,"context_line":""},{"line_number":855,"context_line":"    def _manage_ovn_pf_lb_fip(self, lb, withdraw\u003dFalse):"},{"line_number":856,"context_line":"        fip \u003d None"},{"line_number":857,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":858,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":859,"context_line":"            break"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        if not fip:"},{"line_number":862,"context_line":"            return"},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"        router \u003d lb.external_ids.get("},{"line_number":865,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY, \u0027\u0027).replace(\u0027neutron-\u0027, \"\", 1)"}],"source_content_type":"text/x-python","patch_set":7,"id":"18632268_3e419265","line":862,"range":{"start_line":856,"start_character":0,"end_line":862,"end_character":18},"updated":"2024-01-25 16:55:13.000000000","message":"```\nfor fipport in lb.vips.keys():\n    fip, port \u003d fipport.split(\u0027:\u0027)\n    break\nelse\n    return\n```","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"96f090c6efb298805c1f9e945109b58604056354","unresolved":true,"context_lines":[{"line_number":853,"context_line":"        self._manage_ovn_pf_lb_fip(lb)"},{"line_number":854,"context_line":""},{"line_number":855,"context_line":"    def _manage_ovn_pf_lb_fip(self, lb, withdraw\u003dFalse):"},{"line_number":856,"context_line":"        fip \u003d None"},{"line_number":857,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":858,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":859,"context_line":"            break"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        if not fip:"},{"line_number":862,"context_line":"            return"},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"        router \u003d lb.external_ids.get("},{"line_number":865,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY, \u0027\u0027).replace(\u0027neutron-\u0027, \"\", 1)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ac2ea478_fb231960","line":862,"range":{"start_line":856,"start_character":0,"end_line":862,"end_character":18},"in_reply_to":"18632268_3e419265","updated":"2024-01-25 16:56:37.000000000","message":"Now thinking about it again - I\u0027m not sure what it means. Is it that it just picks the first lb vip from available and if there are no vips, it returns?","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e0e07f5ca03541a91425c472dc87787b236e2a5b","unresolved":false,"context_lines":[{"line_number":853,"context_line":"        self._manage_ovn_pf_lb_fip(lb)"},{"line_number":854,"context_line":""},{"line_number":855,"context_line":"    def _manage_ovn_pf_lb_fip(self, lb, withdraw\u003dFalse):"},{"line_number":856,"context_line":"        fip \u003d None"},{"line_number":857,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":858,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":859,"context_line":"            break"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        if not fip:"},{"line_number":862,"context_line":"            return"},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"        router \u003d lb.external_ids.get("},{"line_number":865,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY, \u0027\u0027).replace(\u0027neutron-\u0027, \"\", 1)"}],"source_content_type":"text/x-python","patch_set":7,"id":"56287db5_1f9bae21","line":862,"range":{"start_line":856,"start_character":0,"end_line":862,"end_character":18},"in_reply_to":"8e63a339_eb1bdb69","updated":"2024-01-26 11:28:55.000000000","message":"yeah, you are right!","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"f5a5f2416f7c3970340aea62fa63429b90923a55","unresolved":true,"context_lines":[{"line_number":853,"context_line":"        self._manage_ovn_pf_lb_fip(lb)"},{"line_number":854,"context_line":""},{"line_number":855,"context_line":"    def _manage_ovn_pf_lb_fip(self, lb, withdraw\u003dFalse):"},{"line_number":856,"context_line":"        fip \u003d None"},{"line_number":857,"context_line":"        for fipport in lb.vips.keys():"},{"line_number":858,"context_line":"            fip, port \u003d fipport.split(\u0027:\u0027)"},{"line_number":859,"context_line":"            break"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        if not fip:"},{"line_number":862,"context_line":"            return"},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"        router \u003d lb.external_ids.get("},{"line_number":865,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY, \u0027\u0027).replace(\u0027neutron-\u0027, \"\", 1)"}],"source_content_type":"text/x-python","patch_set":7,"id":"8e63a339_eb1bdb69","line":862,"range":{"start_line":856,"start_character":0,"end_line":862,"end_character":18},"in_reply_to":"ac2ea478_fb231960","updated":"2024-01-25 17:25:19.000000000","message":"for .. else constructs are indeed like you say; if the `for` loop is not stopped by `break` it will go to `else` and in this case do a return.\n\nSo this basically checks if there is at least one lb.vips.keys entry.\n\nWould be the same as\n```\nif not lb.vips.keys():\n   return\n   \nfip, port \u003d lb.vips.keys()[0].split(\u0027:\u0027, 1)\n```\n\n(since it does not validate if the key itself is valid)","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"586444eafcec28dd00fda10d3da0d646e4b33131","unresolved":true,"context_lines":[{"line_number":870,"context_line":"            return"},{"line_number":871,"context_line":"        net, bridge_device, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":872,"context_line":"            cr_lrp_info[\u0027provider_switch\u0027])"},{"line_number":873,"context_line":"        if withdraw:"},{"line_number":874,"context_line":"            self._withdraw_provider_port("},{"line_number":875,"context_line":"                [fip], cr_lrp_info[\u0027provider_switch\u0027], bridge_device,"},{"line_number":876,"context_line":"                bridge_vlan)"}],"source_content_type":"text/x-python","patch_set":7,"id":"4f465553_8a778564","line":873,"updated":"2024-01-25 16:55:13.000000000","message":"It seems like this is a boolean set outside of this method meaning that caller already knew ahead which method is called at the end introducing unnecessary complexity. It\u0027s much better to break the method down into common things and call explicitly what you want to do, like:\n```\ndef _get_parameters(self, ...):\n    ... lines 856 ... 872 ...\n    return {...method parameters...}\n    \ndef _withdraw_pf_lb_vip(self, lb):\n    kwargs \u003d self._get_parameters(...)\n    self._withdraw_provider_port(**kwargs)\n    \ndef _expose_pf_lb_vip(self, lb):\n    kwargs \u003d self._get_parameters(...)\n    self._expose_provider_port(**kwargs)\n```\nideally we won\u0027t be having a ton of parameters passing around but use data structures but they seem missing in the code ... so kwargs could be a good replacement for that","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e0e07f5ca03541a91425c472dc87787b236e2a5b","unresolved":false,"context_lines":[{"line_number":870,"context_line":"            return"},{"line_number":871,"context_line":"        net, bridge_device, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":872,"context_line":"            cr_lrp_info[\u0027provider_switch\u0027])"},{"line_number":873,"context_line":"        if withdraw:"},{"line_number":874,"context_line":"            self._withdraw_provider_port("},{"line_number":875,"context_line":"                [fip], cr_lrp_info[\u0027provider_switch\u0027], bridge_device,"},{"line_number":876,"context_line":"                bridge_vlan)"}],"source_content_type":"text/x-python","patch_set":7,"id":"41edd5b1_74c4220f","line":873,"in_reply_to":"4f465553_8a778564","updated":"2024-01-26 11:28:55.000000000","message":"Not very convinced of this change finally, since as both methods have different signature it forces to have that additional parameter that includes the exclusive parametors of the longer method (expose) in the common get_parameter one, I agreed with your explanation but let\u0027s see how you like the change once implemented in the code.","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"}],"ovn_bgp_agent/drivers/openstack/utils/driver_utils.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c2b98004691ae85ffadd61520907611af888e3e","unresolved":true,"context_lines":[{"line_number":39,"context_line":"            constants.SUBNET_POOL_ADDR_SCOPE6),"},{"line_number":40,"context_line":"        }"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"def check_name_prefix(entity, prefix):"},{"line_number":44,"context_line":"    try:"},{"line_number":45,"context_line":"        return entity.name.startswith(prefix)"},{"line_number":46,"context_line":"    except AttributeError:"},{"line_number":47,"context_line":"        return False"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def is_pf_lb(lb):"},{"line_number":51,"context_line":"    return check_name_prefix(lb, constants.OVN_LB_PF_NAME_PREFIX)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9d6975e8_1a5f2c9f","line":51,"range":{"start_line":42,"start_character":0,"end_line":51,"end_character":65},"updated":"2024-02-01 22:25:08.000000000","message":"As these are general purpose functions perhaps it would be good cover them with tests to avoid surprises.","commit_id":"c923bd9c799e7480228e8757047bb93e123b3c94"}],"ovn_bgp_agent/drivers/openstack/utils/ovn.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":true,"context_lines":[{"line_number":471,"context_line":"        lbs \u003d []"},{"line_number":472,"context_line":"        cmd \u003d self.db_find_rows(\u0027Load_Balancer\u0027, (\u0027vips\u0027, \u0027!\u003d\u0027, {}))"},{"line_number":473,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":474,"context_line":"            if row.vips and (row.external_ids.get("},{"line_number":475,"context_line":"                    constants.OVN_LB_LR_REF_EXT_ID_KEY, row.external_ids.get("},{"line_number":476,"context_line":"                        constants.OVN_LR_NAME_EXT_ID_KEY, None)).replace("},{"line_number":477,"context_line":"                            \u0027neutron-\u0027, \"\", 1) in local_gateway_ports):"},{"line_number":478,"context_line":"                lbs.append(row)"},{"line_number":479,"context_line":"        return lbs"},{"line_number":480,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8ada6c1e_6f9b6e82","line":477,"range":{"start_line":474,"start_character":2,"end_line":477,"end_character":71},"updated":"2024-01-15 12:34:17.000000000","message":"this looks wrong, I assume you want an \"or\" here to either return the ones with are ovn-octaiva or PF loadbalancers, depending on the external_ids inputs. Please add a note if not the case","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"49f7cd6f10306e207f9b8a2bde069c0d105cd28e","unresolved":false,"context_lines":[{"line_number":471,"context_line":"        lbs \u003d []"},{"line_number":472,"context_line":"        cmd \u003d self.db_find_rows(\u0027Load_Balancer\u0027, (\u0027vips\u0027, \u0027!\u003d\u0027, {}))"},{"line_number":473,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":474,"context_line":"            if row.vips and (row.external_ids.get("},{"line_number":475,"context_line":"                    constants.OVN_LB_LR_REF_EXT_ID_KEY, row.external_ids.get("},{"line_number":476,"context_line":"                        constants.OVN_LR_NAME_EXT_ID_KEY, None)).replace("},{"line_number":477,"context_line":"                            \u0027neutron-\u0027, \"\", 1) in local_gateway_ports):"},{"line_number":478,"context_line":"                lbs.append(row)"},{"line_number":479,"context_line":"        return lbs"},{"line_number":480,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ad648358_177ed4df","line":477,"range":{"start_line":474,"start_character":2,"end_line":477,"end_character":71},"in_reply_to":"8ada6c1e_6f9b6e82","updated":"2024-01-15 17:05:16.000000000","message":"Replaced by a more clearly way, also added a NOTE to clarify why we need to search by two possibles keys.","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":474,"context_line":"        # NOTE(froyo): To retrieve both the OVN-Octavia-provider LBs and the"},{"line_number":475,"context_line":"        # Port Forwarding over FIP ones, it is necessary to search for both"},{"line_number":476,"context_line":"        # keys within the external_ids."},{"line_number":477,"context_line":"        external_id_keys \u003d ["},{"line_number":478,"context_line":"            constants.OVN_LB_LR_REF_EXT_ID_KEY,"},{"line_number":479,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY"},{"line_number":480,"context_line":"        ]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":483,"context_line":"            if not row.vips:"}],"source_content_type":"text/x-python","patch_set":4,"id":"f0a5a7b4_71963995","line":480,"range":{"start_line":477,"start_character":0,"end_line":480,"end_character":9},"updated":"2024-01-16 18:41:15.000000000","message":"This looks like a constant list which can be made a module level constant to avoid creating it on every call of this method.","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":474,"context_line":"        # NOTE(froyo): To retrieve both the OVN-Octavia-provider LBs and the"},{"line_number":475,"context_line":"        # Port Forwarding over FIP ones, it is necessary to search for both"},{"line_number":476,"context_line":"        # keys within the external_ids."},{"line_number":477,"context_line":"        external_id_keys \u003d ["},{"line_number":478,"context_line":"            constants.OVN_LB_LR_REF_EXT_ID_KEY,"},{"line_number":479,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY"},{"line_number":480,"context_line":"        ]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":483,"context_line":"            if not row.vips:"}],"source_content_type":"text/x-python","patch_set":4,"id":"c7b7bd3b_5590580c","line":480,"range":{"start_line":477,"start_character":0,"end_line":480,"end_character":9},"in_reply_to":"f0a5a7b4_71963995","updated":"2024-01-17 08:22:24.000000000","message":"+1","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        # NOTE(froyo): To retrieve both the OVN-Octavia-provider LBs and the"},{"line_number":475,"context_line":"        # Port Forwarding over FIP ones, it is necessary to search for both"},{"line_number":476,"context_line":"        # keys within the external_ids."},{"line_number":477,"context_line":"        external_id_keys \u003d ["},{"line_number":478,"context_line":"            constants.OVN_LB_LR_REF_EXT_ID_KEY,"},{"line_number":479,"context_line":"            constants.OVN_LR_NAME_EXT_ID_KEY"},{"line_number":480,"context_line":"        ]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":483,"context_line":"            if not row.vips:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a0740c3_584f131e","line":480,"range":{"start_line":477,"start_character":0,"end_line":480,"end_character":9},"in_reply_to":"f0a5a7b4_71963995","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":480,"context_line":"        ]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":483,"context_line":"            if not row.vips:"},{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"83e4f611_3914d6a1","line":484,"range":{"start_line":483,"start_character":0,"end_line":484,"end_character":24},"updated":"2024-01-16 18:41:15.000000000","message":"Can this happen? the line 472 is supposed to return only rows that have vips defined.","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":480,"context_line":"        ]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":483,"context_line":"            if not row.vips:"},{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"e1cedcf3_1cb19b37","line":484,"range":{"start_line":483,"start_character":0,"end_line":484,"end_character":24},"in_reply_to":"83e4f611_3914d6a1","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":480,"context_line":"        ]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":483,"context_line":"            if not row.vips:"},{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c172f543_1b8abf57","line":484,"range":{"start_line":483,"start_character":0,"end_line":484,"end_character":24},"in_reply_to":"83e4f611_3914d6a1","updated":"2024-01-17 08:22:24.000000000","message":"right! nice catch!","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                if external_id_value is not None:"},{"line_number":489,"context_line":"                    router_name \u003d external_id_value.replace(\u0027neutron-\u0027, \u0027\u0027, 1)"},{"line_number":490,"context_line":"                    if router_name in local_gateway_ports:"},{"line_number":491,"context_line":"                        lbs.append(row)"}],"source_content_type":"text/x-python","patch_set":4,"id":"879a1b87_7387bc02","line":488,"range":{"start_line":487,"start_character":0,"end_line":488,"end_character":49},"updated":"2024-01-16 18:41:15.000000000","message":"Python recommends to use EAFP unless there is a good reason to double-check values\nhttps://docs.python.org/3.11/glossary.html#term-EAFP","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e0e07f5ca03541a91425c472dc87787b236e2a5b","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                if external_id_value is not None:"},{"line_number":489,"context_line":"                    router_name \u003d external_id_value.replace(\u0027neutron-\u0027, \u0027\u0027, 1)"},{"line_number":490,"context_line":"                    if router_name in local_gateway_ports:"},{"line_number":491,"context_line":"                        lbs.append(row)"}],"source_content_type":"text/x-python","patch_set":4,"id":"19991b35_75c955cf","line":488,"range":{"start_line":487,"start_character":0,"end_line":488,"end_character":49},"in_reply_to":"701c71d8_2ad20c63","updated":"2024-01-26 11:28:55.000000000","message":"ACK, in the worst case (user with only OVN PF LBs) we will fail 50% of the time because we search first by lr_ref, so let\u0027s make the change taking into account that 50% will be the highest probability.","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                if external_id_value is not None:"},{"line_number":489,"context_line":"                    router_name \u003d external_id_value.replace(\u0027neutron-\u0027, \u0027\u0027, 1)"},{"line_number":490,"context_line":"                    if router_name in local_gateway_ports:"},{"line_number":491,"context_line":"                        lbs.append(row)"}],"source_content_type":"text/x-python","patch_set":4,"id":"09ae6981_a3f6d00e","line":488,"range":{"start_line":487,"start_character":0,"end_line":488,"end_character":49},"in_reply_to":"879a1b87_7387bc02","updated":"2024-01-17 08:22:24.000000000","message":"+1","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                if external_id_value is not None:"},{"line_number":489,"context_line":"                    router_name \u003d external_id_value.replace(\u0027neutron-\u0027, \u0027\u0027, 1)"},{"line_number":490,"context_line":"                    if router_name in local_gateway_ports:"},{"line_number":491,"context_line":"                        lbs.append(row)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dfb733cd_213de725","line":488,"range":{"start_line":487,"start_character":0,"end_line":488,"end_character":49},"in_reply_to":"879a1b87_7387bc02","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"586444eafcec28dd00fda10d3da0d646e4b33131","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                continue"},{"line_number":485,"context_line":"            for ext_id_key in external_id_keys:"},{"line_number":486,"context_line":"                external_id_value \u003d row.external_ids.get(ext_id_key, None)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                if external_id_value is not None:"},{"line_number":489,"context_line":"                    router_name \u003d external_id_value.replace(\u0027neutron-\u0027, \u0027\u0027, 1)"},{"line_number":490,"context_line":"                    if router_name in local_gateway_ports:"},{"line_number":491,"context_line":"                        lbs.append(row)"}],"source_content_type":"text/x-python","patch_set":4,"id":"701c71d8_2ad20c63","line":488,"range":{"start_line":487,"start_character":0,"end_line":488,"end_character":49},"in_reply_to":"dfb733cd_213de725","updated":"2024-01-25 16:55:13.000000000","message":"EAFP avoiding getters with default None values and using try-except blocks instead :) It\u0027s faster and more readable, unless there is a lot higher likelyhood the key we\u0027re searching for is not present in the dataset.","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"}],"ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":true,"context_lines":[{"line_number":64,"context_line":"            return"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class OVNPFEvent(Event):"},{"line_number":68,"context_line":"    def __init__(self, bgp_agent, events):"},{"line_number":69,"context_line":"        self.agent \u003d bgp_agent"},{"line_number":70,"context_line":"        table \u003d \u0027Load_Balancer\u0027"},{"line_number":71,"context_line":"        super(OVNPFEvent, self).__init__("},{"line_number":72,"context_line":"            events, table, None)"},{"line_number":73,"context_line":"        self.event_name \u003d self.__class__.__name__"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _get_router(self, row):"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            return row.external_ids["},{"line_number":78,"context_line":"                constants.OVN_LR_NAME_EXT_ID_KEY].replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":79,"context_line":"        except (AttributeError, KeyError):"},{"line_number":80,"context_line":"            return"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _check_name_prefix(self, row):"},{"line_number":83,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":84,"context_line":"                row.name.startswith(constants.OVN_LB_PF_NAME_PREFIX)):"},{"line_number":85,"context_line":"            return True"},{"line_number":86,"context_line":"        return False"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class LSPChassisEvent(Event):"}],"source_content_type":"text/x-python","patch_set":1,"id":"795eb187_af80692a","line":86,"range":{"start_line":67,"start_character":1,"end_line":86,"end_character":20},"updated":"2024-01-15 12:34:17.000000000","message":"do we need an extra class here? can we rely on OVNLBEvent instead and simply add the check_name_prefix method? At the end of the day, the event is the creation of a new Load_Balancer entry","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"3a2e94ca9d55ecd41a3a2988fe54536f8b3a0fa2","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            return"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class OVNPFEvent(Event):"},{"line_number":68,"context_line":"    def __init__(self, bgp_agent, events):"},{"line_number":69,"context_line":"        self.agent \u003d bgp_agent"},{"line_number":70,"context_line":"        table \u003d \u0027Load_Balancer\u0027"},{"line_number":71,"context_line":"        super(OVNPFEvent, self).__init__("},{"line_number":72,"context_line":"            events, table, None)"},{"line_number":73,"context_line":"        self.event_name \u003d self.__class__.__name__"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _get_router(self, row):"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            return row.external_ids["},{"line_number":78,"context_line":"                constants.OVN_LR_NAME_EXT_ID_KEY].replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":79,"context_line":"        except (AttributeError, KeyError):"},{"line_number":80,"context_line":"            return"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _check_name_prefix(self, row):"},{"line_number":83,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":84,"context_line":"                row.name.startswith(constants.OVN_LB_PF_NAME_PREFIX)):"},{"line_number":85,"context_line":"            return True"},{"line_number":86,"context_line":"        return False"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class LSPChassisEvent(Event):"}],"source_content_type":"text/x-python","patch_set":1,"id":"37852fc5_d73d206c","line":86,"range":{"start_line":67,"start_character":1,"end_line":86,"end_character":20},"in_reply_to":"43221298_12b49709","updated":"2024-01-29 10:39:17.000000000","message":"Done","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"49f7cd6f10306e207f9b8a2bde069c0d105cd28e","unresolved":true,"context_lines":[{"line_number":64,"context_line":"            return"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class OVNPFEvent(Event):"},{"line_number":68,"context_line":"    def __init__(self, bgp_agent, events):"},{"line_number":69,"context_line":"        self.agent \u003d bgp_agent"},{"line_number":70,"context_line":"        table \u003d \u0027Load_Balancer\u0027"},{"line_number":71,"context_line":"        super(OVNPFEvent, self).__init__("},{"line_number":72,"context_line":"            events, table, None)"},{"line_number":73,"context_line":"        self.event_name \u003d self.__class__.__name__"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _get_router(self, row):"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            return row.external_ids["},{"line_number":78,"context_line":"                constants.OVN_LR_NAME_EXT_ID_KEY].replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":79,"context_line":"        except (AttributeError, KeyError):"},{"line_number":80,"context_line":"            return"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _check_name_prefix(self, row):"},{"line_number":83,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":84,"context_line":"                row.name.startswith(constants.OVN_LB_PF_NAME_PREFIX)):"},{"line_number":85,"context_line":"            return True"},{"line_number":86,"context_line":"        return False"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class LSPChassisEvent(Event):"}],"source_content_type":"text/x-python","patch_set":1,"id":"43221298_12b49709","line":86,"range":{"start_line":67,"start_character":1,"end_line":86,"end_character":20},"in_reply_to":"795eb187_af80692a","updated":"2024-01-15 17:05:16.000000000","message":"I prefer to keep separately, working over the same Load_Balancer table, but having the additional _check_name_prefix here, and also note that the _get_router is different from this OVNPFEvent to the OVNLBEvent one.","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":53,"context_line":"    def _get_router(self, row, key\u003dconstants.OVN_LB_LR_REF_EXT_ID_KEY):"},{"line_number":54,"context_line":"        try:"},{"line_number":55,"context_line":"            return row.external_ids.get(key, None).replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":56,"context_line":"        except (AttributeError, KeyError):"},{"line_number":57,"context_line":"            return"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _get_vip_fip(self, row):"}],"source_content_type":"text/x-python","patch_set":4,"id":"e4956378_4f907474","line":56,"range":{"start_line":56,"start_character":32,"end_line":56,"end_character":40},"updated":"2024-01-16 18:41:15.000000000","message":"Where can this be raised from now? And why was this change needed? I liked the previous version better.","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    def _get_router(self, row, key\u003dconstants.OVN_LB_LR_REF_EXT_ID_KEY):"},{"line_number":54,"context_line":"        try:"},{"line_number":55,"context_line":"            return row.external_ids.get(key, None).replace(\u0027neutron-\u0027, \"\", 1)"},{"line_number":56,"context_line":"        except (AttributeError, KeyError):"},{"line_number":57,"context_line":"            return"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _get_vip_fip(self, row):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa232505_d65c9ef4","line":56,"range":{"start_line":56,"start_character":32,"end_line":56,"end_character":40},"in_reply_to":"e4956378_4f907474","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"6992451f_c81f7b1c","line":65,"updated":"2024-01-16 18:41:15.000000000","message":"`self` appears to not be used here. That indicates this should be either static method or better a helper function outside of the class as it seems it has logically nothing to do with the event but is rather a general purpose.","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"abd9484f4364bfca3e2f1e60f4fbb0f72739cf39","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"2c36a511_ce6fc783","line":65,"in_reply_to":"04873e26_4dee3faa","updated":"2024-01-29 10:38:31.000000000","message":"Done","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"586444eafcec28dd00fda10d3da0d646e4b33131","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"b96d1d27_e290f7eb","line":65,"in_reply_to":"39d324eb_06fc5278","updated":"2024-01-25 16:55:13.000000000","message":"imho it\u0027s not a good thing to keep repeating bad practices with an argument that this is how we did it in the past. We should keep improving. :)","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"34cb6d07_237d7f73","line":65,"in_reply_to":"6992451f_c81f7b1c","updated":"2024-01-17 08:22:24.000000000","message":"it has to do with the annotations by the CMS on the external_ids... related to the OVN LB object but not the event, perhaps it can be made static, but this is all over the place in this file, so this can be a follow up","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"39d324eb_06fc5278","line":65,"in_reply_to":"6992451f_c81f7b1c","updated":"2024-01-17 08:32:10.000000000","message":"yeah, as said before did to keep coherence with the rest of the file, but good candidate to a follow up patch","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"c952a199ffcf2ef9ffb425efc1a591f8a9da9ff0","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"04873e26_4dee3faa","line":65,"in_reply_to":"94f491f0_1831f50c","updated":"2024-01-29 09:40:30.000000000","message":"yep, lets move this one to a utils or common file and we move the rest in a follow up","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e0e07f5ca03541a91425c472dc87787b236e2a5b","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except (AttributeError, KeyError):"},{"line_number":63,"context_line":"            return"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def _check_name_prefix(self, row, prefix):"},{"line_number":66,"context_line":"        if (hasattr(row, \u0027name\u0027) and"},{"line_number":67,"context_line":"                row.name.startswith(prefix)):"},{"line_number":68,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"94f491f0_1831f50c","line":65,"in_reply_to":"b96d1d27_e290f7eb","updated":"2024-01-26 11:28:55.000000000","message":"Of course and totally agree, but extending the patch with a lot of changes along the file/s not related to the main reason of the patch doesn\u0027t seem right either (basically thinking about the reviewer and a hypothetical revert).","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"}],"ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":true,"context_lines":[{"line_number":671,"context_line":"                self.agent.expose_ovn_lb_vip(row)"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"class OVNPFCreateEvent(base_watcher.OVNPFEvent):"},{"line_number":675,"context_line":"    def __init__(self, bgp_agent):"},{"line_number":676,"context_line":"        events \u003d (self.ROW_CREATE)"},{"line_number":677,"context_line":"        super(OVNPFCreateEvent, self).__init__("}],"source_content_type":"text/x-python","patch_set":1,"id":"4b76ac9f_55aab679","line":674,"range":{"start_line":674,"start_character":0,"end_line":674,"end_character":2},"updated":"2024-01-15 12:34:17.000000000","message":"nit: perhaps put both events together, moving this to line 703","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"49f7cd6f10306e207f9b8a2bde069c0d105cd28e","unresolved":false,"context_lines":[{"line_number":671,"context_line":"                self.agent.expose_ovn_lb_vip(row)"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"class OVNPFCreateEvent(base_watcher.OVNPFEvent):"},{"line_number":675,"context_line":"    def __init__(self, bgp_agent):"},{"line_number":676,"context_line":"        events \u003d (self.ROW_CREATE)"},{"line_number":677,"context_line":"        super(OVNPFCreateEvent, self).__init__("}],"source_content_type":"text/x-python","patch_set":1,"id":"c32900ac_17c51faf","line":674,"range":{"start_line":674,"start_character":0,"end_line":674,"end_character":2},"in_reply_to":"4b76ac9f_55aab679","updated":"2024-01-15 17:05:16.000000000","message":"Done","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":true,"context_lines":[{"line_number":689,"context_line":"                return False"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"            lb_router \u003d self._get_router(row)"},{"line_number":692,"context_line":"            if lb_router not in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":693,"context_line":"                return False"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"            return True"},{"line_number":696,"context_line":"        except (IndexError, AttributeError):"},{"line_number":697,"context_line":"            return False"},{"line_number":698,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"632a4959_ebd95cc7","line":695,"range":{"start_line":692,"start_character":0,"end_line":695,"end_character":23},"updated":"2024-01-15 12:34:17.000000000","message":"probably the other way around is simpler:\nif lb_router in self.agent.ovn_local_cr_lrps.keys():\n    return True\n    \nAnd then add a return False at line 698","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"49f7cd6f10306e207f9b8a2bde069c0d105cd28e","unresolved":false,"context_lines":[{"line_number":689,"context_line":"                return False"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"            lb_router \u003d self._get_router(row)"},{"line_number":692,"context_line":"            if lb_router not in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":693,"context_line":"                return False"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"            return True"},{"line_number":696,"context_line":"        except (IndexError, AttributeError):"},{"line_number":697,"context_line":"            return False"},{"line_number":698,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5d212ba0_d6ae3b57","line":695,"range":{"start_line":692,"start_character":0,"end_line":695,"end_character":23},"in_reply_to":"632a4959_ebd95cc7","updated":"2024-01-15 17:05:16.000000000","message":"Done","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":true,"context_lines":[{"line_number":784,"context_line":"            if not self._check_name_prefix(row):"},{"line_number":785,"context_line":"                return False"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"            if event \u003d\u003d self.ROW_DELETE:"},{"line_number":788,"context_line":"                if not row.vips:"},{"line_number":789,"context_line":"                    return False"},{"line_number":790,"context_line":"                lb_router \u003d self._get_router(row)"}],"source_content_type":"text/x-python","patch_set":1,"id":"755bc369_d2213d96","line":787,"range":{"start_line":787,"start_character":2,"end_line":787,"end_character":40},"updated":"2024-01-15 12:34:17.000000000","message":"you are only listening to ROW_DELETE events here... so either you need to add UPDATE or you don\u0027t need this check","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"49f7cd6f10306e207f9b8a2bde069c0d105cd28e","unresolved":false,"context_lines":[{"line_number":784,"context_line":"            if not self._check_name_prefix(row):"},{"line_number":785,"context_line":"                return False"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"            if event \u003d\u003d self.ROW_DELETE:"},{"line_number":788,"context_line":"                if not row.vips:"},{"line_number":789,"context_line":"                    return False"},{"line_number":790,"context_line":"                lb_router \u003d self._get_router(row)"}],"source_content_type":"text/x-python","patch_set":1,"id":"068a96ab_41bc8d02","line":787,"range":{"start_line":787,"start_character":2,"end_line":787,"end_character":40},"in_reply_to":"755bc369_d2213d96","updated":"2024-01-15 17:05:16.000000000","message":"Acknowledged","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"36c611fd0b3bd3ce2ab933d8f0ca4beb6114be63","unresolved":true,"context_lines":[{"line_number":790,"context_line":"                lb_router \u003d self._get_router(row)"},{"line_number":791,"context_line":"                if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":792,"context_line":"                    return True"},{"line_number":793,"context_line":"                return False"},{"line_number":794,"context_line":"        except (IndexError, AttributeError):"},{"line_number":795,"context_line":"            return False"},{"line_number":796,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"fdf23aad_246d05db","line":793,"updated":"2024-01-15 12:34:17.000000000","message":"this is not needed, the one at line 796 will return false","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"49f7cd6f10306e207f9b8a2bde069c0d105cd28e","unresolved":false,"context_lines":[{"line_number":790,"context_line":"                lb_router \u003d self._get_router(row)"},{"line_number":791,"context_line":"                if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":792,"context_line":"                    return True"},{"line_number":793,"context_line":"                return False"},{"line_number":794,"context_line":"        except (IndexError, AttributeError):"},{"line_number":795,"context_line":"            return False"},{"line_number":796,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"c03cff5d_9965122f","line":793,"in_reply_to":"fdf23aad_246d05db","updated":"2024-01-15 17:05:16.000000000","message":"Done","commit_id":"d8443afe0011e926108b6251cdfe2b081949441f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":759,"context_line":"                return False"},{"line_number":760,"context_line":""},{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"0bad0042_36dd4768","line":763,"range":{"start_line":762,"start_character":0,"end_line":763,"end_character":27},"updated":"2024-01-16 18:41:15.000000000","message":"```\nreturn lb_router in self.agent.ovn_local_cr_lrps\n```\nand remove L766","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":759,"context_line":"                return False"},{"line_number":760,"context_line":""},{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"164148d4_42b3fa62","line":763,"range":{"start_line":762,"start_character":0,"end_line":763,"end_character":27},"in_reply_to":"0bad0042_36dd4768","updated":"2024-01-17 08:22:24.000000000","message":"+1","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":759,"context_line":"                return False"},{"line_number":760,"context_line":""},{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"1d2681be_f93cb591","line":763,"range":{"start_line":762,"start_character":0,"end_line":763,"end_character":27},"in_reply_to":"0bad0042_36dd4768","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"},{"line_number":767,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"2260658f_f87f35ee","line":764,"range":{"start_line":764,"start_character":16,"end_line":764,"end_character":26},"updated":"2024-01-16 18:41:15.000000000","message":"I can\u0027t see where is indexing used here, which line this can raise from?","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"},{"line_number":767,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fe20cad3_1ed66e25","line":764,"range":{"start_line":764,"start_character":28,"end_line":764,"end_character":42},"updated":"2024-01-16 18:41:15.000000000","message":"ditto","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"},{"line_number":767,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"908ffade_94a04b94","line":764,"range":{"start_line":764,"start_character":16,"end_line":764,"end_character":26},"in_reply_to":"2260658f_f87f35ee","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b70d24f3920a36bcb482067a1e9216f606fcb653","unresolved":true,"context_lines":[{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"},{"line_number":767,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"697c80db_60558ce4","line":764,"range":{"start_line":764,"start_character":16,"end_line":764,"end_character":26},"in_reply_to":"2260658f_f87f35ee","updated":"2024-01-17 08:22:24.000000000","message":"yep... this is copy and paste from others... can b e deleted","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"            if lb_router in self.agent.ovn_local_cr_lrps.keys():"},{"line_number":763,"context_line":"                return True"},{"line_number":764,"context_line":"        except (IndexError, AttributeError):"},{"line_number":765,"context_line":"            return False"},{"line_number":766,"context_line":"        return False"},{"line_number":767,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"6cf2382e_78c91eb5","line":764,"range":{"start_line":764,"start_character":28,"end_line":764,"end_character":42},"in_reply_to":"fe20cad3_1ed66e25","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"9ab401f07e3b102f3791f3fd42f7cd8bb311eba9"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"3a3f14bf824b2c9a7cfd6e2cef25d6246c0cf571","unresolved":true,"context_lines":[{"line_number":764,"context_line":"            self.agent.expose_ovn_pf_lb_fip(row)"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":""},{"line_number":767,"context_line":"class OVNPFDeleteEvent(base_watcher.OVNLBEvent):"},{"line_number":768,"context_line":"    def __init__(self, bgp_agent):"},{"line_number":769,"context_line":"        events \u003d (self.ROW_DELETE)"},{"line_number":770,"context_line":"        super(OVNPFDeleteEvent, self).__init__("},{"line_number":771,"context_line":"            bgp_agent, events)"},{"line_number":772,"context_line":""},{"line_number":773,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":774,"context_line":"        # The ovn lb balancers are exposed through the cr-lrp, so if the"},{"line_number":775,"context_line":"        # local agent does not have the matching router there is no need"},{"line_number":776,"context_line":"        # to process the event"},{"line_number":777,"context_line":"        if not self._check_name_prefix(row, constants.OVN_LB_PF_NAME_PREFIX):"},{"line_number":778,"context_line":"            return False"},{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        if not row.vips:"},{"line_number":781,"context_line":"            return False"},{"line_number":782,"context_line":"        lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":783,"context_line":"        return lb_router in self.agent.ovn_local_cr_lrps.keys()"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _run(self, event, row, old):"},{"line_number":786,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"},{"line_number":787,"context_line":"            self.agent.withdraw_ovn_pf_lb_fip(row)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b711140b_224a6ad9","line":787,"range":{"start_line":767,"start_character":0,"end_line":787,"end_character":50},"updated":"2024-01-18 07:30:13.000000000","message":"this looks exactly the same as OVNPFCreateEvent, if only CREATE/DELETE events are possible, this should be merged in a single class (e.g., OVNPFCreateDeleteEvent), that in the _run just call expose or withdraw depending on the event being CREATE or DELETE","commit_id":"385f050986e9987fa670426b52a4512ad0381362"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"979b5a391da765451b204c746392ee391979c590","unresolved":false,"context_lines":[{"line_number":764,"context_line":"            self.agent.expose_ovn_pf_lb_fip(row)"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":""},{"line_number":767,"context_line":"class OVNPFDeleteEvent(base_watcher.OVNLBEvent):"},{"line_number":768,"context_line":"    def __init__(self, bgp_agent):"},{"line_number":769,"context_line":"        events \u003d (self.ROW_DELETE)"},{"line_number":770,"context_line":"        super(OVNPFDeleteEvent, self).__init__("},{"line_number":771,"context_line":"            bgp_agent, events)"},{"line_number":772,"context_line":""},{"line_number":773,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":774,"context_line":"        # The ovn lb balancers are exposed through the cr-lrp, so if the"},{"line_number":775,"context_line":"        # local agent does not have the matching router there is no need"},{"line_number":776,"context_line":"        # to process the event"},{"line_number":777,"context_line":"        if not self._check_name_prefix(row, constants.OVN_LB_PF_NAME_PREFIX):"},{"line_number":778,"context_line":"            return False"},{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        if not row.vips:"},{"line_number":781,"context_line":"            return False"},{"line_number":782,"context_line":"        lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":783,"context_line":"        return lb_router in self.agent.ovn_local_cr_lrps.keys()"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _run(self, event, row, old):"},{"line_number":786,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"},{"line_number":787,"context_line":"            self.agent.withdraw_ovn_pf_lb_fip(row)"}],"source_content_type":"text/x-python","patch_set":5,"id":"17e6ea89_1be40a9a","line":787,"range":{"start_line":767,"start_character":0,"end_line":787,"end_character":50},"in_reply_to":"b711140b_224a6ad9","updated":"2024-01-19 10:22:53.000000000","message":"Done","commit_id":"385f050986e9987fa670426b52a4512ad0381362"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"586444eafcec28dd00fda10d3da0d646e4b33131","unresolved":true,"context_lines":[{"line_number":760,"context_line":""},{"line_number":761,"context_line":"    def _run(self, event, row, old):"},{"line_number":762,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"},{"line_number":763,"context_line":"            if event \u003d\u003d self.ROW_CREATE:"},{"line_number":764,"context_line":"                self.agent.expose_ovn_pf_lb_fip(row)"},{"line_number":765,"context_line":"            else:"},{"line_number":766,"context_line":"                self.agent.withdraw_ovn_pf_lb_fip(row)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5119a043_069fa5cf","line":766,"range":{"start_line":763,"start_character":0,"end_line":766,"end_character":54},"updated":"2024-01-25 16:55:13.000000000","message":"The event type is already checked by the notification handler and here we check it again. It\u0027s better to break this down into 3 classes and use inheritance:\n```\nclass OVNPFBaseEvent(base_watcher.OVNLBEvent):\n    event \u003d None\n    def __init__(...):\n        super().__init__(..., (self.event,))\n        \n    def match_fn(...):\n        ...\n        \nclass OVNPFCreateEvent(OVNPFBaseEvent):\n    event \u003d self.ROW_CREATE\n        \n    def _run(...)\n        with _SYNC_STATE_LOCK.read_lock():\n            self..agent.expose_ovn_pf_lb_fip(row)\n\nclass OVNPFDeleteEvent(OVNPFBaseEvent):\n    event \u003d self.ROW_DELETE\n        \n    def _run(...)\n        with _SYNC_STATE_LOCK.read_lock():\n            self.agent.withdraw_ovn_pf_lb_fip(row)\n```","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"c952a199ffcf2ef9ffb425efc1a591f8a9da9ff0","unresolved":false,"context_lines":[{"line_number":760,"context_line":""},{"line_number":761,"context_line":"    def _run(self, event, row, old):"},{"line_number":762,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"},{"line_number":763,"context_line":"            if event \u003d\u003d self.ROW_CREATE:"},{"line_number":764,"context_line":"                self.agent.expose_ovn_pf_lb_fip(row)"},{"line_number":765,"context_line":"            else:"},{"line_number":766,"context_line":"                self.agent.withdraw_ovn_pf_lb_fip(row)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5b617b67_b0e16687","line":766,"range":{"start_line":763,"start_character":0,"end_line":766,"end_character":54},"in_reply_to":"31e061a7_863dd6c0","updated":"2024-01-29 09:40:30.000000000","message":"as far as I see, there is no event checking in the match... so it was only being checked in the run. No strong opinion, but given this was just a single check, I actually like it more this version than the new one","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e0e07f5ca03541a91425c472dc87787b236e2a5b","unresolved":false,"context_lines":[{"line_number":760,"context_line":""},{"line_number":761,"context_line":"    def _run(self, event, row, old):"},{"line_number":762,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"},{"line_number":763,"context_line":"            if event \u003d\u003d self.ROW_CREATE:"},{"line_number":764,"context_line":"                self.agent.expose_ovn_pf_lb_fip(row)"},{"line_number":765,"context_line":"            else:"},{"line_number":766,"context_line":"                self.agent.withdraw_ovn_pf_lb_fip(row)"}],"source_content_type":"text/x-python","patch_set":7,"id":"31e061a7_863dd6c0","line":766,"range":{"start_line":763,"start_character":0,"end_line":766,"end_character":54},"in_reply_to":"5119a043_069fa5cf","updated":"2024-01-26 11:28:55.000000000","message":"Done","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c2b98004691ae85ffadd61520907611af888e3e","unresolved":false,"context_lines":[{"line_number":760,"context_line":""},{"line_number":761,"context_line":"    def _run(self, event, row, old):"},{"line_number":762,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"},{"line_number":763,"context_line":"            if event \u003d\u003d self.ROW_CREATE:"},{"line_number":764,"context_line":"                self.agent.expose_ovn_pf_lb_fip(row)"},{"line_number":765,"context_line":"            else:"},{"line_number":766,"context_line":"                self.agent.withdraw_ovn_pf_lb_fip(row)"}],"source_content_type":"text/x-python","patch_set":7,"id":"51c96db5_25e83b0a","line":766,"range":{"start_line":763,"start_character":0,"end_line":766,"end_character":54},"in_reply_to":"5b617b67_b0e16687","updated":"2024-02-01 22:25:08.000000000","message":"It is compared twice. First time it\u0027s checked here https://github.com/openstack/ovsdbapp/blob/fc9f465f6056c1259c265d26fb3d51d0b61d76cd/ovsdbapp/backend/ovs_idl/event.py#L47but not a big deal :) Just given that we can do it this way is imho more cleaner and follows more object oriented patterns.","commit_id":"eecfeddb0f2e20248b5698acf18d250358682084"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c2b98004691ae85ffadd61520907611af888e3e","unresolved":true,"context_lines":[{"line_number":759,"context_line":"        if not row.vips:"},{"line_number":760,"context_line":"            return False"},{"line_number":761,"context_line":"        lb_router \u003d self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY)"},{"line_number":762,"context_line":"        return lb_router in self.agent.ovn_local_cr_lrps.keys()"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":""},{"line_number":765,"context_line":"class OVNPFCreateEvent(OVNPFBaseEvent):"}],"source_content_type":"text/x-python","patch_set":9,"id":"8ec6449f_0267dcff","line":762,"range":{"start_line":762,"start_character":56,"end_line":762,"end_character":63},"updated":"2024-02-01 22:25:08.000000000","message":"nit: not needed","commit_id":"c923bd9c799e7480228e8757047bb93e123b3c94"}],"ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c2b98004691ae85ffadd61520907611af888e3e","unresolved":true,"context_lines":[{"line_number":1293,"context_line":"            name\u003d\u0027pf-floatingip-uuid-tcp\u0027,"},{"line_number":1294,"context_line":"            vips\u003d{\u0027fip:port\u0027: \u0027member:port\u0027})"},{"line_number":1295,"context_line":""},{"line_number":1296,"context_line":"        mock_get_ls_localnet_info \u003d mock.patch.object("},{"line_number":1297,"context_line":"            self.nb_bgp_driver, \u0027_get_ls_localnet_info\u0027).start()"},{"line_number":1298,"context_line":"        mock_get_ls_localnet_info.return_value \u003d (\u0027provider-ls\u0027, \u0027br-ex\u0027,"},{"line_number":1299,"context_line":"                                                  100)"},{"line_number":1300,"context_line":"        mock_expose_provider_port \u003d mock.patch.object("},{"line_number":1301,"context_line":"            self.nb_bgp_driver, \u0027_expose_provider_port\u0027).start()"},{"line_number":1302,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a7b07a57_6f519abb","line":1299,"range":{"start_line":1296,"start_character":0,"end_line":1299,"end_character":54},"updated":"2024-02-01 22:25:08.000000000","message":"nit: As `mock_get_ls_localnet_info` is not used anywhere, you can do just\n```\nmock.patch.object(\n    self.nb_bgp_driver, \u0027_get_ls_localnet_info\u0027,\n    return_value\u003d(\u0027provider-ls\u0027, \u0027br-ex\u0027, 100)\n).start()\n```","commit_id":"c923bd9c799e7480228e8757047bb93e123b3c94"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c2b98004691ae85ffadd61520907611af888e3e","unresolved":true,"context_lines":[{"line_number":1448,"context_line":"            name\u003d\u0027pf-floatingip-uuid-tcp\u0027,"},{"line_number":1449,"context_line":"            vips\u003d{\u0027fip:port\u0027: \u0027member:port\u0027})"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        mock_get_ls_localnet_info \u003d mock.patch.object("},{"line_number":1452,"context_line":"            self.nb_bgp_driver, \u0027_get_ls_localnet_info\u0027).start()"},{"line_number":1453,"context_line":"        mock_get_ls_localnet_info.return_value \u003d (\u0027provider-ls\u0027, \u0027br-ex\u0027, 100)"},{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"        mock_withdraw_provider_port \u003d mock.patch.object("},{"line_number":1456,"context_line":"            self.nb_bgp_driver, \u0027_withdraw_provider_port\u0027).start()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9e6e9211_caa31e2e","line":1453,"range":{"start_line":1451,"start_character":0,"end_line":1453,"end_character":78},"updated":"2024-02-01 22:25:08.000000000","message":"ditto","commit_id":"c923bd9c799e7480228e8757047bb93e123b3c94"}],"ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0939778fc7a814bab81c5b481b67214360e6c2c6","unresolved":true,"context_lines":[{"line_number":216,"context_line":"        self.nb_idl.db_find_rows.return_value.execute.return_value \u003d [lb1, lb2]"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        ret \u003d self.nb_idl.get_active_local_lbs(local_gateway_ports)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        self.assertEqual([lb1], ret)"},{"line_number":221,"context_line":"        self.nb_idl.db_find_rows.assert_called_once_with("},{"line_number":222,"context_line":"            \u0027Load_Balancer\u0027, (\u0027vips\u0027, \u0027!\u003d\u0027, {}))"}],"source_content_type":"text/x-python","patch_set":4,"id":"b43e9887_9e6483c4","side":"PARENT","line":219,"updated":"2024-01-16 18:41:15.000000000","message":"This change seems not needed.","commit_id":"3e4a3848963f64a0fd752a5b440290c8a04d764d"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"543891d2ad90af52be97900d33563e238efe1a88","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        self.nb_idl.db_find_rows.return_value.execute.return_value \u003d [lb1, lb2]"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        ret \u003d self.nb_idl.get_active_local_lbs(local_gateway_ports)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        self.assertEqual([lb1], ret)"},{"line_number":221,"context_line":"        self.nb_idl.db_find_rows.assert_called_once_with("},{"line_number":222,"context_line":"            \u0027Load_Balancer\u0027, (\u0027vips\u0027, \u0027!\u003d\u0027, {}))"}],"source_content_type":"text/x-python","patch_set":4,"id":"f7f492c8_90100f62","side":"PARENT","line":219,"in_reply_to":"b43e9887_9e6483c4","updated":"2024-01-17 08:32:10.000000000","message":"Acknowledged","commit_id":"3e4a3848963f64a0fd752a5b440290c8a04d764d"}]}