)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8af650ea664ea28a0ea1c59be814e32d9d480539","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b43c8132_b0dd0b5d","updated":"2026-09-28 23:02:24.000000000","message":"lgtm, one comment about cleanup timeout","commit_id":"1f7920ed07171fdcbe8da328d807205de6efadcd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"40a085b94a91d54a0e24d984b35d53e4fb7b19a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"74c86046_2f8f30f6","updated":"2026-10-02 09:00:16.000000000","message":"unit test failure shows instability in the newly added tests.","commit_id":"12694174a8802a7998fcfb4aeb089954467f342e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"daad47111ace9cd51f3c9e3870a8c14a2a0208bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b2d0c5ab_307b3842","in_reply_to":"74c86046_2f8f30f6","updated":"2026-10-02 12:45:17.000000000","message":"fixed","commit_id":"12694174a8802a7998fcfb4aeb089954467f342e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"81635026e1bda098c43737763c175105720ed5a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1abf3194_3391ff80","updated":"2026-10-06 02:29:57.000000000","message":"lgtm, thanks","commit_id":"e6ddd25443a66524da6527ac754af6c0ac65676a"}],"nova/test.py":[{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"02898d165a6ab0ffc56f83b8a5331e8803440906","unresolved":true,"context_lines":[{"line_number":587,"context_line":"        thread \u003d threading.Thread(target\u003drun_periodics, daemon\u003dTrue)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        def cleanup():"},{"line_number":590,"context_line":"            thread.join(timeout\u003d15)"},{"line_number":591,"context_line":"            self.assertFalse(thread.is_alive(), \u0027Periodic thread did not exit\u0027)"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"        thread.start()"}],"source_content_type":"text/x-python","patch_set":2,"id":"f9aad84d_e7c7f09f","line":590,"updated":"2026-09-25 13:06:50.000000000","message":"this shoudn\u0027t be thread.join(timeout\u003dtimeout)?","commit_id":"1f7920ed07171fdcbe8da328d807205de6efadcd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8af650ea664ea28a0ea1c59be814e32d9d480539","unresolved":true,"context_lines":[{"line_number":587,"context_line":"        thread \u003d threading.Thread(target\u003drun_periodics, daemon\u003dTrue)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        def cleanup():"},{"line_number":590,"context_line":"            thread.join(timeout\u003d15)"},{"line_number":591,"context_line":"            self.assertFalse(thread.is_alive(), \u0027Periodic thread did not exit\u0027)"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"        thread.start()"}],"source_content_type":"text/x-python","patch_set":2,"id":"925d2ccd_47ddecd0","line":590,"in_reply_to":"2a05702e_c135fdb0","updated":"2026-09-28 23:02:24.000000000","message":"But in which case can these two be different? In case of deadlock, the thread can stuck at L581, and the periodic task assert will hit (L595). if periodic task is done (done.set) before \u0027timeout\u0027 then thread has nothing to stuck on right? Or, in other words, thread.join() is not really benefiting here, as we are failing test on timeout and cleanup also failing test on timeout.\n\nIf cleanup timeout can be \u003c periodic timeout, then it can add value when (periodic task done) test pass but stuck thread can fail the test. This case make sense but I am curious in what cases thread can stuck if periodic task is done.","commit_id":"1f7920ed07171fdcbe8da328d807205de6efadcd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0a435c60cca3465173ee6fd9209d726604bcd05e","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        thread \u003d threading.Thread(target\u003drun_periodics, daemon\u003dTrue)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        def cleanup():"},{"line_number":590,"context_line":"            thread.join(timeout\u003d15)"},{"line_number":591,"context_line":"            self.assertFalse(thread.is_alive(), \u0027Periodic thread did not exit\u0027)"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"        thread.start()"}],"source_content_type":"text/x-python","patch_set":2,"id":"6b953f09_a7332476","line":590,"in_reply_to":"925d2ccd_47ddecd0","updated":"2026-10-01 15:08:41.000000000","message":"yeah the latter is not realistic. \n\nOK this does not worth the effort to make perfect. I will just use a single param for both with a param doc that explains the double use.","commit_id":"1f7920ed07171fdcbe8da328d807205de6efadcd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e306980607736ec0f8d6f742f11df6d22ba328b9","unresolved":true,"context_lines":[{"line_number":587,"context_line":"        thread \u003d threading.Thread(target\u003drun_periodics, daemon\u003dTrue)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        def cleanup():"},{"line_number":590,"context_line":"            thread.join(timeout\u003d15)"},{"line_number":591,"context_line":"            self.assertFalse(thread.is_alive(), \u0027Periodic thread did not exit\u0027)"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"        thread.start()"}],"source_content_type":"text/x-python","patch_set":2,"id":"2a05702e_c135fdb0","line":590,"in_reply_to":"f9aad84d_e7c7f09f","updated":"2026-09-28 08:37:38.000000000","message":"that it two different timeout the param is about how long the periodics can run, the second is how long we wait for the thread to exit. It depends on how we want to interpret what the parameter means. I guess the best would be to add a cleanup_timeout parameter defaulted to 15 as well and used in the cleanup. that gives the most control. I will respin.","commit_id":"1f7920ed07171fdcbe8da328d807205de6efadcd"}]}
