)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"af40c24c365a47c33fd2bef619c5dabbd7e3d61d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"bf477bc5_70a219bb","updated":"2026-06-17 14:18:38.000000000","message":"Clean API compatibility, I like the approach, the code is clean and the tests are good! I added two comments that are only suggestions so it\u0027s up to you to apply it or not.","commit_id":"ad6424b53ee6f9c9c8fe6f2107486dfa5c692491"}],"swift/common/concurrency.py":[{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"af40c24c365a47c33fd2bef619c5dabbd7e3d61d","unresolved":true,"context_lines":[{"line_number":296,"context_line":"        \"\"\"Drop-in replacement for eventlet.tpool running in the current"},{"line_number":297,"context_line":"        thread."},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        All calls to execute will run in the current thread and not in a"},{"line_number":300,"context_line":"        separate thread pool. Eventlet uses a threadpool to be able to yield"},{"line_number":301,"context_line":"        to other coros and not block the current one, but without eventlet"},{"line_number":302,"context_line":"        this is not needed - it is already running in a thread."}],"source_content_type":"text/x-python","patch_set":10,"id":"628f4058_812a7498","line":299,"updated":"2026-06-17 14:18:38.000000000","message":"Suggestion: I think there is still room for improvement for this docstring, maybe adding a clear WARNING message mentioning the lack of concurrency limits and the DoS implications?\n\n```\n\"\"\"Drop-in replacement for eventlet.tpool running in the current\nthread.\n\nAll calls to execute will run in the current thread and not in a\nseparate thread pool. Eventlet uses a threadpool to be able to yield\nto other coros and not block the current one, but without eventlet\nthis is not needed - it is already running in a thread.\n  \nWARNING: Unlike eventlet.tpool, this has NO CONCURRENCY LIMIT.\neventlet.tpool limits concurrent execution to the threadpool size\n(default 20). This Executor runs all execute() calls immediately in\nthe current thread. Code that relied on tpool\u0027s threadpool size to\nlimit concurrent execution (e.g., auth middleware limiting password\nhashing for DoS protection) must implement their own rate limiting\nwhen running in threading mode.\n\"\"\"\n```","commit_id":"ad6424b53ee6f9c9c8fe6f2107486dfa5c692491"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"59730f49067d680b1caa4531a274718d4df69ec3","unresolved":true,"context_lines":[{"line_number":222,"context_line":"    spawn_n \u003d spawn"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    # Class to replaceme eventlet.pools.Pool"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    class Pool(object):"},{"line_number":227,"context_line":"        \"\"\""},{"line_number":228,"context_line":"        Thread-safe connection pool replacement for eventlet.pools.Pool."}],"source_content_type":"text/x-python","patch_set":12,"id":"325349df_7ed13e38","line":225,"updated":"2026-07-09 07:24:28.000000000","message":"nit: can be removed","commit_id":"25dc6bc11a67a0d2c682e7789eb76cec990fa6b8"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"59730f49067d680b1caa4531a274718d4df69ec3","unresolved":true,"context_lines":[{"line_number":310,"context_line":"        def set_num_threads(self, *args, **kwargs):"},{"line_number":311,"context_line":"            pass"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        @staticmethod"},{"line_number":314,"context_line":"        def execute(func, *args, **kwargs):"},{"line_number":315,"context_line":"            return func(*args, **kwargs)"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    # No need for a threadpool when already running in threads."},{"line_number":318,"context_line":"    tpool \u003d Executor()"}],"source_content_type":"text/x-python","patch_set":12,"id":"361b6853_d60a19be","line":315,"range":{"start_line":313,"start_character":0,"end_line":315,"end_character":40},"updated":"2026-07-09 07:24:28.000000000","message":"In thread mode tpool.execute() just call function on caller thread. It not use bounded pool like eventlet.\n\nExample: tpool.execute(fdatasync, self._fd) in diskfile block Gunicorn worker during sync. Other request on same worker can stall.\n\nPlease document this in docstring so people understand difference from eventlet mode.\n\nProbable fix: use small bounded ThreadPoolExecutor for tpool.execute() in thread mode (like eventlet tpool), so disk work run in pool with limit and not unlimited inline on request thread.","commit_id":"25dc6bc11a67a0d2c682e7789eb76cec990fa6b8"}],"swift/common/threadpool.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"99626a5c8d69fa0f7e4c6bbdbfe51b401b40e04f","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            self.put(item)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"class Executor:"},{"line_number":89,"context_line":"    # No-op to be compatible with eventlet call"},{"line_number":90,"context_line":"    def set_num_threads(self, *args, **kwargs):"},{"line_number":91,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"8932775e_e07bc956","line":88,"updated":"2026-03-13 01:51:32.000000000","message":"We should probably include a docstring, if only to call out that all calls to `execute` will be run in the current thread.\n\nThat should also be prominent in any migration docs we might write -- I know there are auth middlewares that use (an inlining of) our old `ThreadPool` class. They\u0027ll do it for two reasons:\n\n1. To keep some compute-heavy workloads (like running scrypt with passwords) from jamming up the eventlet hub and\n2. To limit the overall number of compute-heavy workloads that a proxy may service -- don\u0027t want an auth DoS bringing down the whole proxy\n\nMoving to real threads eliminates 1, but not 2, so they definitely *shouldn\u0027t* be looking to swap to this.","commit_id":"c644e226f502b50cd07afa73eef037cb5df7b48c"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"7ded645a8c2ff8a1e226bb80d5527e0456eb16e9","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            self.put(item)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"class Executor:"},{"line_number":89,"context_line":"    # No-op to be compatible with eventlet call"},{"line_number":90,"context_line":"    def set_num_threads(self, *args, **kwargs):"},{"line_number":91,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"cb9cec3c_5b66302c","line":88,"in_reply_to":"8932775e_e07bc956","updated":"2026-03-13 10:48:20.000000000","message":"Will add a docstring in the next patchset.\n\nThat said, do you think it might more sense to use a real threadpool?","commit_id":"c644e226f502b50cd07afa73eef037cb5df7b48c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2d3965a4a3f8470bce27d2777d106e10c3459728","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            self.put(item)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"class Executor:"},{"line_number":89,"context_line":"    # No-op to be compatible with eventlet call"},{"line_number":90,"context_line":"    def set_num_threads(self, *args, **kwargs):"},{"line_number":91,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"f2bede54_4af823ed","line":88,"in_reply_to":"cb9cec3c_5b66302c","updated":"2026-03-13 15:37:24.000000000","message":"Within swift? I don\u0027t think so -- I guess there was a (per process) semaphore for number of concurrent disk accesses, but ops can get a similar effect with max_clients. Having it just pass through when we\u0027re not dealing with eventlet seems fine. (I think?)","commit_id":"c644e226f502b50cd07afa73eef037cb5df7b48c"}],"test/unit/common/test_concurrency.py":[{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"af40c24c365a47c33fd2bef619c5dabbd7e3d61d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":10,"id":"371aece8_9c5f0932","updated":"2026-06-17 14:18:38.000000000","message":"Suggestion: The set_num_threads() method is implemented but not tested. While it\u0027s a no-op, we could verify it\u0027s callable and doesn\u0027t raise errors.\n\nMaybe something like:\n```\n  def test_set_num_threads_noop(self):\n      # Should be callable and not raise\n      tpool.set_num_threads(10)\n      tpool.set_num_threads(num_threads\u003d5)\n      # verify it doesn\u0027t error\n```","commit_id":"ad6424b53ee6f9c9c8fe6f2107486dfa5c692491"}],"test/unit/common/test_threadpool.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"99626a5c8d69fa0f7e4c6bbdbfe51b401b40e04f","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        def fail():"},{"line_number":109,"context_line":"            raise Exception(\u0027reason\u0027)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        with self.assertRaises(Exception):"},{"line_number":112,"context_line":"            tpool.execute(fail)"}],"source_content_type":"text/x-python","patch_set":2,"id":"abac09e4_773ab285","line":111,"range":{"start_line":111,"start_character":31,"end_line":111,"end_character":40},"updated":"2026-03-13 01:51:32.000000000","message":"This becomes a **lot** stronger if we raise some custom exception defined just for this test. As it is, this still passes with\n```\ndiff --git a/test/unit/common/test_threadpool.py b/test/unit/common/test_threadpool.py\nindex 36b180643..c157b36c3 100644\n--- a/test/unit/common/test_threadpool.py\n+++ b/test/unit/common/test_threadpool.py\n@@ -106,6 +106,7 @@ class TestTpool(unittest.TestCase):\n\n     def test_exception(self):\n         def fail():\n+            asdf\n             raise Exception(\u0027reason\u0027)\n\n         with self.assertRaises(Exception):\n```\napplied.","commit_id":"c644e226f502b50cd07afa73eef037cb5df7b48c"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"7ded645a8c2ff8a1e226bb80d5527e0456eb16e9","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        def fail():"},{"line_number":109,"context_line":"            raise Exception(\u0027reason\u0027)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        with self.assertRaises(Exception):"},{"line_number":112,"context_line":"            tpool.execute(fail)"}],"source_content_type":"text/x-python","patch_set":2,"id":"2777a492_adf11b52","line":111,"range":{"start_line":111,"start_character":31,"end_line":111,"end_character":40},"in_reply_to":"abac09e4_773ab285","updated":"2026-03-13 10:48:20.000000000","message":"Indeed, that makes a lot of sense - will include this in the next patchset as well.","commit_id":"c644e226f502b50cd07afa73eef037cb5df7b48c"}]}
