)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"df18687d25871667fe5e1f3b688a1ce3c1b9b907","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1d1ad6c1_2f3c7f01","updated":"2025-10-13 08:34:27.000000000","message":"The fixture needs more love","commit_id":"19bc180fc698265649b738dbd13e0658cf425d22"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"28b5f71804ea67c3862257f905e6f7199e927ae8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d1d1bc15_d1e9f389","updated":"2025-10-14 16:10:04.000000000","message":"My comment were fixed. Thanks. Dan was +2 before so I\u0027m happy to proxy that now.","commit_id":"f6314d9027df8449366a1246830bb9bbdff79519"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3385045e6df24a69c6ed514447b4d875472ae63a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"94be8878_2c03c0f9","updated":"2026-01-11 03:42:21.000000000","message":"post merge comment","commit_id":"f6314d9027df8449366a1246830bb9bbdff79519"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0e22a2b17ffd5e3384bcd5136505b06ad9a557d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5d769c85_7a3baf78","updated":"2025-10-13 20:48:37.000000000","message":"the issue with leaking the executor between test has been addressed so im OK with proceeding with this now.\n\nWe can consider the other refactors in a followup.","commit_id":"f6314d9027df8449366a1246830bb9bbdff79519"}],"nova/cmd/conductor.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3385045e6df24a69c6ed514447b4d875472ae63a","unresolved":true,"context_lines":[{"line_number":45,"context_line":"                                    topic\u003drpcapi.RPC_TOPIC)"},{"line_number":46,"context_line":"    workers \u003d CONF.conductor.workers or processutils.get_worker_count()"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    utils.destroy_default_executor()"},{"line_number":49,"context_line":"    service.serve(server, workers\u003dworkers)"},{"line_number":50,"context_line":"    service.wait()"}],"source_content_type":"text/x-python","patch_set":4,"id":"35586aca_60a926c0","line":48,"range":{"start_line":48,"start_character":35,"end_line":48,"end_character":36},"updated":"2026-01-11 03:42:21.000000000","message":"how about destroy_scatter_gather_executor ? conductor service also check for old compute node and use scatter_gather before serice is forked by oslo.service?","commit_id":"f6314d9027df8449366a1246830bb9bbdff79519"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"df18687d25871667fe5e1f3b688a1ce3c1b9b907","unresolved":true,"context_lines":[{"line_number":1237,"context_line":"    def reset_globals(self):"},{"line_number":1238,"context_line":"        utils.SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1239,"context_line":"        utils.DEFAULT_EXECUTOR \u003d None"},{"line_number":1240,"context_line":"        utils.CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1241,"context_line":""},{"line_number":1242,"context_line":"    def do_cleanup_executor(self, executor):"},{"line_number":1243,"context_line":"        # NOTE(gibi): we cannot rely on utils.concurrency_mode_threading"}],"source_content_type":"text/x-python","patch_set":3,"id":"d5fc5ff5_58235f0c","line":1240,"updated":"2025-10-13 08:34:27.000000000","message":"This is just part of the fixture\u0027s logic. See how _setUp() redirects the global executor in a way that we can make sure that each test case gets a new executor so threads are not leaked across test cases","commit_id":"19bc180fc698265649b738dbd13e0658cf425d22"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"a10eff97cfc017fb782e1949f847d35b139adb64","unresolved":true,"context_lines":[{"line_number":1237,"context_line":"    def reset_globals(self):"},{"line_number":1238,"context_line":"        utils.SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1239,"context_line":"        utils.DEFAULT_EXECUTOR \u003d None"},{"line_number":1240,"context_line":"        utils.CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1241,"context_line":""},{"line_number":1242,"context_line":"    def do_cleanup_executor(self, executor):"},{"line_number":1243,"context_line":"        # NOTE(gibi): we cannot rely on utils.concurrency_mode_threading"}],"source_content_type":"text/x-python","patch_set":3,"id":"21288559_ccfef7d5","line":1240,"in_reply_to":"d5fc5ff5_58235f0c","updated":"2025-10-13 13:20:07.000000000","message":"thanks! I updated fixture in similar way for cache_images_executor","commit_id":"19bc180fc698265649b738dbd13e0658cf425d22"}],"nova/utils.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fc239bfb8812e95cc66202120fd9270a8b512974","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"    SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1221,"context_line":""},{"line_number":1222,"context_line":""},{"line_number":1223,"context_line":"CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"def get_cache_images_executor():"}],"source_content_type":"text/x-python","patch_set":2,"id":"267d83b8_43d33f80","line":1223,"updated":"2025-10-02 11:30:45.000000000","message":"as an aside we have a bunch of global reset in our base test case for globals \nlike this in cludign some protections to wait for greenthread to be complete\n\nhttps://github.com/openstack/nova/blob/master/nova/test.py#L279-L280\nhttps://github.com/openstack/nova/blob/master/nova/test.py#L310-L313\nhttps://github.com/openstack/nova/blob/master/nova/test.py#L323-L334\n\nshoudl we 1 add a reset_globals function to nova.utils and have it destroy all the executors and call that formthe basee testcase?\n\ngibi i know we talked about makeing shoure we between tests in general in the past but i dont quite recall what we currently do in terms of ensuring that the executre do not leak across test executions.\n\ni belive we are currently doing this here https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L1188-L1294\n\nbut the reset_globals in that fixture\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L1237-L1239\n\ndoes nto handel the new executor and i think that function really shoudl be in nova.utils rahter then messign with the global state form the fixture itself.\n\nwhat do we think.\n\nim +2 other then that","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5eb68144805da33c6dea9080ee32880e3b01fe2c","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"    SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1221,"context_line":""},{"line_number":1222,"context_line":""},{"line_number":1223,"context_line":"CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"def get_cache_images_executor():"}],"source_content_type":"text/x-python","patch_set":2,"id":"cd3c446a_acd3582c","line":1223,"in_reply_to":"267d83b8_43d33f80","updated":"2025-10-02 14:33:08.000000000","message":"I think the fixture currently resets the other executors, so this patch should have added this one to that list. I don\u0027t have a problem with a fixture modifying global state (as I think any one that does a mock does the same thing) so I\u0027d be fine with just adding this there.\n\nI will point out that my comment below would make resetting all of these easier and less failure-prone if we had generic functions to do it.","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"23d73a6fe8b42bafe063c9353ca1b16686406a42","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"    SCATTER_GATHER_EXECUTOR \u003d None"},{"line_number":1221,"context_line":""},{"line_number":1222,"context_line":""},{"line_number":1223,"context_line":"CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"def get_cache_images_executor():"}],"source_content_type":"text/x-python","patch_set":2,"id":"b4f6654f_6fc6cb5b","line":1223,"in_reply_to":"cd3c446a_acd3582c","updated":"2025-10-09 14:00:14.000000000","message":"I go with adding additional value to reset_globals()","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fc239bfb8812e95cc66202120fd9270a8b512974","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"def get_cache_images_executor():"},{"line_number":1227,"context_line":"    \"\"\"Returns the executor used for cache images operations.\"\"\""},{"line_number":1228,"context_line":"    global CACHE_IMAGES_EXECUTOR"},{"line_number":1229,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"4e939e76_1aabc9d2","line":1226,"updated":"2025-10-02 11:30:45.000000000","message":"i was going to suggest that we move away form manually doing global caching liek this and use funtools.cache\n\nand then do nova.utils.get_cache_images_executor.reset() to destroy it\n\nbut that would not actully call shutdown until the executor was garbage collected so as much a si dislike havign to repeat this i think this is proably the correct approch as is.","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5eb68144805da33c6dea9080ee32880e3b01fe2c","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"            \"The cache images thread pool %s is closed\","},{"line_number":1255,"context_line":"            CACHE_IMAGES_EXECUTOR.name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1258,"context_line":""},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"def _log_executor_stats(executor):"}],"source_content_type":"text/x-python","patch_set":2,"id":"0ff03619_17bad1ed","line":1257,"updated":"2025-10-02 14:33:08.000000000","message":"Feels like we could DRY up these get/destroy functions to be a single implementation with a named pool.","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"664851f5c208cd15636201e8153d8d7b2dc97b67","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"            \"The cache images thread pool %s is closed\","},{"line_number":1255,"context_line":"            CACHE_IMAGES_EXECUTOR.name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1258,"context_line":""},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"def _log_executor_stats(executor):"}],"source_content_type":"text/x-python","patch_set":2,"id":"f112debe_6a09e203","line":1257,"in_reply_to":"0ff03619_17bad1ed","updated":"2025-10-02 14:59:08.000000000","message":"ya that what i was thinking as well. \n\nbasicaly having a factory function that will allow us to define executors, cache them and register them in a collection dynamicly so that we can have a signel function that will clean up all the executors ensuring they shutdown.\n\nthen the fixture or base testcase can just call that reset_globals funtion.\nthat way the lifchcle management of the execurorts is behind a set of fuctions contianed in utils.py and the cmd modules or test fixtures can just call those without needign to knwo the exact details fo how that works.\n\ni didn\u0027t want to suggest that before checking if others felt the same that we shoudl abstract the creation and destruction fo the executors behind something so its easier to do without forgetting to adjust the tests.\n\nif we just adjust https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L1237-L1239 in this change to properly handle the new executor im fine with deferring the refactor to have a less error prone pattern to a seperate commit.","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cb6ace3d3e882a2c54e6ab76e19f6d0db6bc7c94","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"            \"The cache images thread pool %s is closed\","},{"line_number":1255,"context_line":"            CACHE_IMAGES_EXECUTOR.name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1258,"context_line":""},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"def _log_executor_stats(executor):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bd333aaf_7f09d841","line":1257,"in_reply_to":"84dc4843_b689a8e6","updated":"2025-10-09 16:06:43.000000000","message":"Yep, doesn\u0027t need to be in here.. I\u0027m just saying the symptom indicates that we should make this more modular.","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3385045e6df24a69c6ed514447b4d875472ae63a","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"            \"The cache images thread pool %s is closed\","},{"line_number":1255,"context_line":"            CACHE_IMAGES_EXECUTOR.name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1258,"context_line":""},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"def _log_executor_stats(executor):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7cec9d9a_32eaa967","line":1257,"in_reply_to":"bd333aaf_7f09d841","updated":"2026-01-11 03:42:21.000000000","message":"But where do we call/need this? As this executor used in conductor ComputeTaskManager which is all done after oslo.service forked the conductor services workers so we do not need to cleanup this right? or this is used before serviec is forked?","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"23d73a6fe8b42bafe063c9353ca1b16686406a42","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"            \"The cache images thread pool %s is closed\","},{"line_number":1255,"context_line":"            CACHE_IMAGES_EXECUTOR.name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    CACHE_IMAGES_EXECUTOR \u003d None"},{"line_number":1258,"context_line":""},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"def _log_executor_stats(executor):"}],"source_content_type":"text/x-python","patch_set":2,"id":"84dc4843_b689a8e6","line":1257,"in_reply_to":"f112debe_6a09e203","updated":"2025-10-09 14:00:14.000000000","message":"Can this be a follow up? This changes will have quite big diff","commit_id":"a8f97dcf119e5a254ef7752a20aac5c60fc8f9a5"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3385045e6df24a69c6ed514447b4d875472ae63a","unresolved":true,"context_lines":[{"line_number":1240,"context_line":"    return CACHE_IMAGES_EXECUTOR"},{"line_number":1241,"context_line":""},{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"def destroy_cache_images_executor():"},{"line_number":1244,"context_line":"    \"\"\"Closes the executor and resets the global to None to allow forked worker"},{"line_number":1245,"context_line":"    processes to properly init it."},{"line_number":1246,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"05740218_fb36a5d3","line":1243,"range":{"start_line":1243,"start_character":0,"end_line":1243,"end_character":36},"updated":"2026-01-11 03:42:21.000000000","message":"where do we call this? As this executor used in condiuctor","commit_id":"f6314d9027df8449366a1246830bb9bbdff79519"}]}