)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"4e255f611cf5146bafb549fb6565590e839fc31a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"31622f8d_b46dd3c3","updated":"2024-11-05 08:29:50.000000000","message":"Got it. Sorry for that, I did not know about monitoring recheck reasons.\nI will avoid it in future. Thank you for the feedback.\nand thank you for review","commit_id":"798a3a95402dfb4420ce6ca4333528161cdc44e9"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"24b4d411c244bcbc71849230cc7256eaab10d65c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"479c624a_b06b37ff","updated":"2024-11-02 07:46:28.000000000","message":"recheck","commit_id":"798a3a95402dfb4420ce6ca4333528161cdc44e9"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"693218c1f3ef8a2f4bcdef62f2d2710ad249e1cf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f3fcd4c4_68f4151c","in_reply_to":"479c624a_b06b37ff","updated":"2024-11-05 08:00:43.000000000","message":"also, please don\u0027t use bare rechecks (TC is monitoring them and blames projects for that). always provide a reason like (\"recheck mirror timeout\")\ndon\u0027t hesitate to ping us if you don\u0027t know what happened","commit_id":"798a3a95402dfb4420ce6ca4333528161cdc44e9"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"e75115d33a8ceceb48d0567966516c977009e8f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8c573281_d6a2445b","updated":"2024-11-05 13:57:48.000000000","message":"sorry I have an additional question","commit_id":"2996dd36ef0021d9cb3b9b3739fd2d2a14236460"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"c437955c5b4774397bd0128657b016672b3d7397","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"f33c934a_1cbfc32d","updated":"2024-11-08 09:35:59.000000000","message":"LGTM, but I think we can remove one line from the test that is no longer needed","commit_id":"483ce46f1128ae0341c76b07ea3a064375526497"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"fb888a2796d1991d4359a3135cbd2416bf67e2b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"efe0555f_f16af1dd","updated":"2024-11-08 12:54:35.000000000","message":"LGTM Thanks!","commit_id":"f76fa1ae330e933e86c666b857acfee06f675adb"}],"octavia/controller/worker/v2/tasks/database_tasks.py":[{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"d4f805ed253261de47b5ef993835f02ba380b887","unresolved":true,"context_lines":[{"line_number":144,"context_line":"        except Exception as e:"},{"line_number":145,"context_line":"            # this record could be missed here"},{"line_number":146,"context_line":"            LOG.debug(\"Failed to delete amphora %(amp)s \""},{"line_number":147,"context_line":"                      \"in the database amphora_health due to: \""},{"line_number":148,"context_line":"                      \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"4e4df158_f9b409bc","line":147,"range":{"start_line":147,"start_character":30,"end_line":147,"end_character":39},"updated":"2024-10-28 09:17:12.000000000","message":"database table","commit_id":"bcd71636cf4b5e0d6bae0c402ef431ef86cdc98c"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"3c4b6ffb4dcda35fb0c52df0d231106d2557712d","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        except Exception as e:"},{"line_number":145,"context_line":"            # this record could be missed here"},{"line_number":146,"context_line":"            LOG.debug(\"Failed to delete amphora %(amp)s \""},{"line_number":147,"context_line":"                      \"in the database amphora_health due to: \""},{"line_number":148,"context_line":"                      \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"67417989_9d573f30","line":147,"range":{"start_line":147,"start_character":30,"end_line":147,"end_character":39},"in_reply_to":"4e4df158_f9b409bc","updated":"2024-10-28 16:57:10.000000000","message":"oh right! thank you for catching it. fixed","commit_id":"bcd71636cf4b5e0d6bae0c402ef431ef86cdc98c"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"1946ce9118f3b49e715155d22f02dace8373fc04","unresolved":true,"context_lines":[{"line_number":140,"context_line":"                      \"in the database due to: \""},{"line_number":141,"context_line":"                      \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"        try:"},{"line_number":143,"context_line":"            self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"        except Exception as e:"},{"line_number":145,"context_line":"            # this record could be missed here"},{"line_number":146,"context_line":"            LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":4,"id":"5e0b9772_68c3d8dc","line":143,"range":{"start_line":143,"start_character":12,"end_line":143,"end_character":36},"updated":"2024-11-05 07:54:00.000000000","message":"delete should be called inside a DB session block\n\nmy advice is to move the `with db_apis.session().begin() as session:`\noutside the try block\n\nsomething like\n\nwith db_apis.session().begin() as session:\n    try:\n        # delete amp\n    except ...:\n        ...\n    try:\n        # delete amp health\n    except ...:\n         ...","commit_id":"798a3a95402dfb4420ce6ca4333528161cdc44e9"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"d57cac5bc2adeb521bd19fee3be25aec0368a9c2","unresolved":true,"context_lines":[{"line_number":140,"context_line":"                      \"in the database due to: \""},{"line_number":141,"context_line":"                      \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"        try:"},{"line_number":143,"context_line":"            self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"        except Exception as e:"},{"line_number":145,"context_line":"            # this record could be missed here"},{"line_number":146,"context_line":"            LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":4,"id":"b1ca98c6_6f1106cb","line":143,"range":{"start_line":143,"start_character":12,"end_line":143,"end_character":36},"in_reply_to":"5e0b9772_68c3d8dc","updated":"2024-11-05 08:53:33.000000000","message":"done.\n\nI just think, may be","commit_id":"798a3a95402dfb4420ce6ca4333528161cdc44e9"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"6d016847fe69c9d3b118d06585cb945d993089ac","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                      \"in the database due to: \""},{"line_number":141,"context_line":"                      \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"        try:"},{"line_number":143,"context_line":"            self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"        except Exception as e:"},{"line_number":145,"context_line":"            # this record could be missed here"},{"line_number":146,"context_line":"            LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":4,"id":"89fef888_16eed709","line":143,"range":{"start_line":143,"start_character":12,"end_line":143,"end_character":36},"in_reply_to":"b1ca98c6_6f1106cb","updated":"2024-11-05 11:46:11.000000000","message":"looks like I missed part of original message.\nI thought about adding one general try/except block for catching error on session begin. However let\u0027s start with your suggestion first.","commit_id":"798a3a95402dfb4420ce6ca4333528161cdc44e9"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"e75115d33a8ceceb48d0567966516c977009e8f9","unresolved":true,"context_lines":[{"line_number":140,"context_line":"                          \"in the database due to: \""},{"line_number":141,"context_line":"                          \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"            try:"},{"line_number":143,"context_line":"                self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"            except Exception as e:"},{"line_number":145,"context_line":"                # this record could be missed here"},{"line_number":146,"context_line":"                LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"4d5f12bd_a8204527","line":143,"range":{"start_line":143,"start_character":16,"end_line":143,"end_character":43},"updated":"2024-11-05 13:57:48.000000000","message":"maybe an additional question if you ran some tests with this code.\nDoes it display a \"Failed to delete ...\" error when there\u0027s no record in the DB?","commit_id":"2996dd36ef0021d9cb3b9b3739fd2d2a14236460"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"3fbe337b4a10e45af7455caed0c77413e9630e09","unresolved":true,"context_lines":[{"line_number":140,"context_line":"                          \"in the database due to: \""},{"line_number":141,"context_line":"                          \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"            try:"},{"line_number":143,"context_line":"                self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"            except Exception as e:"},{"line_number":145,"context_line":"                # this record could be missed here"},{"line_number":146,"context_line":"                LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9bc01ddd_0f56f99f","line":143,"range":{"start_line":143,"start_character":16,"end_line":143,"end_character":43},"in_reply_to":"4d5f12bd_a8204527","updated":"2024-11-06 06:39:05.000000000","message":"yes, but I have debug level enabled. \nactually I could set the error level instead. wdyt about it ?","commit_id":"2996dd36ef0021d9cb3b9b3739fd2d2a14236460"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"2c84c8525edd88ce974bd43367c66ce82fada00c","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                          \"in the database due to: \""},{"line_number":141,"context_line":"                          \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"            try:"},{"line_number":143,"context_line":"                self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"            except Exception as e:"},{"line_number":145,"context_line":"                # this record could be missed here"},{"line_number":146,"context_line":"                LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"93acdc93_227d54e4","line":143,"range":{"start_line":143,"start_character":16,"end_line":143,"end_character":43},"in_reply_to":"5137f8f7_d43d8368","updated":"2024-11-06 10:36:29.000000000","message":"Let\u0027s ignore all messages.","commit_id":"2996dd36ef0021d9cb3b9b3739fd2d2a14236460"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"17b24409cdce1d95501252f11d7ba924ad36180f","unresolved":true,"context_lines":[{"line_number":140,"context_line":"                          \"in the database due to: \""},{"line_number":141,"context_line":"                          \"%(except)s\", {\u0027amp\u0027: result, \u0027except\u0027: str(e)})"},{"line_number":142,"context_line":"            try:"},{"line_number":143,"context_line":"                self.amp_health_repo.delete(session, amphora_id\u003dresult)"},{"line_number":144,"context_line":"            except Exception as e:"},{"line_number":145,"context_line":"                # this record could be missed here"},{"line_number":146,"context_line":"                LOG.debug(\"Failed to delete amphora %(amp)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"5137f8f7_d43d8368","line":143,"range":{"start_line":143,"start_character":16,"end_line":143,"end_character":43},"in_reply_to":"9bc01ddd_0f56f99f","updated":"2024-11-06 08:28:54.000000000","message":"my point is that the user can have a false-positive \"Failed to delete amphora ...\" message in case the records don\u0027t exist.\n\nnote: the housekeeping service ignores this error:\nhttps://github.com/openstack/octavia/blob/master/octavia/controller/housekeeping/house_keeping.py#L59-L60\n\nwe may have 2 choices:\n- catch NoResultFound and skip it (just pass) + catch Exception and display it\n- catch Exception and skip all those exceptions\n\nIMHO ignoring all the exceptions when deleting the amp_health is fine.","commit_id":"2996dd36ef0021d9cb3b9b3739fd2d2a14236460"}],"octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py":[{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"c437955c5b4774397bd0128657b016672b3d7397","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        # Test the revert"},{"line_number":214,"context_line":"        create_amp_in_db.revert(_tf_failure_mock)"},{"line_number":215,"context_line":"        self.assertFalse(mock_amphora_repo_delete.called)"},{"line_number":216,"context_line":"        self.assertFalse(mock_amphora_health_repo_delete.called)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        amp_id \u003d \u0027AMP\u0027"},{"line_number":219,"context_line":"        mock_amphora_repo_delete.reset_mock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fc269a1_d11e4789","line":216,"range":{"start_line":216,"start_character":8,"end_line":216,"end_character":24},"updated":"2024-11-08 09:35:59.000000000","message":"nit: could be replaced with mock_amphora_health_repo_delete.assert_called(_once)()","commit_id":"483ce46f1128ae0341c76b07ea3a064375526497"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"ecf8172189c304ec715cecfabe449417345e885c","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        # Test the revert"},{"line_number":214,"context_line":"        create_amp_in_db.revert(_tf_failure_mock)"},{"line_number":215,"context_line":"        self.assertFalse(mock_amphora_repo_delete.called)"},{"line_number":216,"context_line":"        self.assertFalse(mock_amphora_health_repo_delete.called)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        amp_id \u003d \u0027AMP\u0027"},{"line_number":219,"context_line":"        mock_amphora_repo_delete.reset_mock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ef6825c8_bc5fb199","line":216,"range":{"start_line":216,"start_character":8,"end_line":216,"end_character":24},"in_reply_to":"48d34124_7dae3240","updated":"2024-11-08 11:11:46.000000000","message":"Done","commit_id":"483ce46f1128ae0341c76b07ea3a064375526497"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"e86f6fc9b94e9891cb0a64d909bb07ca3801b0ab","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        # Test the revert"},{"line_number":214,"context_line":"        create_amp_in_db.revert(_tf_failure_mock)"},{"line_number":215,"context_line":"        self.assertFalse(mock_amphora_repo_delete.called)"},{"line_number":216,"context_line":"        self.assertFalse(mock_amphora_health_repo_delete.called)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        amp_id \u003d \u0027AMP\u0027"},{"line_number":219,"context_line":"        mock_amphora_repo_delete.reset_mock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"48d34124_7dae3240","line":216,"range":{"start_line":216,"start_character":8,"end_line":216,"end_character":24},"in_reply_to":"9fc269a1_d11e4789","updated":"2024-11-08 09:44:27.000000000","message":"assert_not_called in this case","commit_id":"483ce46f1128ae0341c76b07ea3a064375526497"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"c437955c5b4774397bd0128657b016672b3d7397","unresolved":true,"context_lines":[{"line_number":249,"context_line":"            \"Failed to delete amphora %(amp)s \""},{"line_number":250,"context_line":"            \"in the database due to: \""},{"line_number":251,"context_line":"            \"%(except)s\", {\u0027amp\u0027: amp_id, \u0027except\u0027: err1_msg})"},{"line_number":252,"context_line":"        mock_LOG.debug.assert_not_called()"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @mock.patch(\u0027octavia.db.repositories.ListenerRepository.delete\u0027)"},{"line_number":255,"context_line":"    def test_delete_listener_in_db(self,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3e661e26_00b51b44","line":252,"range":{"start_line":252,"start_character":8,"end_line":252,"end_character":42},"updated":"2024-11-08 09:35:59.000000000","message":"we can remove this line, debug was called in a previous patchset and it\u0027s no longer called, so this assert is useless.","commit_id":"483ce46f1128ae0341c76b07ea3a064375526497"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"7e3fb88ea64e36b6d214c679a1ce450c2f1fb7d4","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            \"Failed to delete amphora %(amp)s \""},{"line_number":250,"context_line":"            \"in the database due to: \""},{"line_number":251,"context_line":"            \"%(except)s\", {\u0027amp\u0027: amp_id, \u0027except\u0027: err1_msg})"},{"line_number":252,"context_line":"        mock_LOG.debug.assert_not_called()"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @mock.patch(\u0027octavia.db.repositories.ListenerRepository.delete\u0027)"},{"line_number":255,"context_line":"    def test_delete_listener_in_db(self,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3c1dd705_19fc1755","line":252,"range":{"start_line":252,"start_character":8,"end_line":252,"end_character":42},"in_reply_to":"3e661e26_00b51b44","updated":"2024-11-08 11:12:04.000000000","message":"Done","commit_id":"483ce46f1128ae0341c76b07ea3a064375526497"}]}