)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8696102644d22003d2da7ab12aa8c2df8dc3683f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"81d3d65d_e8f9428e","updated":"2025-09-11 13:12:31.000000000","message":"Putting temporary -W until past RC1","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ac291434e3efe8c53afb0eb7ee46554097ebd59b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"647cf4a6_d677c06e","updated":"2025-09-15 12:37:04.000000000","message":"The functional tests failed because of \n\n2025-09-12 22:41:07.025 75165 ERROR ovsdbapp.backend.ovs_idl.vlog [None req-4c1b32dd-9479-49a7-a03f-3939c2e1ab5b - e7b1cdeb-08d6-40fd-81a1-5f90b8232bfc - - - -] attempting to write bad value to column networks (ovsdb error: 0 values when type requires between 1 and 9223372036854775807): ovs.db.error.Error: ovsdb error: 0 values when type requires between 1 and 9223372036854775807\n\nWe\u0027re missing this patch in OVN - https://github.com/ovn-org/ovn/commit/0ee90e29eb8b68bf8462bb5bc256a1002f2043cf","commit_id":"344712409bba8a70c14f04ac5c06ab5862bc45ef"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1450b2836f7ef867f5588ac593a9c896f3eb3575","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5c33b351_0b38d08d","updated":"2025-09-18 12:57:24.000000000","message":"recheck","commit_id":"14c6bfd11bb6aad0b3a4898884657eedcd264cf7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6cfe8e47e9df21c6c1e406bcd83764e9763b8f00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e1727b7f_e9f22cf9","updated":"2025-09-18 15:36:54.000000000","message":"recheck","commit_id":"14c6bfd11bb6aad0b3a4898884657eedcd264cf7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"48b757d2fb236543292cecd75131286fa8d19ce6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e957b5c6_9119fdc3","updated":"2025-10-14 08:00:22.000000000","message":"Soft -1","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f6d605322d4818dc4981f58c7422eb9ea399491","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"1e0723eb_ce56754c","updated":"2025-10-23 00:07:00.000000000","message":"recheck bug 2129284","commit_id":"b01480ac241f196a9947c59ee58ad5ffc2be093d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8bc09d2e84e00f66790c0352bacc108d41f0a55e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"97ab8368_bc3374c7","updated":"2025-11-05 14:55:59.000000000","message":"recheck neutron-tempest-plugin-openvswitch unrelated","commit_id":"b01480ac241f196a9947c59ee58ad5ffc2be093d"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"8194296d1d733b27c77fc3a36514e05f953b1313","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"01d20817_19871468","updated":"2025-12-10 11:58:00.000000000","message":"LGTM for me but needs to be rebased first","commit_id":"a5f18ca278b4f3c6d405f09270d61fe846e9fc90"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e1121130d85c4369367a13842d447fbbfb8a082","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"30d42de1_e59d1693","updated":"2025-12-18 21:32:11.000000000","message":"Thanks for comments, I fixed some and will send a new PS.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea2a632285f3f3dfdacefc1e71afabe76fd5fab0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"1641e008_ef814e05","updated":"2025-12-18 20:13:57.000000000","message":"just small comments.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"12e95792961d2faa28e2682d5a795dd5efd8b807","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"a7678e57_2fca01b6","updated":"2025-12-19 10:15:18.000000000","message":"I\u0027m definitely not such an expert in the ovsdbapp related stuff as Terry is so I will definitely wait for his testing but other than that it looks good for me.","commit_id":"a44ab9fd5c104f5fdbac22e6a2204c0b5eb1d996"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c4f3ec89e32b2416562a7cf1f5b60883503a6b73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"352d652d_fcd70682","updated":"2025-12-19 13:21:57.000000000","message":"recheck ovs-rally task timed out, unrelated to this patch","commit_id":"a44ab9fd5c104f5fdbac22e6a2204c0b5eb1d996"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"594b0bba36ca506e4b0fbaa56d1ee4f4349ba79f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"04aa636c_e1d6648b","updated":"2025-12-22 19:08:57.000000000","message":"LGoodTM","commit_id":"c11fabb4beef7c2df03402c342eb596cbd6bacc2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a04f2992c1b3cd9d764e85948be7a93ca8526a66","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"13a502b8_8022e154","updated":"2025-12-23 13:19:29.000000000","message":"recheck - VM running devstack ran out of memory","commit_id":"a2c318e344b1b515fcc88e74cea593303684462d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e681b84e47b56a9ed68fce328fb74dad8b0ea830","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"bfbd9b3b_24797894","updated":"2025-12-23 18:31:03.000000000","message":"recheck - oom killer again","commit_id":"a2c318e344b1b515fcc88e74cea593303684462d"}],"neutron/services/bgp/commands.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a6c4c8358941b73a1a611d9337cb5ca038a33d41","unresolved":true,"context_lines":[{"line_number":166,"context_line":"class LrRouteAddCommand(nb_cmd.LrRouteAddCommand):"},{"line_number":167,"context_line":"    def run_idl(self, txn):"},{"line_number":168,"context_line":"        super().run_idl(txn)"},{"line_number":169,"context_line":"        if isinstance(self.result, uuid.UUID):"},{"line_number":170,"context_line":"            self.result \u003d self.api.lookup("},{"line_number":171,"context_line":"                \u0027Logical_Router_Static_Route\u0027, self.result)"},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"c8dad6bb_67a654d5","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":46},"updated":"2025-09-11 07:30:16.000000000","message":"That will happen when the command is creating the LR instead of returning an existing one. I don\u0027t think we have this logic in any other command (neutron or ovsdbapp). If you need the result UUID (as I see in L479), why don\u0027t use use `ovsdbapp.utils.get_uuid`?","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"eb2d17e32b510e82410b0c098868ec48b9a4fedb","unresolved":true,"context_lines":[{"line_number":166,"context_line":"class LrRouteAddCommand(nb_cmd.LrRouteAddCommand):"},{"line_number":167,"context_line":"    def run_idl(self, txn):"},{"line_number":168,"context_line":"        super().run_idl(txn)"},{"line_number":169,"context_line":"        if isinstance(self.result, uuid.UUID):"},{"line_number":170,"context_line":"            self.result \u003d self.api.lookup("},{"line_number":171,"context_line":"                \u0027Logical_Router_Static_Route\u0027, self.result)"},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9c613db3_d14baa57","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":46},"in_reply_to":"32ca848a_c4fcf444","updated":"2025-09-24 18:22:03.000000000","message":"Seeing as these Commands are only used locally in this file, and not being added to (or replacing existing API commands), I\u0027d just prefix all of the Command classes with `_` and not worry too much about the implementation details unless something is actually broken. Either subclassing or wrapping the results with get_uuid() are both fine with me for local use.\n\nI believe LrRouteAddCommand would always return a RowView in ovsdbapp, but it relies on the inerited post_commit() which obviously presents a problem when using nested Commands/manually calling run_idl() and using the result pre-commit.","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e7e554505f75a50507b3e00b34d036067013e7cb","unresolved":true,"context_lines":[{"line_number":166,"context_line":"class LrRouteAddCommand(nb_cmd.LrRouteAddCommand):"},{"line_number":167,"context_line":"    def run_idl(self, txn):"},{"line_number":168,"context_line":"        super().run_idl(txn)"},{"line_number":169,"context_line":"        if isinstance(self.result, uuid.UUID):"},{"line_number":170,"context_line":"            self.result \u003d self.api.lookup("},{"line_number":171,"context_line":"                \u0027Logical_Router_Static_Route\u0027, self.result)"},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d85320da_518d9e9a","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":46},"in_reply_to":"9c613db3_d14baa57","updated":"2025-09-24 18:23:50.000000000","message":"nm on the last paragraph, LrRouteAddCommand is not an AddCommand subclass despite the name. :p","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5f982e4e55ed8928b85f1af6ff2a17bc837561aa","unresolved":true,"context_lines":[{"line_number":166,"context_line":"class LrRouteAddCommand(nb_cmd.LrRouteAddCommand):"},{"line_number":167,"context_line":"    def run_idl(self, txn):"},{"line_number":168,"context_line":"        super().run_idl(txn)"},{"line_number":169,"context_line":"        if isinstance(self.result, uuid.UUID):"},{"line_number":170,"context_line":"            self.result \u003d self.api.lookup("},{"line_number":171,"context_line":"                \u0027Logical_Router_Static_Route\u0027, self.result)"},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"32ca848a_c4fcf444","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":46},"in_reply_to":"b46174ba_b2955b51","updated":"2025-09-18 11:03:51.000000000","message":"I would like Terry to participate here. If this command is executed in a txn, we would expect a command, not a UUID. I still insist of not implementing this change here but using `ovsdbapp.utils.get_uuid` in `ReconcileChassisPeerCommand.run`.\n\nAlso if you actually need the UUID, please ask Terry about the persistent UUIDs but that could be over engineering.","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"145e684573bd30eee2f48668d4c9d989b3cdc876","unresolved":true,"context_lines":[{"line_number":166,"context_line":"class LrRouteAddCommand(nb_cmd.LrRouteAddCommand):"},{"line_number":167,"context_line":"    def run_idl(self, txn):"},{"line_number":168,"context_line":"        super().run_idl(txn)"},{"line_number":169,"context_line":"        if isinstance(self.result, uuid.UUID):"},{"line_number":170,"context_line":"            self.result \u003d self.api.lookup("},{"line_number":171,"context_line":"                \u0027Logical_Router_Static_Route\u0027, self.result)"},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b46174ba_b2955b51","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":46},"in_reply_to":"c8dad6bb_67a654d5","updated":"2025-09-11 17:57:58.000000000","message":"I can use that too, the main thing is to have the output consistent and not have uuid if it\u0027s new and Row if it already existed. I\u0027m fine with either.","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2327ca0be6ce7ecb779d6e3fd84f5a5313f0bac0","unresolved":false,"context_lines":[{"line_number":166,"context_line":"class LrRouteAddCommand(nb_cmd.LrRouteAddCommand):"},{"line_number":167,"context_line":"    def run_idl(self, txn):"},{"line_number":168,"context_line":"        super().run_idl(txn)"},{"line_number":169,"context_line":"        if isinstance(self.result, uuid.UUID):"},{"line_number":170,"context_line":"            self.result \u003d self.api.lookup("},{"line_number":171,"context_line":"                \u0027Logical_Router_Static_Route\u0027, self.result)"},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"77640f1d_bd5bf3ea","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":46},"in_reply_to":"d85320da_518d9e9a","updated":"2025-09-26 15:16:15.000000000","message":"Done","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a6c4c8358941b73a1a611d9337cb5ca038a33d41","unresolved":true,"context_lines":[{"line_number":394,"context_line":"        ).run_idl(txn)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        router_name \u003d get_chassis_router_name(self.hostname)"},{"line_number":397,"context_line":"        bgp_peer_mapping \u003d helpers.get_chassis_bgp_peer_mapping(self.chassis)"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"        for peer_index, (network_name, (lrp_ip, peer_ip)) in enumerate("},{"line_number":400,"context_line":"                bgp_peer_mapping.items()):"}],"source_content_type":"text/x-python","patch_set":1,"id":"8f5aa337_888b65ef","line":397,"range":{"start_line":397,"start_character":8,"end_line":397,"end_character":77},"updated":"2025-09-11 07:30:16.000000000","message":"just a question: does it mean a main router can be connected to multiple external networks? I think I didn\u0027t see that in the spec. I thought it was needed this architecture [1] per external network\n\n[1]https://review.opendev.org/c/openstack/neutron-specs/+/952872/12/images/ovn-bgp-topology.jpg","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5f982e4e55ed8928b85f1af6ff2a17bc837561aa","unresolved":false,"context_lines":[{"line_number":394,"context_line":"        ).run_idl(txn)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        router_name \u003d get_chassis_router_name(self.hostname)"},{"line_number":397,"context_line":"        bgp_peer_mapping \u003d helpers.get_chassis_bgp_peer_mapping(self.chassis)"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"        for peer_index, (network_name, (lrp_ip, peer_ip)) in enumerate("},{"line_number":400,"context_line":"                bgp_peer_mapping.items()):"}],"source_content_type":"text/x-python","patch_set":1,"id":"8f746ae1_2128a712","line":397,"range":{"start_line":397,"start_character":8,"end_line":397,"end_character":77},"in_reply_to":"16a999ce_260669dd","updated":"2025-09-18 11:03:51.000000000","message":"Thanks!","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"145e684573bd30eee2f48668d4c9d989b3cdc876","unresolved":true,"context_lines":[{"line_number":394,"context_line":"        ).run_idl(txn)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        router_name \u003d get_chassis_router_name(self.hostname)"},{"line_number":397,"context_line":"        bgp_peer_mapping \u003d helpers.get_chassis_bgp_peer_mapping(self.chassis)"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"        for peer_index, (network_name, (lrp_ip, peer_ip)) in enumerate("},{"line_number":400,"context_line":"                bgp_peer_mapping.items()):"}],"source_content_type":"text/x-python","patch_set":1,"id":"16a999ce_260669dd","line":397,"range":{"start_line":397,"start_character":8,"end_line":397,"end_character":77},"in_reply_to":"8f5aa337_888b65ef","updated":"2025-09-11 17:57:58.000000000","message":"No, it\u0027s still one spine\u0027n\u0027leaf topology - it uses ECMP routes (L496) which means traffic can ingress or egress on two physical interface but then they are joined/split by the per-chassis router. The main BGP router then connects to these chassis routers. It doesn\u0027t necessarily need to be two NICs, can be multiple.","commit_id":"a11fc1d8254e60fdf836b8b0f339aeeedc95a493"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ae9686ca51547ee8ddfb3a2bfa96415549726bd2","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            ha_chassis_group\u003dself.hcg,"},{"line_number":358,"context_line":"            options\u003d{\u0027dynamic-routing-maintain-vrf\u0027: \u0027true\u0027},"},{"line_number":359,"context_line":"        ).run_idl(txn)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"class ReconcileChassisCommand(cmd.BaseCommand):"},{"line_number":363,"context_line":"    def __init__(self, api, sb_api, chassis):"}],"source_content_type":"text/x-python","patch_set":5,"id":"b54ffa8f_7344acc3","line":360,"updated":"2025-09-24 13:45:38.000000000","message":"Rodolfo found this: it\u0027s missing a LR policy to steer traffic to the right LRP","commit_id":"06f5acc96b204d98620bdb476a94ccbde8f72945"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"df3db40d883d6ae83139dbbdd3f9a3d7ceaf9244","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            ha_chassis_group\u003dself.hcg,"},{"line_number":358,"context_line":"            options\u003d{\u0027dynamic-routing-maintain-vrf\u0027: \u0027true\u0027},"},{"line_number":359,"context_line":"        ).run_idl(txn)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"class ReconcileChassisCommand(cmd.BaseCommand):"},{"line_number":363,"context_line":"    def __init__(self, api, sb_api, chassis):"}],"source_content_type":"text/x-python","patch_set":5,"id":"c7b36a32_14d6d1ee","line":360,"in_reply_to":"b54ffa8f_7344acc3","updated":"2025-09-24 18:02:23.000000000","message":"This is actually correct, we don\u0027t have the code handling the connections to the provider switches yet.","commit_id":"06f5acc96b204d98620bdb476a94ccbde8f72945"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2327ca0be6ce7ecb779d6e3fd84f5a5313f0bac0","unresolved":false,"context_lines":[{"line_number":357,"context_line":"            ha_chassis_group\u003dself.hcg,"},{"line_number":358,"context_line":"            options\u003d{\u0027dynamic-routing-maintain-vrf\u0027: \u0027true\u0027},"},{"line_number":359,"context_line":"        ).run_idl(txn)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"class ReconcileChassisCommand(cmd.BaseCommand):"},{"line_number":363,"context_line":"    def __init__(self, api, sb_api, chassis):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9059b210_1396d8c3","line":360,"in_reply_to":"c7b36a32_14d6d1ee","updated":"2025-09-26 15:16:15.000000000","message":"Done","commit_id":"06f5acc96b204d98620bdb476a94ccbde8f72945"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"48b757d2fb236543292cecd75131286fa8d19ce6","unresolved":true,"context_lines":[{"line_number":430,"context_line":"            ).run_idl(txn)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"class ReconcileChassisPeerCommand(cmd.BaseCommand):"},{"line_number":434,"context_line":"    def __init__("},{"line_number":435,"context_line":"            self, api, sb_api, chassis, network_name, lrp_ip, peer_ip,"},{"line_number":436,"context_line":"            peer_index):"}],"source_content_type":"text/x-python","patch_set":8,"id":"525242dc_4494fb70","line":433,"updated":"2025-10-14 08:00:22.000000000","message":"nit: in these complex commands, it will be very helpful to add a little description of each operation (why the LS is created, why we connect this LR to this localnet, the route, the policy, etc.). This is just a suggestion for future debugging and maintenance.","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cce4fa3daeb77e17cbce090535b37fd96db45a04","unresolved":false,"context_lines":[{"line_number":430,"context_line":"            ).run_idl(txn)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"class ReconcileChassisPeerCommand(cmd.BaseCommand):"},{"line_number":434,"context_line":"    def __init__("},{"line_number":435,"context_line":"            self, api, sb_api, chassis, network_name, lrp_ip, peer_ip,"},{"line_number":436,"context_line":"            peer_index):"}],"source_content_type":"text/x-python","patch_set":8,"id":"e157c194_b5d86fce","line":433,"in_reply_to":"525242dc_4494fb70","updated":"2025-10-14 14:25:01.000000000","message":"Done","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"48b757d2fb236543292cecd75131286fa8d19ce6","unresolved":true,"context_lines":[{"line_number":486,"context_line":"            may_exist\u003dTrue,"},{"line_number":487,"context_line":"        ), txn)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # TODO(jlibosva): Workaround for ovsdbapp bug, remove once fixed"},{"line_number":490,"context_line":"        cmd.DbSetCommand("},{"line_number":491,"context_line":"            self.api,"},{"line_number":492,"context_line":"            \u0027Logical_Router_Static_Route\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"a0e9d301_bde3f2dc","line":489,"range":{"start_line":489,"start_character":8,"end_line":489,"end_character":72},"updated":"2025-10-14 08:00:22.000000000","message":"Can you add the LP bug for this?","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a7bff7a7f265f1e5bf1d9eda6e5d2e3f3c2237ba","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            may_exist\u003dTrue,"},{"line_number":487,"context_line":"        ), txn)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # TODO(jlibosva): Workaround for ovsdbapp bug, remove once fixed"},{"line_number":490,"context_line":"        cmd.DbSetCommand("},{"line_number":491,"context_line":"            self.api,"},{"line_number":492,"context_line":"            \u0027Logical_Router_Static_Route\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"84770b21_a4b4af1b","line":489,"range":{"start_line":489,"start_character":8,"end_line":489,"end_character":72},"in_reply_to":"01ce081e_a74ed877","updated":"2025-10-14 14:59:05.000000000","message":"Done","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cce4fa3daeb77e17cbce090535b37fd96db45a04","unresolved":true,"context_lines":[{"line_number":486,"context_line":"            may_exist\u003dTrue,"},{"line_number":487,"context_line":"        ), txn)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # TODO(jlibosva): Workaround for ovsdbapp bug, remove once fixed"},{"line_number":490,"context_line":"        cmd.DbSetCommand("},{"line_number":491,"context_line":"            self.api,"},{"line_number":492,"context_line":"            \u0027Logical_Router_Static_Route\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"01ce081e_a74ed877","line":489,"range":{"start_line":489,"start_character":8,"end_line":489,"end_character":72},"in_reply_to":"a0e9d301_bde3f2dc","updated":"2025-10-14 14:25:01.000000000","message":"I opened https://bugs.launchpad.net/ovsdbapp/+bug/2127861 but because I changed the egress routing from static route to a policy we no longer need this.","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"48b757d2fb236543292cecd75131286fa8d19ce6","unresolved":true,"context_lines":[{"line_number":497,"context_line":"        main_router_lrp_ip \u003d helpers.InternalIpManager.get_ip("},{"line_number":498,"context_line":"            self.chassis_index, constants.LRP_MAIN_ROUTER_TO_CHASSIS)"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        match\u003df\u0027inport\u003d\u003d\\\"{get_lrp_name(self.router_name, self.switch_name)}\\\"\u0027"},{"line_number":501,"context_line":"        nb_cmd.LrPolicyAddCommand("},{"line_number":502,"context_line":"            self.api,"},{"line_number":503,"context_line":"            self.router_name,"}],"source_content_type":"text/x-python","patch_set":8,"id":"073056c1_7977879e","line":500,"updated":"2025-10-14 08:00:22.000000000","message":"Just a question: if I\u0027m not wrong, this policy (that is defined in the local LR) is the counterpart of [1], so when traffic (external) coming from br-eth0/br-eth1 (according to the spec diagram [2]), that will be routed back to the distributed LR. Is that correct?\n\n[1]https://review.opendev.org/c/openstack/neutron-specs/+/952872/12/specs/2025.2/ovn-bgp-integration.rst#102\n[2]https://review.opendev.org/c/openstack/neutron-specs/+/952872/12/images/ovn-bgp-topology.jpg","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cce4fa3daeb77e17cbce090535b37fd96db45a04","unresolved":true,"context_lines":[{"line_number":497,"context_line":"        main_router_lrp_ip \u003d helpers.InternalIpManager.get_ip("},{"line_number":498,"context_line":"            self.chassis_index, constants.LRP_MAIN_ROUTER_TO_CHASSIS)"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        match\u003df\u0027inport\u003d\u003d\\\"{get_lrp_name(self.router_name, self.switch_name)}\\\"\u0027"},{"line_number":501,"context_line":"        nb_cmd.LrPolicyAddCommand("},{"line_number":502,"context_line":"            self.api,"},{"line_number":503,"context_line":"            self.router_name,"}],"source_content_type":"text/x-python","patch_set":8,"id":"6c94070b_b3d136c2","line":500,"in_reply_to":"073056c1_7977879e","updated":"2025-10-14 14:25:01.000000000","message":"no, this is the ChassisPeerCommand - so that is what is on [2] in the chassis between the eth NIC and the per-chassis-router. I realized we can route the traffic based on the inport with a policy instead of the destination subnet prefix and a static route, so this is a bit different to the spec. That is also why we no longer need the workaround for the ovsdbapp bug.","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a7bff7a7f265f1e5bf1d9eda6e5d2e3f3c2237ba","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        main_router_lrp_ip \u003d helpers.InternalIpManager.get_ip("},{"line_number":498,"context_line":"            self.chassis_index, constants.LRP_MAIN_ROUTER_TO_CHASSIS)"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        match\u003df\u0027inport\u003d\u003d\\\"{get_lrp_name(self.router_name, self.switch_name)}\\\"\u0027"},{"line_number":501,"context_line":"        nb_cmd.LrPolicyAddCommand("},{"line_number":502,"context_line":"            self.api,"},{"line_number":503,"context_line":"            self.router_name,"}],"source_content_type":"text/x-python","patch_set":8,"id":"a9d974f5_c1d8ad08","line":500,"in_reply_to":"6c94070b_b3d136c2","updated":"2025-10-14 14:59:05.000000000","message":"Done","commit_id":"5db1a4ed3fd64b150285f4b3a6a671fd3a5a00c0"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9238230cd3a503c2545c7ad0bff517570dc6baf5","unresolved":true,"context_lines":[{"line_number":122,"context_line":"    def run_idl(self, txn):"},{"line_number":123,"context_line":"        super().run_idl(txn)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        lsp \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.localnet_lsp_name)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        columns \u003d {"},{"line_number":128,"context_line":"            \u0027type\u0027: ovn_const.LSP_TYPE_LOCALNET,"}],"source_content_type":"text/x-python","patch_set":14,"id":"b2402523_2671de63","line":125,"updated":"2025-12-15 22:24:53.000000000","message":"if the super().run_idl() set the uuid in self.result, it seems like you could do the lookup() with that, which would just be a dict lookup instead of scanning for the name(which is probably indexed so it\u0027s not really that bad).","commit_id":"a5f18ca278b4f3c6d405f09270d61fe846e9fc90"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dd574332f3df398cfa9b540e8230b37f7517de4f","unresolved":false,"context_lines":[{"line_number":122,"context_line":"    def run_idl(self, txn):"},{"line_number":123,"context_line":"        super().run_idl(txn)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        lsp \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.localnet_lsp_name)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        columns \u003d {"},{"line_number":128,"context_line":"            \u0027type\u0027: ovn_const.LSP_TYPE_LOCALNET,"}],"source_content_type":"text/x-python","patch_set":14,"id":"d68a6e5e_4814a4e9","line":125,"in_reply_to":"b2402523_2671de63","updated":"2025-12-16 13:56:20.000000000","message":"Actually we already have the lsp row in the result at this point from the `super().run_idl()`.","commit_id":"a5f18ca278b4f3c6d405f09270d61fe846e9fc90"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9238230cd3a503c2545c7ad0bff517570dc6baf5","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":69,"context_line":"        except idlutils.RowNotFound:"},{"line_number":70,"context_line":"            super().run_idl(txn)"},{"line_number":71,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.set_columns(self.result, **self.columns)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"b798783d_9470e1bd","line":71,"updated":"2025-12-15 22:24:53.000000000","message":"I\u0027m having trouble determining if doing a lookup here breaks things--it\u0027s certainly different to what AddCommand.post_commit expects, which is that if we create the row in this txn, self.result will be the \"insert uuid\" which will then, post commit change, and then we look up the actual commited row and return it. If result is already a Row, we just return that Row instead, which seems like it would be the uncommitted row. But I\u0027ll need to do some playing around to see what actually happens in this case.","commit_id":"0cf7ffb6e398d4dfa90369464137a6793916d7ff"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"399321824079216104e8efdfba6894ec07504f6c","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":69,"context_line":"        except idlutils.RowNotFound:"},{"line_number":70,"context_line":"            super().run_idl(txn)"},{"line_number":71,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.set_columns(self.result, **self.columns)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"aef24678_cacbd5bc","line":71,"in_reply_to":"206da355_f9a702f1","updated":"2025-12-19 14:59:49.000000000","message":"To test that the result is not the correct row result, I did\n```\ndiff --git a/neutron/tests/functional/services/bgp/test_commands.py b/neutron/tests/functional/services/bgp/test_commands.py\nindex dd02882233..adc4d8757f 100644\n--- a/neutron/tests/functional/services/bgp/test_commands.py\n+++ b/neutron/tests/functional/services/bgp/test_commands.py\n@@ -57,7 +57,10 @@ class _AddBaseCommand:\n             return None\n \n     def create_row(self, name, **kwargs):\n-        self.command(self.nb_api, name, **kwargs).execute(check_error\u003dTrue)\n+        cmd \u003d self.command(self.nb_api, name, **kwargs)\n+        cmd.execute(check_error\u003dTrue)\n+        result \u003d self.nb_api.lookup(self.table, name)\n+        self.assertEqual(result.uuid, cmd.result.uuid)```\n\nand did get failures like\n```\nneutron.tests.functional.services.bgp.test_commands.LrAddCommandTestCase.test_create_new_row\n--------------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n\n      File \"/opt/stack/neutron/neutron/tests/base.py\", line 176, in func\n    return f(self, *args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^\n\n      File \"/opt/stack/neutron/neutron/tests/functional/services/bgp/test_commands.py\", line 71, in test_create_new_row\n    self.create_row(name)\n\n      File \"/opt/stack/neutron/neutron/tests/functional/services/bgp/test_commands.py\", line 63, in create_row\n    self.assertEqual(result.uuid, cmd.result.uuid)\n\n      File \"/opt/stack/neutron/.tox/dsvm-functional/lib/python3.12/site-packages/testtools/testcase.py\", line 419, in assertEqual\n    self.assertThat(observed, matcher, message)\n\n      File \"/opt/stack/neutron/.tox/dsvm-functional/lib/python3.12/site-packages/testtools/testcase.py\", line 509, in assertThat\n    raise mismatch_error\n\n    testtools.matchers._impl.MismatchError: !\u003d:\nreference \u003d UUID(\u002730275275-cb67-4b5e-811f-c31716eb6559\u0027)\nactual    \u003d UUID(\u002716844319-1f5e-4aef-b4ba-e06922f9aff9\u0027)```\n\na fix could be something like\n```\n@ -54,12 +54,13 @@ class _LrAddCommand(nb_cmd.LrAddCommand):\n     \"\"\"\n     def run_idl(self, txn):\n         try:\n-            self.result \u003d self.api.lookup(\u0027Logical_Router\u0027, self.router)\n+            row \u003d self.result \u003d self.api.lookup(\u0027Logical_Router\u0027, self.router)\n         except idlutils.RowNotFound:\n             super().run_idl(txn)\n-            self.result \u003d self.api.lookup(\u0027Logical_Router\u0027, self.router)\n+            row \u003d self.api.lookup(\u0027Logical_Router\u0027, self.router)\n+            self.result \u003d row.uuid\n \n-        self.set_columns(self.result, **self.columns)\n+        self.set_columns(row, **self.columns)```\n\nfor each of the places this pattern exists.","commit_id":"0cf7ffb6e398d4dfa90369464137a6793916d7ff"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"69bfd0084c5c39ef0d40480d240a3e2571a24a58","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":69,"context_line":"        except idlutils.RowNotFound:"},{"line_number":70,"context_line":"            super().run_idl(txn)"},{"line_number":71,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.set_columns(self.result, **self.columns)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"206da355_f9a702f1","line":71,"in_reply_to":"254c0fdb_92f1e713","updated":"2025-12-16 14:02:18.000000000","message":"Eventually I can set a different attribute for the row that I use later and let the result being a uuid so post_commit can pick it up","commit_id":"0cf7ffb6e398d4dfa90369464137a6793916d7ff"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"3fb9426046e1fd07b5c3df844e2f0e10a0e11f81","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":69,"context_line":"        except idlutils.RowNotFound:"},{"line_number":70,"context_line":"            super().run_idl(txn)"},{"line_number":71,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.set_columns(self.result, **self.columns)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"e8b8fc8a_231a220f","line":71,"in_reply_to":"aef24678_cacbd5bc","updated":"2025-12-19 16:49:43.000000000","message":"Just noticed that set_columns() not needed in the RowNotFound case, and we could just do the super() and leave it at that and do the set_columns in the initial try: block.","commit_id":"0cf7ffb6e398d4dfa90369464137a6793916d7ff"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dd574332f3df398cfa9b540e8230b37f7517de4f","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":69,"context_line":"        except idlutils.RowNotFound:"},{"line_number":70,"context_line":"            super().run_idl(txn)"},{"line_number":71,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.set_columns(self.result, **self.columns)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"254c0fdb_92f1e713","line":71,"in_reply_to":"b798783d_9470e1bd","updated":"2025-12-16 13:56:20.000000000","message":"Thank you! This code pattern is here multiple times so it would be great to know if there was an issue :)","commit_id":"0cf7ffb6e398d4dfa90369464137a6793916d7ff"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ecc7768512337429e625a384eb45556075e16c3c","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":69,"context_line":"        except idlutils.RowNotFound:"},{"line_number":70,"context_line":"            super().run_idl(txn)"},{"line_number":71,"context_line":"            self.result \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.port)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.set_columns(self.result, **self.columns)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"825f8891_ea444a8e","line":71,"in_reply_to":"e8b8fc8a_231a220f","updated":"2025-12-19 17:31:08.000000000","message":"I changed the new code added in this patch and for the rest pushed https://review.opendev.org/c/openstack/neutron/+/971561","commit_id":"0cf7ffb6e398d4dfa90369464137a6793916d7ff"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea2a632285f3f3dfdacefc1e71afabe76fd5fab0","unresolved":true,"context_lines":[{"line_number":118,"context_line":"        self.result \u003d hcg.uuid"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"class _LrPolicyAddCommand(nb_cmd.LrPolicyAddCommand):"},{"line_number":122,"context_line":"    \"\"\"An idempotent command to add a logical router policy."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    We need to bypass the validation of output_port when action is reroute"}],"source_content_type":"text/x-python","patch_set":18,"id":"000d884a_04c9812d","line":121,"updated":"2025-12-18 20:13:57.000000000","message":"No issue, I am just wondering why not update `LrPolicyAddCommand.__init__` to handle this scenario?","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e1121130d85c4369367a13842d447fbbfb8a082","unresolved":true,"context_lines":[{"line_number":118,"context_line":"        self.result \u003d hcg.uuid"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"class _LrPolicyAddCommand(nb_cmd.LrPolicyAddCommand):"},{"line_number":122,"context_line":"    \"\"\"An idempotent command to add a logical router policy."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    We need to bypass the validation of output_port when action is reroute"}],"source_content_type":"text/x-python","patch_set":18,"id":"cdb9a425_5f4f25ae","line":121,"in_reply_to":"000d884a_04c9812d","updated":"2025-12-18 21:32:11.000000000","message":"The ovsdbapp commands are more like a reference implementation and the projects importing ovsdbapp shall write their own tailored commands. We could implement it there as well, however we\u0027d still need some workaround until there is a new ovsdbapp release.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"594b0bba36ca506e4b0fbaa56d1ee4f4349ba79f","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        self.result \u003d hcg.uuid"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"class _LrPolicyAddCommand(nb_cmd.LrPolicyAddCommand):"},{"line_number":122,"context_line":"    \"\"\"An idempotent command to add a logical router policy."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    We need to bypass the validation of output_port when action is reroute"}],"source_content_type":"text/x-python","patch_set":18,"id":"a3cd220c_f956459c","line":121,"in_reply_to":"cdb9a425_5f4f25ae","updated":"2025-12-22 19:08:57.000000000","message":"Acknowledged","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea2a632285f3f3dfdacefc1e71afabe76fd5fab0","unresolved":true,"context_lines":[{"line_number":403,"context_line":"            self.chassis_router_name,"},{"line_number":404,"context_line":"            priority\u003d10,"},{"line_number":405,"context_line":"            match\u003dmatch,"},{"line_number":406,"context_line":"            action\u003d\u0027reroute\u0027,"},{"line_number":407,"context_line":"            may_exist\u003dTrue,"},{"line_number":408,"context_line":"            output_port\u003dlrp,"},{"line_number":409,"context_line":"        ).run_idl(txn)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bf3fb490_47362f72","line":406,"updated":"2025-12-18 20:13:57.000000000","message":"Use already imported ovsdbapp constant\n`const.POLICY_ACTION_REROUTE`","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e1121130d85c4369367a13842d447fbbfb8a082","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            self.chassis_router_name,"},{"line_number":404,"context_line":"            priority\u003d10,"},{"line_number":405,"context_line":"            match\u003dmatch,"},{"line_number":406,"context_line":"            action\u003d\u0027reroute\u0027,"},{"line_number":407,"context_line":"            may_exist\u003dTrue,"},{"line_number":408,"context_line":"            output_port\u003dlrp,"},{"line_number":409,"context_line":"        ).run_idl(txn)"}],"source_content_type":"text/x-python","patch_set":18,"id":"093bdb18_adfcc469","line":406,"in_reply_to":"bf3fb490_47362f72","updated":"2025-12-18 21:32:11.000000000","message":"Done","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5abea835a99e029f8aaf091e6240f6aa1d497103","unresolved":true,"context_lines":[{"line_number":127,"context_line":"    \"\"\""},{"line_number":128,"context_line":"    def __init__(self, api, router, priority, match, action, *args, **kwargs):"},{"line_number":129,"context_line":"        nexthop \u003d kwargs.get(\u0027nexthop\u0027)"},{"line_number":130,"context_line":"        fake_nexthop \u003d object()"},{"line_number":131,"context_line":"        if (action \u003d\u003d ovsdbapp_const.POLICY_ACTION_REROUTE and"},{"line_number":132,"context_line":"                \u0027output_port\u0027 in kwargs):"},{"line_number":133,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"b65b3996_e145332e","line":130,"updated":"2025-12-19 15:41:00.000000000","message":"Reading the ovn-nbctl code for `nbctl_lr_policy_add` it looks like output_port requires a single nexthop (and also validates that the LRP for output-port exists). I don\u0027t have any experience with the actual usage of this, but here\u0027s the code:\n\n```\nif (reroute) {\n    ...\n    if (output_port) {\n        if (vector_len(\u0026nexthops) !\u003d 1) { \n            ctl_error(ctx, \"output-port option is supported only for \"\n                           \"policies with a single reroute next hop\");\n            goto free_nexthops;\n        }\n        \n        error \u003d lrp_by_name_or_uuid(ctx, output_port, true, \u0026out_lrp);\n        if (error) {\n            ctl_error(ctx, \"unknown output-port: %s\", output_port);\n            free(error);\n            goto free_nexthops;\n        }\n    }\n    ...\n}\n```\n\nSo do we need a fake nexthop if ovn-nbctl at least thinks that one is required even if output_port is set?","commit_id":"a44ab9fd5c104f5fdbac22e6a2204c0b5eb1d996"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dd0026816d3a4b66178a1822b2581863359bfd85","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    \"\"\""},{"line_number":128,"context_line":"    def __init__(self, api, router, priority, match, action, *args, **kwargs):"},{"line_number":129,"context_line":"        nexthop \u003d kwargs.get(\u0027nexthop\u0027)"},{"line_number":130,"context_line":"        fake_nexthop \u003d object()"},{"line_number":131,"context_line":"        if (action \u003d\u003d ovsdbapp_const.POLICY_ACTION_REROUTE and"},{"line_number":132,"context_line":"                \u0027output_port\u0027 in kwargs):"},{"line_number":133,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"d2a0001a_90b6c148","line":130,"in_reply_to":"488bae67_648908ef","updated":"2025-12-22 16:29:32.000000000","message":"So far OVN requires also nexthops even if output_port is defined.","commit_id":"a44ab9fd5c104f5fdbac22e6a2204c0b5eb1d996"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ecc7768512337429e625a384eb45556075e16c3c","unresolved":true,"context_lines":[{"line_number":127,"context_line":"    \"\"\""},{"line_number":128,"context_line":"    def __init__(self, api, router, priority, match, action, *args, **kwargs):"},{"line_number":129,"context_line":"        nexthop \u003d kwargs.get(\u0027nexthop\u0027)"},{"line_number":130,"context_line":"        fake_nexthop \u003d object()"},{"line_number":131,"context_line":"        if (action \u003d\u003d ovsdbapp_const.POLICY_ACTION_REROUTE and"},{"line_number":132,"context_line":"                \u0027output_port\u0027 in kwargs):"},{"line_number":133,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"488bae67_648908ef","line":130,"in_reply_to":"b65b3996_e145332e","updated":"2025-12-19 17:31:08.000000000","message":"I\u0027ll figure that out. I thought we do not need the IP as that was only to determine the output_port but maybe it\u0027s needed for easier mac lookup.","commit_id":"a44ab9fd5c104f5fdbac22e6a2204c0b5eb1d996"}],"neutron/services/bgp/constants.py":[{"author":{"_account_id":32288,"name":"Martin Kalcok","email":"martin.kalcok@canonical.com","username":"martin.kalcok"},"change_message_id":"a79d9f7a13c131e8c7adfe68ab707d74c5dae90f","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"BGP_ROUTER_REDISTRIBUTE \u003d \u0027connected-as-host,nat\u0027"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CHASSIS_PEER_CONNECTIONS \u003d \u0027bgp-peer-connections\u0027"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"BGP_CHASSIS_NETWORK_NAME \u003d \u0027bgp-chassis-network-name\u0027"},{"line_number":27,"context_line":"BGP_CHASSIS_LRP_MAC \u003d \u0027bgp-chassis-lrp-mac\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"c5cfa5e6_e8eadca5","line":27,"range":{"start_line":24,"start_character":0,"end_line":27,"end_character":43},"updated":"2025-11-14 18:12:48.000000000","message":"Since these constants are used as keys in `external_ids`, would it make sense to prefix them with `neutron-`, to avoid potential clashing in the future either with OVN itself? Or would it make the options too verbose with little benefit?","commit_id":"61720cd29410d8c102092c2266cf3f50e4893e10"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c256030f52a7481745edf54c0c529ca66cfb3548","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"BGP_ROUTER_REDISTRIBUTE \u003d \u0027connected-as-host,nat\u0027"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CHASSIS_PEER_CONNECTIONS \u003d \u0027bgp-peer-connections\u0027"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"BGP_CHASSIS_NETWORK_NAME \u003d \u0027bgp-chassis-network-name\u0027"},{"line_number":27,"context_line":"BGP_CHASSIS_LRP_MAC \u003d \u0027bgp-chassis-lrp-mac\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"95f3093e_79c5bd6d","line":27,"range":{"start_line":24,"start_character":0,"end_line":27,"end_character":43},"in_reply_to":"372c67db_d86df833","updated":"2026-01-02 19:16:23.000000000","message":"Done","commit_id":"61720cd29410d8c102092c2266cf3f50e4893e10"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e1cc871d452cdf9ec4ee7241886faeb67243bd45","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"BGP_ROUTER_REDISTRIBUTE \u003d \u0027connected-as-host,nat\u0027"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CHASSIS_PEER_CONNECTIONS \u003d \u0027bgp-peer-connections\u0027"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"BGP_CHASSIS_NETWORK_NAME \u003d \u0027bgp-chassis-network-name\u0027"},{"line_number":27,"context_line":"BGP_CHASSIS_LRP_MAC \u003d \u0027bgp-chassis-lrp-mac\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"372c67db_d86df833","line":27,"range":{"start_line":24,"start_character":0,"end_line":27,"end_character":43},"in_reply_to":"c5cfa5e6_e8eadca5","updated":"2025-11-17 19:04:36.000000000","message":"I\u0027d consider the `neutron-` prefix being from \"the Neutron world\", so Neutron manages those, while `bgp-` is managed by the `ovn-bgp` extension. That said the syncs and maintenance should not touch resources tagged as such.","commit_id":"61720cd29410d8c102092c2266cf3f50e4893e10"}],"neutron/services/bgp/helpers.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9360987258200c37576fe3c41f39c4c918b7cbe7","unresolved":true,"context_lines":[{"line_number":53,"context_line":"        LOG.warning(\"Chassis %s has no BGP bridges set\", chassis.name)"},{"line_number":54,"context_line":"        return []"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return [bridge.strip() for bridge in bgp_bridges.split(\u0027,\u0027)"},{"line_number":57,"context_line":"            if bridge.strip()]"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5e7bf4a2_a058d428","line":56,"updated":"2025-12-29 18:01:41.000000000","message":"nit: If you wanted to avoid calling strip() twice, we now require \u003e Python 3.8 so we could use the walrus operator :\u003d here. e.g.\n```\nreturn [bridge for s in bgp_bridges.split(\u0027,\u0027) if (bridge :\u003d s.strip())]\n```\nSince I know you like the useful features that aren\u0027t often used. :) Granted, it might confuse people who are not familiar with it.","commit_id":"a2c318e344b1b515fcc88e74cea593303684462d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"754b638ae4e22b47973a873c19021ce5e875c2e1","unresolved":true,"context_lines":[{"line_number":53,"context_line":"        LOG.warning(\"Chassis %s has no BGP bridges set\", chassis.name)"},{"line_number":54,"context_line":"        return []"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return [bridge.strip() for bridge in bgp_bridges.split(\u0027,\u0027)"},{"line_number":57,"context_line":"            if bridge.strip()]"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"e44d1c79_a7f6e679","line":56,"in_reply_to":"5e7bf4a2_a058d428","updated":"2026-01-02 13:40:05.000000000","message":"Yes, this is a perfect place for it! Thanks!","commit_id":"a2c318e344b1b515fcc88e74cea593303684462d"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ee81e3103d9cf04851878b071ecccada4e2058de","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        LOG.warning(\"Chassis %s has no BGP bridges set\", chassis.name)"},{"line_number":54,"context_line":"        return []"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return [bridge.strip() for bridge in bgp_bridges.split(\u0027,\u0027)"},{"line_number":57,"context_line":"            if bridge.strip()]"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"4df90e31_d29ff5ea","line":56,"in_reply_to":"87d19fd9_3664d293","updated":"2026-01-02 14:49:53.000000000","message":"Done","commit_id":"a2c318e344b1b515fcc88e74cea593303684462d"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"93f930796c6ee9ca69a7b097a23e3243b01939a6","unresolved":true,"context_lines":[{"line_number":53,"context_line":"        LOG.warning(\"Chassis %s has no BGP bridges set\", chassis.name)"},{"line_number":54,"context_line":"        return []"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return [bridge.strip() for bridge in bgp_bridges.split(\u0027,\u0027)"},{"line_number":57,"context_line":"            if bridge.strip()]"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"87d19fd9_3664d293","line":56,"in_reply_to":"e44d1c79_a7f6e679","updated":"2026-01-02 14:49:30.000000000","message":"Thanks Terry, that is neat!","commit_id":"a2c318e344b1b515fcc88e74cea593303684462d"}],"neutron/tests/functional/services/bgp/test_commands.py":[{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea2a632285f3f3dfdacefc1e71afabe76fd5fab0","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":143,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-1\u0027)"},{"line_number":144,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value1\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        # Should update the options"},{"line_number":147,"context_line":"        self.create_row(name, options\u003d{\u0027peer-port\u0027: \u0027lsp-peer-2\u0027},"},{"line_number":148,"context_line":"                        external_ids\u003d{\u0027id1\u0027: \u0027value2\u0027})"},{"line_number":149,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":150,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-2\u0027)"},{"line_number":151,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value2\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class LrpAddCommandTestCase(bgp.BaseBgpNbIdlTestCase, _AddBaseCommand):"}],"source_content_type":"text/x-python","patch_set":18,"id":"d45024fc_d3b90ecb","line":151,"range":{"start_line":145,"start_character":0,"end_line":151,"end_character":63},"updated":"2025-12-18 20:13:57.000000000","message":"nit: Feels like it should be its own test?","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cab848b5ec520405fb0883269d612b6bd387bddc","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":143,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-1\u0027)"},{"line_number":144,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value1\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        # Should update the options"},{"line_number":147,"context_line":"        self.create_row(name, options\u003d{\u0027peer-port\u0027: \u0027lsp-peer-2\u0027},"},{"line_number":148,"context_line":"                        external_ids\u003d{\u0027id1\u0027: \u0027value2\u0027})"},{"line_number":149,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":150,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-2\u0027)"},{"line_number":151,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value2\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class LrpAddCommandTestCase(bgp.BaseBgpNbIdlTestCase, _AddBaseCommand):"}],"source_content_type":"text/x-python","patch_set":18,"id":"b18f5898_e5e5b6bb","line":151,"range":{"start_line":145,"start_character":0,"end_line":151,"end_character":63},"in_reply_to":"12f9a9fe_40e92b54","updated":"2025-12-22 15:43:18.000000000","message":"What I meant is that it should be in it own function i.e. `test_should_update_existing_attributes`","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"594b0bba36ca506e4b0fbaa56d1ee4f4349ba79f","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":143,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-1\u0027)"},{"line_number":144,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value1\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        # Should update the options"},{"line_number":147,"context_line":"        self.create_row(name, options\u003d{\u0027peer-port\u0027: \u0027lsp-peer-2\u0027},"},{"line_number":148,"context_line":"                        external_ids\u003d{\u0027id1\u0027: \u0027value2\u0027})"},{"line_number":149,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":150,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-2\u0027)"},{"line_number":151,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value2\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class LrpAddCommandTestCase(bgp.BaseBgpNbIdlTestCase, _AddBaseCommand):"}],"source_content_type":"text/x-python","patch_set":18,"id":"ce80eb1a_64a11f65","line":151,"range":{"start_line":145,"start_character":0,"end_line":151,"end_character":63},"in_reply_to":"5cebc094_e75d909a","updated":"2025-12-22 19:08:57.000000000","message":"No I was thinking two tests, one you have and then another I suggested. But that is probably overkill. All good(different day different feelings 😊 )","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"164a293d8473cdb56a8fbb9b629b01c6287a2d83","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":143,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-1\u0027)"},{"line_number":144,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value1\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        # Should update the options"},{"line_number":147,"context_line":"        self.create_row(name, options\u003d{\u0027peer-port\u0027: \u0027lsp-peer-2\u0027},"},{"line_number":148,"context_line":"                        external_ids\u003d{\u0027id1\u0027: \u0027value2\u0027})"},{"line_number":149,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":150,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-2\u0027)"},{"line_number":151,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value2\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class LrpAddCommandTestCase(bgp.BaseBgpNbIdlTestCase, _AddBaseCommand):"}],"source_content_type":"text/x-python","patch_set":18,"id":"5cebc094_e75d909a","line":151,"range":{"start_line":145,"start_character":0,"end_line":151,"end_character":63},"in_reply_to":"b18f5898_e5e5b6bb","updated":"2025-12-22 16:50:09.000000000","message":"So you want me to rename the test name?","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e1121130d85c4369367a13842d447fbbfb8a082","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":143,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-1\u0027)"},{"line_number":144,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value1\u0027)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        # Should update the options"},{"line_number":147,"context_line":"        self.create_row(name, options\u003d{\u0027peer-port\u0027: \u0027lsp-peer-2\u0027},"},{"line_number":148,"context_line":"                        external_ids\u003d{\u0027id1\u0027: \u0027value2\u0027})"},{"line_number":149,"context_line":"        lsp \u003d self._assert_table_row_exists(name)"},{"line_number":150,"context_line":"        self.assertEqual(lsp.options.get(\u0027peer-port\u0027), \u0027lsp-peer-2\u0027)"},{"line_number":151,"context_line":"        self.assertEqual(lsp.external_ids.get(\u0027id1\u0027), \u0027value2\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class LrpAddCommandTestCase(bgp.BaseBgpNbIdlTestCase, _AddBaseCommand):"}],"source_content_type":"text/x-python","patch_set":18,"id":"12f9a9fe_40e92b54","line":151,"range":{"start_line":145,"start_character":0,"end_line":151,"end_character":63},"in_reply_to":"d45024fc_d3b90ecb","updated":"2025-12-18 21:32:11.000000000","message":"it is :) This test creates an LSP and then calls the command that should update the attributes.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea2a632285f3f3dfdacefc1e71afabe76fd5fab0","unresolved":true,"context_lines":[{"line_number":698,"context_line":"        self.nb_api.lsp_add("},{"line_number":699,"context_line":"            self.ls_name, lsp_name,"},{"line_number":700,"context_line":"            type\u003d\u0027patch\u0027,  # wrong type"},{"line_number":701,"context_line":"            addresses\u003d[\u002700:00:00:00:00:01\u0027],  # wrong addresses"},{"line_number":702,"context_line":"            options\u003d{\u0027peer\u0027: \u0027wrong-peer\u0027}  # wrong options"},{"line_number":703,"context_line":"        ).execute(check_error\u003dTrue)"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        # Execute command should fix attributes"}],"source_content_type":"text/x-python","patch_set":18,"id":"cf257f5c_c7e1dcf9","line":702,"range":{"start_line":701,"start_character":47,"end_line":702,"end_character":59},"updated":"2025-12-18 20:13:57.000000000","message":"Who this? Claude? 😊\nSometimes I feel like claude over comments everything. It is clear by looking at values they are not expected.. Also lot of obvious comments in these unit tests. No issue...","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cab848b5ec520405fb0883269d612b6bd387bddc","unresolved":false,"context_lines":[{"line_number":698,"context_line":"        self.nb_api.lsp_add("},{"line_number":699,"context_line":"            self.ls_name, lsp_name,"},{"line_number":700,"context_line":"            type\u003d\u0027patch\u0027,  # wrong type"},{"line_number":701,"context_line":"            addresses\u003d[\u002700:00:00:00:00:01\u0027],  # wrong addresses"},{"line_number":702,"context_line":"            options\u003d{\u0027peer\u0027: \u0027wrong-peer\u0027}  # wrong options"},{"line_number":703,"context_line":"        ).execute(check_error\u003dTrue)"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        # Execute command should fix attributes"}],"source_content_type":"text/x-python","patch_set":18,"id":"0acd0fef_53cbe838","line":702,"range":{"start_line":701,"start_character":47,"end_line":702,"end_character":59},"in_reply_to":"112ecf48_bc667298","updated":"2025-12-22 15:43:18.000000000","message":"Sure, no issue.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e1121130d85c4369367a13842d447fbbfb8a082","unresolved":false,"context_lines":[{"line_number":698,"context_line":"        self.nb_api.lsp_add("},{"line_number":699,"context_line":"            self.ls_name, lsp_name,"},{"line_number":700,"context_line":"            type\u003d\u0027patch\u0027,  # wrong type"},{"line_number":701,"context_line":"            addresses\u003d[\u002700:00:00:00:00:01\u0027],  # wrong addresses"},{"line_number":702,"context_line":"            options\u003d{\u0027peer\u0027: \u0027wrong-peer\u0027}  # wrong options"},{"line_number":703,"context_line":"        ).execute(check_error\u003dTrue)"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        # Execute command should fix attributes"}],"source_content_type":"text/x-python","patch_set":18,"id":"112ecf48_bc667298","line":702,"range":{"start_line":701,"start_character":47,"end_line":702,"end_character":59},"in_reply_to":"cf257f5c_c7e1dcf9","updated":"2025-12-18 21:32:11.000000000","message":"Now I can\u0027t remember if I wrote it there or not but it is possible this was written manually by hand. I can remove the comments but I thought it was not obvious, maybe \"wrong-peer\" but the MACs, types and addresses were not.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea2a632285f3f3dfdacefc1e71afabe76fd5fab0","unresolved":true,"context_lines":[{"line_number":793,"context_line":"                                              self.main_router_name)"},{"line_number":794,"context_line":"            inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)"},{"line_number":795,"context_line":"            if (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and"},{"line_number":796,"context_line":"                    policy.action \u003d\u003d \u0027reroute\u0027 and"},{"line_number":797,"context_line":"                    policy.output_port[0].name \u003d\u003d output_lrp):"},{"line_number":798,"context_line":"                return True"},{"line_number":799,"context_line":"            return False"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"5c5aae30_3e862e01","line":797,"range":{"start_line":796,"start_character":0,"end_line":797,"end_character":62},"updated":"2025-12-18 20:13:57.000000000","message":"Wouldn\u0027t be better to assert to know what exactly failed if this the test fails, instead of just overall True and False?","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5e9c5d032dc7d466a603f019d0e0b64043249225","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                                              self.main_router_name)"},{"line_number":794,"context_line":"            inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)"},{"line_number":795,"context_line":"            if (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and"},{"line_number":796,"context_line":"                    policy.action \u003d\u003d \u0027reroute\u0027 and"},{"line_number":797,"context_line":"                    policy.output_port[0].name \u003d\u003d output_lrp):"},{"line_number":798,"context_line":"                return True"},{"line_number":799,"context_line":"            return False"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9e50eff3_62ef6688","line":797,"range":{"start_line":796,"start_character":0,"end_line":797,"end_character":62},"in_reply_to":"18086cd5_c905c7fd","updated":"2025-12-22 19:20:44.000000000","message":"Done","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e1121130d85c4369367a13842d447fbbfb8a082","unresolved":true,"context_lines":[{"line_number":793,"context_line":"                                              self.main_router_name)"},{"line_number":794,"context_line":"            inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)"},{"line_number":795,"context_line":"            if (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and"},{"line_number":796,"context_line":"                    policy.action \u003d\u003d \u0027reroute\u0027 and"},{"line_number":797,"context_line":"                    policy.output_port[0].name \u003d\u003d output_lrp):"},{"line_number":798,"context_line":"                return True"},{"line_number":799,"context_line":"            return False"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"836590ea_0f5b3746","line":797,"range":{"start_line":796,"start_character":0,"end_line":797,"end_character":62},"in_reply_to":"5c5aae30_3e862e01","updated":"2025-12-18 21:32:11.000000000","message":"Could you please write a code snippet?\n\nThe goal here is to test that based on the provided `peers` we have all policies on the router.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cab848b5ec520405fb0883269d612b6bd387bddc","unresolved":true,"context_lines":[{"line_number":793,"context_line":"                                              self.main_router_name)"},{"line_number":794,"context_line":"            inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)"},{"line_number":795,"context_line":"            if (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and"},{"line_number":796,"context_line":"                    policy.action \u003d\u003d \u0027reroute\u0027 and"},{"line_number":797,"context_line":"                    policy.output_port[0].name \u003d\u003d output_lrp):"},{"line_number":798,"context_line":"                return True"},{"line_number":799,"context_line":"            return False"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"a17d7b41_7cab3771","line":797,"range":{"start_line":796,"start_character":0,"end_line":797,"end_character":62},"in_reply_to":"836590ea_0f5b3746","updated":"2025-12-22 15:43:18.000000000","message":"What I meant is that instead of just failing with generic `Policy for peer {peer} not found\"` you could assert at exactly what was wrong.\nE.g.\n```\ndef _check_policy(policy, peer):\n    peer_switch_name \u003d helpers.get_chassis_peer_switch_name(\n        chassis_name, peer)\n    output_lrp \u003d helpers.get_lrp_name(router.name,\n                                      self.main_router_name)\n    inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)\n    self.assertEqual(policy.match, f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027, f\"Policy match failed for peer {peer}: {policy.match} !\u003d inport\u003d\u003d\\\"{inport_lrp}\\\"\")\n    self.assertEqual(policy.action, \u0027reroute\u0027, f\"Policy action is not \u0027reroute\u0027 for peer {peer}: {policy.action}\")\n    self.assertEqual(policy.output_port[0].name, output_lrp, f\"Policy output_port[0].name failed for peer {peer}: {policy.output_port[0].name} !\u003d {output_lrp}\")\n    ```","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"164a293d8473cdb56a8fbb9b629b01c6287a2d83","unresolved":true,"context_lines":[{"line_number":793,"context_line":"                                              self.main_router_name)"},{"line_number":794,"context_line":"            inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)"},{"line_number":795,"context_line":"            if (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and"},{"line_number":796,"context_line":"                    policy.action \u003d\u003d \u0027reroute\u0027 and"},{"line_number":797,"context_line":"                    policy.output_port[0].name \u003d\u003d output_lrp):"},{"line_number":798,"context_line":"                return True"},{"line_number":799,"context_line":"            return False"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"cfc881b5_15553d89","line":797,"range":{"start_line":796,"start_character":0,"end_line":797,"end_character":62},"in_reply_to":"a17d7b41_7cab3771","updated":"2025-12-22 16:50:09.000000000","message":"maybe it\u0027s the name that is confusing? If I renamed `_check_policy` to `is_policy_equal` then it would make more sense? Again, the goal is not to assert the attributes but to see if any of the policies that the router has is the one we\u0027re expecting.","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f0efefafa2e179a2525268564cc0d1cb83987166","unresolved":true,"context_lines":[{"line_number":793,"context_line":"                                              self.main_router_name)"},{"line_number":794,"context_line":"            inport_lrp \u003d helpers.get_lrp_name(router.name, peer_switch_name)"},{"line_number":795,"context_line":"            if (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and"},{"line_number":796,"context_line":"                    policy.action \u003d\u003d \u0027reroute\u0027 and"},{"line_number":797,"context_line":"                    policy.output_port[0].name \u003d\u003d output_lrp):"},{"line_number":798,"context_line":"                return True"},{"line_number":799,"context_line":"            return False"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"18086cd5_c905c7fd","line":797,"range":{"start_line":796,"start_character":0,"end_line":797,"end_character":62},"in_reply_to":"cfc881b5_15553d89","updated":"2025-12-22 19:03:39.000000000","message":"Ok I got it now, I think I overlooked the indent on the line which calls this function. I was reading it as `if ... else` but that is actually `for ... else` statement. So I am good with your code.\nOnly feedback I have left is to make the return statement more pythonic\n```\nreturn (policy.match \u003d\u003d f\u0027inport\u003d\u003d\\\"{inport_lrp}\\\"\u0027 and\n        policy.action \u003d\u003d \u0027reroute\u0027 and\n        policy.output_port[0].name \u003d\u003d output_lrp)\n```","commit_id":"3b4eef813f5cc16b8da911ea57357c3db37c3d56"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6b46714f688417bd3183baf8189823a4011ca46e","unresolved":true,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"class TestCaseWithOutputPort(bgp.BaseBgpNbIdlTestCase):"},{"line_number":109,"context_line":"    def _is_bgp_supported(self):"},{"line_number":110,"context_line":"        return \u0027output_port\u0027 in self.nb_api.idl.tables["},{"line_number":111,"context_line":"            \u0027Logical_Router_Policy\u0027].columns"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"2e2163c7_86c19d54","line":110,"updated":"2025-12-22 22:13:40.000000000","message":"nit: It looks like neutron requirements.txt has ovsdbapp 2.14.1 for ovsdbapp, which has had table_has_columns() since 2.13.0 from what I can tell. Though I see this is overriding an existing `is_bgp_supported()` that looks at a different table. Should the base `is_bgp_supported()` be empty/raise NotImplemented, and this and the sb-focused `is_bgp_supported()` be defined in the Nb/Sb base clases?","commit_id":"5ddcfc0995f708071a763b98d2e0dee841ec9411"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ef3733abe2e5e58ec3a9874390c7e67b7bfd643b","unresolved":true,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"class TestCaseWithOutputPort(bgp.BaseBgpNbIdlTestCase):"},{"line_number":109,"context_line":"    def _is_bgp_supported(self):"},{"line_number":110,"context_line":"        return \u0027output_port\u0027 in self.nb_api.idl.tables["},{"line_number":111,"context_line":"            \u0027Logical_Router_Policy\u0027].columns"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"d520f93b_4d5ed815","line":110,"in_reply_to":"2e2163c7_86c19d54","updated":"2025-12-22 23:27:48.000000000","message":"Thanks, good catch.\n\nI don\u0027t think the NB schema has any changes for BGP - except the specific keys in `options` that is picked by northd. So we need to use the SB schema to check if BGP is supported for NB tests.","commit_id":"5ddcfc0995f708071a763b98d2e0dee841ec9411"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9360987258200c37576fe3c41f39c4c918b7cbe7","unresolved":false,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"class TestCaseWithOutputPort(bgp.BaseBgpNbIdlTestCase):"},{"line_number":109,"context_line":"    def _is_bgp_supported(self):"},{"line_number":110,"context_line":"        return \u0027output_port\u0027 in self.nb_api.idl.tables["},{"line_number":111,"context_line":"            \u0027Logical_Router_Policy\u0027].columns"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"920740cc_708419de","line":110,"in_reply_to":"d520f93b_4d5ed815","updated":"2025-12-29 18:01:41.000000000","message":"Done","commit_id":"5ddcfc0995f708071a763b98d2e0dee841ec9411"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6b46714f688417bd3183baf8189823a4011ca46e","unresolved":true,"context_lines":[{"line_number":670,"context_line":"        self.assertEqual([\u0027router\u0027], lsp.addresses)"},{"line_number":671,"context_line":"        self.assertEqual(lrp_name, lsp.options.get(\u0027router-port\u0027))"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"    def test_connect_router_to_switch_with_ip(self):"},{"line_number":674,"context_line":"        lrp_ip \u003d \u0027192.168.1.1/24\u0027"},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        commands.ConnectRouterToSwitchCommand("}],"source_content_type":"text/x-python","patch_set":21,"id":"dd2b7478_e1733b77","line":673,"updated":"2025-12-22 22:13:40.000000000","message":"Just to double check, does this test not need the @bgp.requires_ovn_version_with_bgp()?","commit_id":"5ddcfc0995f708071a763b98d2e0dee841ec9411"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ef3733abe2e5e58ec3a9874390c7e67b7bfd643b","unresolved":true,"context_lines":[{"line_number":670,"context_line":"        self.assertEqual([\u0027router\u0027], lsp.addresses)"},{"line_number":671,"context_line":"        self.assertEqual(lrp_name, lsp.options.get(\u0027router-port\u0027))"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"    def test_connect_router_to_switch_with_ip(self):"},{"line_number":674,"context_line":"        lrp_ip \u003d \u0027192.168.1.1/24\u0027"},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        commands.ConnectRouterToSwitchCommand("}],"source_content_type":"text/x-python","patch_set":21,"id":"d548cbb8_8169b65b","line":673,"in_reply_to":"dd2b7478_e1733b77","updated":"2025-12-22 23:27:48.000000000","message":"This one does not as the actual patch in OVN it requires is the one that allows empty `networks` column in LRP - which was implemented with the BGP. This test configures the `lrp_ip` as the LRP IP.\n\nBut the base class is wrong as we don\u0027t use the output_port column here :)","commit_id":"5ddcfc0995f708071a763b98d2e0dee841ec9411"}]}