)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e47ae9d9c21cd6ba02d0ca8aab0b71eca546fb4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f31d9ac4_44244897","updated":"2026-04-22 08:23:09.000000000","message":"NOTE: we need to backport this fix up to stable/2025.1, where the referred change modified the `MaintenanceThread` class.","commit_id":"853d8fe9bc09da69d8b2c39f7ccebb16fe439a5e"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"b3037425ba7c2f3578a677c6f6e91c1a97028d37","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a99056a0_323818df","updated":"2026-04-22 07:55:22.000000000","message":"looks a legit fix","commit_id":"853d8fe9bc09da69d8b2c39f7ccebb16fe439a5e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2451272181492161a095462f476047523520ffc7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c6e2b39e_e195f3f1","updated":"2026-04-22 03:57:01.000000000","message":"recheck ovn-bgp","commit_id":"853d8fe9bc09da69d8b2c39f7ccebb16fe439a5e"}],"neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6b6901e8f45953b5dbf4d2df2c955cd0a6850754","unresolved":true,"context_lines":[{"line_number":36,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class TestMaintenanceThread(base.BaseTestCase):"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def test_start_bounds_thread_pool_executor(self):"},{"line_number":42,"context_line":"        \"\"\"Verify MaintenanceThread limits ThreadPoolExecutor to 1 worker.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"84705911_a9efb1ac","line":39,"updated":"2026-04-22 08:21:52.000000000","message":"This test is not actually testing the number of threads spawn by the executor.\n\nI would suggest something like this:\n```\nclass TestMaintenanceThread(base.BaseTestCase):\n\n    def test_start_bounds_thread_pool_executor(self):\n        \"\"\"Verify MaintenanceThread runs periodic tasks with one worker.\"\"\"\n        num_tasks \u003d 5\n        run_counter \u003d [0] * num_tasks\n        active_threads \u003d []\n        target_runs \u003d 5\n\n        class _PeriodicTask:\n            def __init__(self, idx):\n                self.idx \u003d idx\n\n            @periodics.periodic(spacing\u003d0.05)\n            def trivial_task(self):\n                nonlocal run_counter\n                run_counter[self.idx] +\u003d 1\n                active_threads.append(threading.active_count())\n                time.sleep(0)\n                if run_counter[self.idx] \u003e\u003d target_runs:\n                    raise periodics.NeverAgain\n\n        mt \u003d maintenance.MaintenanceThread()\n        for idx in range(num_tasks):\n            mt.add_periodics(_PeriodicTask(idx))\n\n        threads_before \u003d threading.active_count()\n        mt.start()\n        threads_after_start \u003d threading.active_count()\n        self.assertGreater(threads_after_start, threads_before)\n\n        executor \u003d mt._worker._executor_factory()\n        # Let some tasks be executed.\n        time.sleep(2)\n        threads_after_wait \u003d threading.active_count()\n        self.assertEqual(threads_after_wait, threads_after_start + 1)\n\n        executor.shutdown(wait\u003dFalse)\n        mt._thread.join(timeout\u003d3)\n        threads_end \u003d threading.active_count()\n        self.assertEqual(threads_end, threads_after_wait)\n        self.assertTrue(all(target_runs \u003d\u003d _c for _c in run_counter))\n```\n\nThe `threads_after_wait` will be 1 thread bigger than `threads_after_start` due to the single worker created. Actually if you change the `max_workers` to another value (10, for example), that will be the difference between both thread counts.","commit_id":"853d8fe9bc09da69d8b2c39f7ccebb16fe439a5e"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"5d0c9e91c0f85b71d335c80931a059084285916d","unresolved":false,"context_lines":[{"line_number":36,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class TestMaintenanceThread(base.BaseTestCase):"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def test_start_bounds_thread_pool_executor(self):"},{"line_number":42,"context_line":"        \"\"\"Verify MaintenanceThread limits ThreadPoolExecutor to 1 worker.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"b67d7c79_1243b4da","line":39,"in_reply_to":"84705911_a9efb1ac","updated":"2026-04-22 08:58:09.000000000","message":"Done","commit_id":"853d8fe9bc09da69d8b2c39f7ccebb16fe439a5e"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"49708dd1e65ba8597190be03b95688e9e9a0b28e","unresolved":false,"context_lines":[{"line_number":36,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class TestMaintenanceThread(base.BaseTestCase):"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def test_start_bounds_thread_pool_executor(self):"},{"line_number":42,"context_line":"        \"\"\"Verify MaintenanceThread limits ThreadPoolExecutor to 1 worker.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"85cb30b6_c3b569c6","line":39,"in_reply_to":"b67d7c79_1243b4da","updated":"2026-04-22 09:56:46.000000000","message":"So, I don\u0027t have a huge issue with this the way it is now, but I\u0027ve run like three models against it and they all warn about it possibly being flaky due to background thread noise and using sleep()...\n\nPer opus46 (which I trust the most of them):\n\u003e Keep the real execution test but make it deterministic:\n- Don\u0027t count threads — instead inspect executor._max_workers directly (the actual invariant we care about)\n- Still run real tasks to verify they complete (proves the executor works)\n- Remove unused active_threads collection\n- Remove the extra executor_factory() call\n\nSo maybe something like this?\n```\ndef test_start_bounds_thread_pool_executor(self):\n    \"\"\"Verify MaintenanceThread runs periodic tasks with one worker.\"\"\"\n    num_tasks \u003d 5\n    run_counter \u003d [0] * num_tasks\n    target_runs \u003d 5\n    class _PeriodicTask:\n        def __init__(self, idx):\n            self.idx \u003d idx\n        @periodics.periodic(spacing\u003d0.05)\n        def trivial_task(self):\n            run_counter[self.idx] +\u003d 1\n            if run_counter[self.idx] \u003e\u003d target_runs:\n                raise periodics.NeverAgain\n    mt \u003d maintenance.MaintenanceThread()\n    for idx in range(num_tasks):\n        mt.add_periodics(_PeriodicTask(idx))\n    mt.start()\n    # Assert the executor is bounded to 1 worker\n    executor \u003d mt._worker._executor_factory()\n    self.assertEqual(1, executor._max_workers)\n    executor.shutdown(wait\u003dFalse)\n    # Wait for all periodic tasks to complete\n    mt._thread.join(timeout\u003d10)\n    self.assertTrue(all(target_runs \u003d\u003d c for c in run_counter))\n```\nWe still avoid mocks but try to be more deterministic (assert _max_workers directly, no thread counting which might get skewed by parallel stuff).\n\nThoughts? I feel like this might be bikeshedding at this point but I don\u0027t want to be the one to introduce a flaky test to the neutron gates :D","commit_id":"853d8fe9bc09da69d8b2c39f7ccebb16fe439a5e"}]}