)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"59e2493067971f086dcdf8224a75f3b33d671bbd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4a0a7030_b5ab61f0","updated":"2023-08-08 16:05:10.000000000","message":"Just a quick question re: error handling, otherwise looks good to me.","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"5956694ba3f25872865dcd7f7a0d8c59f35efb98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2175a848_d9799afb","updated":"2023-08-10 09:01:55.000000000","message":"recheck","commit_id":"4693836a1b58b477298e51cb47622222e3556752"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c086eaae6ac70bb761911d813373c010e888ac2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"4b07c034_92cba3ac","updated":"2023-08-15 09:45:47.000000000","message":"recheck","commit_id":"4693836a1b58b477298e51cb47622222e3556752"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"68ad43db91273137cab74946fa56886409a5b062","unresolved":true,"context_lines":[{"line_number":473,"context_line":"                lrp_name).execute()"},{"line_number":474,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"        return db_lrport_nets !\u003d lrport_nets or db_lrport_ra !\u003d lrport_ra"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def sync_routers_and_rports(self, ctx):"},{"line_number":479,"context_line":"        \"\"\"Sync Routers between neutron and NB."}],"source_content_type":"text/x-python","patch_set":1,"id":"4fe68bc2_18454812","line":476,"range":{"start_line":476,"start_character":15,"end_line":476,"end_character":44},"updated":"2023-08-08 14:40:51.000000000","message":"I think you could put this check after L464 and skip some work,\n\nif db_lrport_nets !\u003d lrport_nets:\n    return True","commit_id":"8928e5c4f6f1036b0bc8770d0891e7d122a529f2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"81f6bfbb0eac9c552a952b431281ff93c1b1a851","unresolved":false,"context_lines":[{"line_number":473,"context_line":"                lrp_name).execute()"},{"line_number":474,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"        return db_lrport_nets !\u003d lrport_nets or db_lrport_ra !\u003d lrport_ra"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def sync_routers_and_rports(self, ctx):"},{"line_number":479,"context_line":"        \"\"\"Sync Routers between neutron and NB."}],"source_content_type":"text/x-python","patch_set":1,"id":"fcdb07eb_9dbab70c","line":476,"range":{"start_line":476,"start_character":15,"end_line":476,"end_character":44},"in_reply_to":"4fe68bc2_18454812","updated":"2023-08-08 15:04:53.000000000","message":"Good point, will do that","commit_id":"8928e5c4f6f1036b0bc8770d0891e7d122a529f2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"68ad43db91273137cab74946fa56886409a5b062","unresolved":true,"context_lines":[{"line_number":579,"context_line":"                        db_router_port \u003d db_router_ports[lrport]"},{"line_number":580,"context_line":"                        if self._is_router_port_changed(db_router_port,"},{"line_number":581,"context_line":"                                lrport_nets):"},{"line_number":582,"context_line":"                            update_lrport_list.append(db_router_ports[lrport])"},{"line_number":583,"context_line":"                        del db_router_ports[lrport]"},{"line_number":584,"context_line":"                    else:"},{"line_number":585,"context_line":"                        del_lrouter_ports_list.append("}],"source_content_type":"text/x-python","patch_set":1,"id":"80c0377d_461d4bf3","line":582,"range":{"start_line":582,"start_character":54,"end_line":582,"end_character":77},"updated":"2023-08-08 14:40:51.000000000","message":"I guess you can use db_router_port here as well","commit_id":"8928e5c4f6f1036b0bc8770d0891e7d122a529f2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2ca536024b782176daf6b4ce54a2bee0c7ffc9a9","unresolved":false,"context_lines":[{"line_number":579,"context_line":"                        db_router_port \u003d db_router_ports[lrport]"},{"line_number":580,"context_line":"                        if self._is_router_port_changed(db_router_port,"},{"line_number":581,"context_line":"                                lrport_nets):"},{"line_number":582,"context_line":"                            update_lrport_list.append(db_router_ports[lrport])"},{"line_number":583,"context_line":"                        del db_router_ports[lrport]"},{"line_number":584,"context_line":"                    else:"},{"line_number":585,"context_line":"                        del_lrouter_ports_list.append("}],"source_content_type":"text/x-python","patch_set":1,"id":"7addc3e0_545c4cda","line":582,"range":{"start_line":582,"start_character":54,"end_line":582,"end_character":77},"in_reply_to":"80c0377d_461d4bf3","updated":"2023-08-08 15:18:53.000000000","message":"Oops, indeed. Missed it.","commit_id":"8928e5c4f6f1036b0bc8770d0891e7d122a529f2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"59e2493067971f086dcdf8224a75f3b33d671bbd","unresolved":true,"context_lines":[{"line_number":473,"context_line":"        if ipv6_ra_supported:"},{"line_number":474,"context_line":"            lrp_name \u003d utils.ovn_lrouter_port_name(db_router_port[\u0027id\u0027])"},{"line_number":475,"context_line":"            ovn_lrport \u003d self.ovn_api.lrp_get("},{"line_number":476,"context_line":"                lrp_name).execute()"},{"line_number":477,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        return db_lrport_ra !\u003d lrport_ra"}],"source_content_type":"text/x-python","patch_set":2,"id":"88402430_dbafe602","line":476,"range":{"start_line":476,"start_character":26,"end_line":476,"end_character":35},"updated":"2023-08-08 16:05:10.000000000","message":"Without `check_error\u003dTrue`, `lrp_get()` could return `None`, causing an `AttributeError` below. Should `_is_router_port_changed()` handle this case and still return a value?","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2a48273f0c1674387ba8d80dd4d5e55a71283849","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        if ipv6_ra_supported:"},{"line_number":474,"context_line":"            lrp_name \u003d utils.ovn_lrouter_port_name(db_router_port[\u0027id\u0027])"},{"line_number":475,"context_line":"            ovn_lrport \u003d self.ovn_api.lrp_get("},{"line_number":476,"context_line":"                lrp_name).execute()"},{"line_number":477,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        return db_lrport_ra !\u003d lrport_ra"}],"source_content_type":"text/x-python","patch_set":2,"id":"288a39cc_876bfc15","line":476,"range":{"start_line":476,"start_character":26,"end_line":476,"end_character":35},"in_reply_to":"120b1e3b_999c19a7","updated":"2023-08-10 09:02:06.000000000","message":"Done","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"807300325802b342437331f4760b060981610108","unresolved":true,"context_lines":[{"line_number":473,"context_line":"        if ipv6_ra_supported:"},{"line_number":474,"context_line":"            lrp_name \u003d utils.ovn_lrouter_port_name(db_router_port[\u0027id\u0027])"},{"line_number":475,"context_line":"            ovn_lrport \u003d self.ovn_api.lrp_get("},{"line_number":476,"context_line":"                lrp_name).execute()"},{"line_number":477,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        return db_lrport_ra !\u003d lrport_ra"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf9043d1_84e8d2c0","line":476,"range":{"start_line":476,"start_character":26,"end_line":476,"end_character":35},"in_reply_to":"541258e5_9e414bff","updated":"2023-08-09 13:59:23.000000000","message":"I was checking the code, so there\u0027s another list where we try to create the missing router ports in OVN before we go thru the list for updating them [0].\n\nAfter seem that it seems to me that this patch should just return false if the port is not found because later this script will create the port again and it will have all the latest information in it.\n\nWhat do you think ?\n\n[0] https://github.com/openstack/neutron/blob/176b144460570fcf2792a199d2b639b335822f14/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py#L641-L667","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"a46476955c5948882d308afd7736bcbae1373cea","unresolved":true,"context_lines":[{"line_number":473,"context_line":"        if ipv6_ra_supported:"},{"line_number":474,"context_line":"            lrp_name \u003d utils.ovn_lrouter_port_name(db_router_port[\u0027id\u0027])"},{"line_number":475,"context_line":"            ovn_lrport \u003d self.ovn_api.lrp_get("},{"line_number":476,"context_line":"                lrp_name).execute()"},{"line_number":477,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        return db_lrport_ra !\u003d lrport_ra"}],"source_content_type":"text/x-python","patch_set":2,"id":"541258e5_9e414bff","line":476,"range":{"start_line":476,"start_character":26,"end_line":476,"end_character":35},"in_reply_to":"548dd9dd_6fc04cf1","updated":"2023-08-09 13:33:53.000000000","message":"Not sure either. If we can\u0027t fetch the LRP, it\u0027s been deleted and if it\u0027s been deleted from OVN but is still in the neutron db, then it seems like that would need to be fixed. If we raise the exception, it looks like we would just give up on syncing halfway through the sync (skipping QoS and stateful fips stuff). Seems like if we can\u0027t figure out how to fix the port, maybe logging an issue and continuing to do the rest of the sync might be an option?","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9390a35a7d2d1dcf740404af01a9a83da1b352d0","unresolved":true,"context_lines":[{"line_number":473,"context_line":"        if ipv6_ra_supported:"},{"line_number":474,"context_line":"            lrp_name \u003d utils.ovn_lrouter_port_name(db_router_port[\u0027id\u0027])"},{"line_number":475,"context_line":"            ovn_lrport \u003d self.ovn_api.lrp_get("},{"line_number":476,"context_line":"                lrp_name).execute()"},{"line_number":477,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        return db_lrport_ra !\u003d lrport_ra"}],"source_content_type":"text/x-python","patch_set":2,"id":"548dd9dd_6fc04cf1","line":476,"range":{"start_line":476,"start_character":26,"end_line":476,"end_character":35},"in_reply_to":"88402430_dbafe602","updated":"2023-08-09 08:31:21.000000000","message":"Good point, not sure it should return a value or just error out. Because if we can\u0027t fetch the port from OVN we do not know what is the ipv6_ra_configs value and anything else would be just a guess. I think that adding check_error\u003dTrue and letting it error out is the safest we can do here. User can run the script again. I will add check_error\u003dTrue to it.\n\nI see similar problems in other places of this code too, for example: L198 and L236.","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"36ee620bd7f02e748f1ccd26c2e40efdff114723","unresolved":true,"context_lines":[{"line_number":473,"context_line":"        if ipv6_ra_supported:"},{"line_number":474,"context_line":"            lrp_name \u003d utils.ovn_lrouter_port_name(db_router_port[\u0027id\u0027])"},{"line_number":475,"context_line":"            ovn_lrport \u003d self.ovn_api.lrp_get("},{"line_number":476,"context_line":"                lrp_name).execute()"},{"line_number":477,"context_line":"            lrport_ra \u003d ovn_lrport.ipv6_ra_configs"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        return db_lrport_ra !\u003d lrport_ra"}],"source_content_type":"text/x-python","patch_set":2,"id":"120b1e3b_999c19a7","line":476,"range":{"start_line":476,"start_character":26,"end_line":476,"end_character":35},"in_reply_to":"bf9043d1_84e8d2c0","updated":"2023-08-09 14:05:12.000000000","message":"+1 This sounds reasonable to me.","commit_id":"e76fc931079e2f2f5cbb1c65c6e6792df4f5ae3b"}]}