)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"8f0df3b2af867d11ac51f318aaf4b357cb9b23f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cb7c4124_da586af2","updated":"2024-03-13 15:16:27.000000000","message":"recheck","commit_id":"27b2f22df10e6e41c6fc4e1ce7f839aeb3dc3e13"}],"neutron/db/extraroute_db.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"eb9a0bfc5f98f4685a473a5533144743950d954e","unresolved":true,"context_lines":[{"line_number":139,"context_line":"                destination\u003droute[\u0027destination\u0027],"},{"line_number":140,"context_line":"                nexthop\u003droute[\u0027nexthop\u0027])"},{"line_number":141,"context_line":"            if obj is not None:"},{"line_number":142,"context_line":"                obj.delete()"},{"line_number":143,"context_line":"            else:"},{"line_number":144,"context_line":"                LOG.debug(\u0027Route %s via %s of router %s has already been \u0027"},{"line_number":145,"context_line":"                          \u0027deleted\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"09d4c6ad_d8a9f068","line":142,"range":{"start_line":142,"start_character":16,"end_line":142,"end_character":28},"updated":"2024-03-13 07:29:13.000000000","message":"1) Use ``NeutronDbObject.delete_objects`` instead of retrieving and deleting (two operations).\n2) If you want to print a debug message, use the len(db_objs) returned. If 0, then print the debug message.\n3) As Brian pointed out in the LP bug, please provide the error traceback that you captured when the error happened. It would be also helpful to have a reproducer (if possible) and the test case this issue was breaking.","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"d316a5a6882acc506b0bc6c7dbacf81dc902d860","unresolved":true,"context_lines":[{"line_number":139,"context_line":"                destination\u003droute[\u0027destination\u0027],"},{"line_number":140,"context_line":"                nexthop\u003droute[\u0027nexthop\u0027])"},{"line_number":141,"context_line":"            if obj is not None:"},{"line_number":142,"context_line":"                obj.delete()"},{"line_number":143,"context_line":"            else:"},{"line_number":144,"context_line":"                LOG.debug(\u0027Route %s via %s of router %s has already been \u0027"},{"line_number":145,"context_line":"                          \u0027deleted\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e6a9cc6_f11fc963","line":142,"range":{"start_line":142,"start_character":16,"end_line":142,"end_character":28},"in_reply_to":"09d4c6ad_d8a9f068","updated":"2024-03-13 10:44:24.000000000","message":"1) That\u0027s a better solution than mine and I wasn\u0027t aware of that method. Thanks!\n2) Removed the message altogether, as it probably doesn\u0027t provide much value.\n3) I added the the traceback to the bug. I did not try to reproduce it in my prod env after I\u0027ve written the breaking test case, as that + sentry traceback was enough for me. The breaking test case is part of this commit, would you like me to copypaste that into the bugreport as well?","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ee9c805eeae2f4da78982799d2c1aea8aeb6f393","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                obj.delete()"},{"line_number":143,"context_line":"            else:"},{"line_number":144,"context_line":"                LOG.debug(\u0027Route %s via %s of router %s has already been \u0027"},{"line_number":145,"context_line":"                          \u0027deleted\u0027,"},{"line_number":146,"context_line":"                          route[\u0027destination\u0027], route[\u0027nexthop\u0027], router[\u0027id\u0027])"},{"line_number":147,"context_line":"        return added, removed"},{"line_number":148,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"56851cca_01c6aae8","line":145,"updated":"2024-03-12 19:11:03.000000000","message":"Can you just add the word \u0027concurrently\u0027 here? Most of the other places we log these warnings we use that word, for example:\n\n\"Port %s has been deleted concurrently\"\n\nSo something like:\n\n\u0027Route %s via %s of router %s has been deleted concurrently\u0027","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"d316a5a6882acc506b0bc6c7dbacf81dc902d860","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                obj.delete()"},{"line_number":143,"context_line":"            else:"},{"line_number":144,"context_line":"                LOG.debug(\u0027Route %s via %s of router %s has already been \u0027"},{"line_number":145,"context_line":"                          \u0027deleted\u0027,"},{"line_number":146,"context_line":"                          route[\u0027destination\u0027], route[\u0027nexthop\u0027], router[\u0027id\u0027])"},{"line_number":147,"context_line":"        return added, removed"},{"line_number":148,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"4396c362_eb2ecad1","line":145,"in_reply_to":"56851cca_01c6aae8","updated":"2024-03-13 10:44:24.000000000","message":"I decided to delete the message altogether after just using delete_objects(). Thanks for the hint though, always good to get a better feeling for how these things are worded.","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"}],"neutron/tests/unit/db/test_extraroute_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ee9c805eeae2f4da78982799d2c1aea8aeb6f393","unresolved":true,"context_lines":[{"line_number":186,"context_line":"                routes \u003d orig_func(ctx, router_id)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"                # forcefully delete route to 10.2.0.0/24"},{"line_number":189,"context_line":"                ctx2 \u003d context.get_admin_context()"},{"line_number":190,"context_line":"                l3_obj.RouterRoute.get_object("},{"line_number":191,"context_line":"                    ctx2,"},{"line_number":192,"context_line":"                    router_id\u003drouter_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"42f06bed_a19549e0","line":189,"updated":"2024-03-12 19:11:03.000000000","message":"Do you need this second context?","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"d316a5a6882acc506b0bc6c7dbacf81dc902d860","unresolved":true,"context_lines":[{"line_number":186,"context_line":"                routes \u003d orig_func(ctx, router_id)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"                # forcefully delete route to 10.2.0.0/24"},{"line_number":189,"context_line":"                ctx2 \u003d context.get_admin_context()"},{"line_number":190,"context_line":"                l3_obj.RouterRoute.get_object("},{"line_number":191,"context_line":"                    ctx2,"},{"line_number":192,"context_line":"                    router_id\u003drouter_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"cfd67457_d8b7cc56","line":189,"in_reply_to":"42f06bed_a19549e0","updated":"2024-03-13 10:44:24.000000000","message":"When I was writing the test I was not sure if I couldn\u0027t also solve this with some sort of transaction magic, or that this might be done in the future (though the required locking seemed like something that would impact performance at the rate we\u0027re having changing routes). My idea with this patched method was deleting the route outside the current transaction / session with a new session, so we can simulate two sessions working on the same rows. For my patch, this wouldn\u0027t make a difference. Does this make sense to you?","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d70f4304ecaba54cf2a7e06dc87d16d60f5426f7","unresolved":true,"context_lines":[{"line_number":186,"context_line":"                routes \u003d orig_func(ctx, router_id)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"                # forcefully delete route to 10.2.0.0/24"},{"line_number":189,"context_line":"                ctx2 \u003d context.get_admin_context()"},{"line_number":190,"context_line":"                l3_obj.RouterRoute.get_object("},{"line_number":191,"context_line":"                    ctx2,"},{"line_number":192,"context_line":"                    router_id\u003drouter_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5beadee9_1d9c3502","line":189,"in_reply_to":"cfd67457_d8b7cc56","updated":"2024-03-25 00:21:35.000000000","message":"Yes, makes sense.","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ee9c805eeae2f4da78982799d2c1aea8aeb6f393","unresolved":true,"context_lines":[{"line_number":201,"context_line":"                    self._plugin._get_extra_routes_by_router_id)) \\"},{"line_number":202,"context_line":"                as mock_get_routes:"},{"line_number":203,"context_line":"            self._test_update_routes(ctx, router_id, router, routes)"},{"line_number":204,"context_line":"            mock_get_routes.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5548d126_534c1d49","line":204,"range":{"start_line":204,"start_character":28,"end_line":204,"end_character":46},"updated":"2024-03-12 19:11:03.000000000","message":"Is there a value you can use to have this be assert_called_once_with() ?","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"d316a5a6882acc506b0bc6c7dbacf81dc902d860","unresolved":true,"context_lines":[{"line_number":201,"context_line":"                    self._plugin._get_extra_routes_by_router_id)) \\"},{"line_number":202,"context_line":"                as mock_get_routes:"},{"line_number":203,"context_line":"            self._test_update_routes(ctx, router_id, router, routes)"},{"line_number":204,"context_line":"            mock_get_routes.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fbc2dbe0_c39350df","line":204,"range":{"start_line":204,"start_character":28,"end_line":204,"end_character":46},"in_reply_to":"5548d126_534c1d49","updated":"2024-03-13 10:44:24.000000000","message":"I have written this test with the purpose of checking that the router update works (i.e. does not raise an AttributeError). The only reason I put that assert in there is that I want to make sure my patched method was actually called and not ignored, because it provides the side effect of deleting that route after having fetched it. If it were not called, the test would be worthless.","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d70f4304ecaba54cf2a7e06dc87d16d60f5426f7","unresolved":true,"context_lines":[{"line_number":201,"context_line":"                    self._plugin._get_extra_routes_by_router_id)) \\"},{"line_number":202,"context_line":"                as mock_get_routes:"},{"line_number":203,"context_line":"            self._test_update_routes(ctx, router_id, router, routes)"},{"line_number":204,"context_line":"            mock_get_routes.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":1,"id":"84852a7c_49e6f46d","line":204,"range":{"start_line":204,"start_character":28,"end_line":204,"end_character":46},"in_reply_to":"fbc2dbe0_c39350df","updated":"2024-03-25 00:21:35.000000000","message":"Understood, I only mentioned due to the very small number of these calls (52) compared to the with() version at 1400+.","commit_id":"2c2d17a5d8b607d10a9f6638a223f60ccd1a20c4"}]}