)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"a95362867cdd3dd201968f862ef82861cf96bc9e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8c889163_385345a8","updated":"2026-05-21 20:03:19.000000000","message":"Found one thing that would be good to change before merging this I think.\n\nStatistics updates are quite frequent and would cause churn if we don\u0027t filter carefully.","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"}],"neutron/agent/ovn/extensions/bgp/__init__.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"96713e4fd8ab4de6a5127e781a79d0afb7ab5b5a","unresolved":true,"context_lines":[{"line_number":111,"context_line":"            return"},{"line_number":112,"context_line":"        LOG.info(\u0027Setting interconnect bridge to %s\u0027, name)"},{"line_number":113,"context_line":"        self.interconnect_bridge \u003d bridge.BGPInterconnectBridge(self, name)"},{"line_number":114,"context_line":"        self.interconnect_bridge.scan_existing_patch_ports()"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def clear_interconnect_bridge(self):"},{"line_number":117,"context_line":"        if self.interconnect_bridge is None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ed6f7395_e940406c","line":114,"range":{"start_line":114,"start_character":8,"end_line":114,"end_character":60},"updated":"2026-05-21 08:20:11.000000000","message":"nit: why don\u0027t you call this in the `BGPInterconnectBridge.__init__` method?","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e0424b44f2e8ae017b5a98413492ea785a684c4b","unresolved":true,"context_lines":[{"line_number":111,"context_line":"            return"},{"line_number":112,"context_line":"        LOG.info(\u0027Setting interconnect bridge to %s\u0027, name)"},{"line_number":113,"context_line":"        self.interconnect_bridge \u003d bridge.BGPInterconnectBridge(self, name)"},{"line_number":114,"context_line":"        self.interconnect_bridge.scan_existing_patch_ports()"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def clear_interconnect_bridge(self):"},{"line_number":117,"context_line":"        if self.interconnect_bridge is None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3b6b382e_91e323ec","line":114,"range":{"start_line":114,"start_character":8,"end_line":114,"end_character":60},"in_reply_to":"ed6f7395_e940406c","updated":"2026-05-21 12:51:58.000000000","message":"I thought it would be considered a side-effect of the constructor.","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"}],"neutron/agent/ovn/extensions/bgp/events.py":[{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"a95362867cdd3dd201968f862ef82861cf96bc9e","unresolved":true,"context_lines":[{"line_number":288,"context_line":"        if row.type !\u003d \u0027patch\u0027:"},{"line_number":289,"context_line":"            return False"},{"line_number":290,"context_line":"        if not row.ofport:"},{"line_number":291,"context_line":"            return False"},{"line_number":292,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":293,"context_line":"        if ic is None:"},{"line_number":294,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"688e709e_9a7593b6","line":291,"updated":"2026-05-21 20:03:19.000000000","message":"Similar to https://review.opendev.org/c/openstack/neutron/+/988701/4/neutron/agent/ovn/extensions/bgp/events.py#180\n\nwe need to be more selective about the updates here, e.g.:\n\n```\n class InterconnectPatchPortCreatedEvent(BGPAgentEvent):\n     TABLE \u003d \u0027Interface\u0027\n     EVENTS \u003d (BGPAgentEvent.ROW_CREATE, BGPAgentEvent.ROW_UPDATE,)\n+    COLUMNS \u003d (\u0027name\u0027, \u0027ofport\u0027, \u0027options\u0027, \u0027type\u0027)\n \n     def match_fn(self, event, row, old):\n         if not super().match_fn(event, row, old):\n@@ -289,6 +290,9 @@ class InterconnectPatchPortCreatedEvent(BGPAgentEvent):\n             return False\n         if not row.ofport:\n             return False\n+        if event \u003d\u003d self.ROW_UPDATE:\n+            if not any(hasattr(old, col) for col in self.COLUMNS):\n+                return False\n         ic \u003d self.bgp_agent.interconnect_bridge\n         if ic is None:\n             return False\n```\n\nOtherwise, statistics updates (which happen every 5s) and other updates will cause matches as well.\n\nFound out by accident while testing the patch in my env.","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e90f3fb53358ef047ad190fe4d12bf162d73e9be","unresolved":true,"context_lines":[{"line_number":288,"context_line":"        if row.type !\u003d \u0027patch\u0027:"},{"line_number":289,"context_line":"            return False"},{"line_number":290,"context_line":"        if not row.ofport:"},{"line_number":291,"context_line":"            return False"},{"line_number":292,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":293,"context_line":"        if ic is None:"},{"line_number":294,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"6d8e596b_700b5598","line":291,"in_reply_to":"688e709e_9a7593b6","updated":"2026-05-26 14:29:40.000000000","message":"Good catch! Thanks!","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6fd15eca56a75307b1b6378aec59a71bd9b91817","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        if row.type !\u003d \u0027patch\u0027:"},{"line_number":289,"context_line":"            return False"},{"line_number":290,"context_line":"        if not row.ofport:"},{"line_number":291,"context_line":"            return False"},{"line_number":292,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":293,"context_line":"        if ic is None:"},{"line_number":294,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"cfbab5dd_bf5c9745","line":291,"in_reply_to":"6d8e596b_700b5598","updated":"2026-05-26 14:45:45.000000000","message":"Done","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"96713e4fd8ab4de6a5127e781a79d0afb7ab5b5a","unresolved":true,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"e324a314_b0c34643","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"updated":"2026-05-21 08:20:11.000000000","message":"Why double checking this?","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fb24908cc810034c6b5120162ff6e6697b509a8d","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"75054bc4_0583820f","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"in_reply_to":"1649c3b7_51b5c8b8","updated":"2026-05-28 21:50:34.000000000","message":"Done","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"4f9d89bbe56d91b29d712f5cbf337da4da56603c","unresolved":true,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"1649c3b7_51b5c8b8","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"in_reply_to":"3d0164c8_1c44a15c","updated":"2026-05-28 15:51:19.000000000","message":"Yeah, the double check is valid. It\u0027s not a likely problem, but it is possible. (and I had marked it resolved, so don\u0027t blame yourself) 😉","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"606a9372216132174ceed732963425e77b45e28d","unresolved":true,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3d0164c8_1c44a15c","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"in_reply_to":"48765d76_7fb717a1","updated":"2026-05-28 15:13:31.000000000","message":"Didn\u0027t mean to mark it as resolved.","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f0eecead484779c6cdcb54577517c29f31f7eacb","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f8025f0_c696a762","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"in_reply_to":"5bfe244b_dbb934db","updated":"2026-05-27 23:25:34.000000000","message":"And this is a fundamentally hard thing to solve. API thread handles request, queues txns and waits on the result, Connection thread takes over (or can fire when data is available to read from ovsdb-server) and calls Idl.run() to send txns/process replies where the Event notify() and therefor match_fn() calls happen. But because Event.run() can call back into neutron API code which commits new txns and wait on them, we have to queue matches to the RowEventHandler thread to avoid the potential deadlock. I really wish we could do all of that inside the Connection thread, but things like changing API port status on events etc. make it necessary to allow calling back into neutron code from Event.run().","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"087a8b08209f807d21a6385079195c4f8726290f","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"48765d76_7fb717a1","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"in_reply_to":"5f8025f0_c696a762","updated":"2026-05-28 15:00:57.000000000","message":"but that said, we should double-check here, is that correct?","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e0424b44f2e8ae017b5a98413492ea785a684c4b","unresolved":true,"context_lines":[{"line_number":295,"context_line":"        return ic.ovs_bridge.get_bridge_for_iface(row.name) \u003d\u003d ic.name"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def run(self, event, row, old):"},{"line_number":298,"context_line":"        ic \u003d self.bgp_agent.interconnect_bridge"},{"line_number":299,"context_line":"        if ic is None:"},{"line_number":300,"context_line":"            return"},{"line_number":301,"context_line":"        ic.add_patch_port(row)"},{"line_number":302,"context_line":"        if ic.check_requirements_for_flows_met():"},{"line_number":303,"context_line":"            ic.configure_flows()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5bfe244b_dbb934db","line":300,"range":{"start_line":298,"start_character":8,"end_line":300,"end_character":18},"in_reply_to":"e324a314_b0c34643","updated":"2026-05-21 12:51:58.000000000","message":"Because match_fn and run_idl are run in separate threads. There could be a race condition where the bridge was deleted in between calling match_fn and run_idl.","commit_id":"089a6528ae0b86695c83134429b3a32b1e0a3e7d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f0eecead484779c6cdcb54577517c29f31f7eacb","unresolved":true,"context_lines":[{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def run(self, event, row, old):"},{"line_number":338,"context_line":"        name \u003d row.external_ids.get("},{"line_number":339,"context_line":"            constants.AGENT_BGP_INTERCONNECT_BRIDGE, \u0027\u0027).strip() or None"},{"line_number":340,"context_line":"        if name and self.agent_api.ovs_idl.br_exists(name).execute():"},{"line_number":341,"context_line":"            self.bgp_agent.set_interconnect_bridge(name)"},{"line_number":342,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bccadc49_a384e379","line":339,"updated":"2026-05-27 23:25:34.000000000","message":"nit: Here we are stripping before doing a comparison, but in the match_fn() we aren\u0027t, so it seems like we could technically fail to match on a condition that this handles. (But is there ever going to be strippable whitespace there?)","commit_id":"1a5ffe4f63b60555aebd6403dc3bef6f7b1d45d2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1156c15f9ec4aa6db39021506362920b16d7817a","unresolved":false,"context_lines":[{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def run(self, event, row, old):"},{"line_number":338,"context_line":"        name \u003d row.external_ids.get("},{"line_number":339,"context_line":"            constants.AGENT_BGP_INTERCONNECT_BRIDGE, \u0027\u0027).strip() or None"},{"line_number":340,"context_line":"        if name and self.agent_api.ovs_idl.br_exists(name).execute():"},{"line_number":341,"context_line":"            self.bgp_agent.set_interconnect_bridge(name)"},{"line_number":342,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"b431e2a4_a9379d5e","line":339,"in_reply_to":"80d7574b_d5073a84","updated":"2026-05-28 15:20:06.000000000","message":"Done","commit_id":"1a5ffe4f63b60555aebd6403dc3bef6f7b1d45d2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"087a8b08209f807d21a6385079195c4f8726290f","unresolved":true,"context_lines":[{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def run(self, event, row, old):"},{"line_number":338,"context_line":"        name \u003d row.external_ids.get("},{"line_number":339,"context_line":"            constants.AGENT_BGP_INTERCONNECT_BRIDGE, \u0027\u0027).strip() or None"},{"line_number":340,"context_line":"        if name and self.agent_api.ovs_idl.br_exists(name).execute():"},{"line_number":341,"context_line":"            self.bgp_agent.set_interconnect_bridge(name)"},{"line_number":342,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"80d7574b_d5073a84","line":339,"in_reply_to":"bccadc49_a384e379","updated":"2026-05-28 15:00:57.000000000","message":"let\u0027s be defensive and strip() in match_fn() too, to be on the safe side","commit_id":"1a5ffe4f63b60555aebd6403dc3bef6f7b1d45d2"}],"neutron/tests/functional/agent/ovn/extensions/bgp/test_bridge.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f0eecead484779c6cdcb54577517c29f31f7eacb","unresolved":false,"context_lines":[{"line_number":265,"context_line":"            port_external_ids\u003d{ovn_const.OVN_PHYSNET_EXT_ID_KEY: \u0027public\u0027})"},{"line_number":266,"context_line":"        iface_row \u003d self._get_iface_row(port_name)"},{"line_number":267,"context_line":"        self.ic_bridge.add_patch_port(iface_row)"},{"line_number":268,"context_line":"        self.assertGreater(self.ic_bridge.provider_patch_ofport, 0)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test_bgp_patch_ofport(self):"},{"line_number":271,"context_line":"        port_name \u003d self._add_patch_port()"}],"source_content_type":"text/x-python","patch_set":5,"id":"d67c6304_0ee7c4a8","line":268,"range":{"start_line":268,"start_character":42,"end_line":268,"end_character":63},"updated":"2026-05-27 23:25:34.000000000","message":"I was originally going to complain that provider_patch_ofport can return None or [], but assertGreater() will still throw an exception in those cases, just not an AssertionError, so I assume we\u0027re good. :)","commit_id":"1a5ffe4f63b60555aebd6403dc3bef6f7b1d45d2"}]}
