)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cb3319e1815bf14704eb0b91421712bc678be5a2","unresolved":true,"context_lines":[{"line_number":11,"context_line":"Close connections more promptly rather than waiting on GC."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"See also: https://github.com/python/cpython/issues/135117"},{"line_number":14,"context_line":"and https://github.com/python/cpython/issues/144345"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I1e0052717d9b25f97e3cf58508d005f43d306267"},{"line_number":17,"context_line":"Signed-off-by: Tim Burke \u003ctim.burke@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9c064625_9322d106","line":14,"updated":"2026-04-23 04:28:37.000000000","message":"In the end of thread ``cpython/issues/144345``, there was a lru_cache fix proposed once at https://github.com/python/cpython/issues/148459, and then it was closed as \"not planned\". ☹️ If we don\u0027t know if and when a fix from cpython community will arrive, we do need our own fix.","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee8102cf7b5f5449e0faf965043efbda62a384b0","unresolved":false,"context_lines":[{"line_number":11,"context_line":"Close connections more promptly rather than waiting on GC."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"See also: https://github.com/python/cpython/issues/135117"},{"line_number":14,"context_line":"and https://github.com/python/cpython/issues/144345"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I1e0052717d9b25f97e3cf58508d005f43d306267"},{"line_number":17,"context_line":"Signed-off-by: Tim Burke \u003ctim.burke@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"32e2cfe3_aeedbb7c","line":14,"in_reply_to":"9c064625_9322d106","updated":"2026-05-27 17:57:32.000000000","message":"Acknowledged","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cb3319e1815bf14704eb0b91421712bc678be5a2","unresolved":true,"context_lines":[{"line_number":13,"context_line":"See also: https://github.com/python/cpython/issues/135117"},{"line_number":14,"context_line":"and https://github.com/python/cpython/issues/144345"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I1e0052717d9b25f97e3cf58508d005f43d306267"},{"line_number":17,"context_line":"Signed-off-by: Tim Burke \u003ctim.burke@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"54a09d9a_f402b272","line":16,"updated":"2026-04-23 04:28:37.000000000","message":"@tburke@nvidia.com had another approach in the first round: https://review.opendev.org/c/openstack/swift/+/980388\n\nThe laze umount lets the kernel defer unmount until all fds close. The sqlite fd is still open, it just doesn\u0027t block the caller of umount. So basically lazy umount is just a workaround to have those probe tests pass with py3.11+, while the approach in this patch is the structurally correct response to this cpython regression.","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee8102cf7b5f5449e0faf965043efbda62a384b0","unresolved":false,"context_lines":[{"line_number":13,"context_line":"See also: https://github.com/python/cpython/issues/135117"},{"line_number":14,"context_line":"and https://github.com/python/cpython/issues/144345"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I1e0052717d9b25f97e3cf58508d005f43d306267"},{"line_number":17,"context_line":"Signed-off-by: Tim Burke \u003ctim.burke@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"a841161e_09cf0014","line":16,"in_reply_to":"54a09d9a_f402b272","updated":"2026-05-27 17:57:32.000000000","message":"Acknowledged","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"13a66701e90ccca22d34b818ff9cc39faf247ce9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9d881caa_7d885fae","updated":"2026-03-25 18:33:21.000000000","message":"I think this is a great idea!  And absolutly necessary given the CPython bugs.  It\u0027ll take awhile to get ALL the usage of brokers under context managers; so starting with the account and container servers a MVP priority first step is 100% the right judgement call for initial scope.\n\nMASTER:\n\n```\nvagrant@saio:~$ grep -i pretty /etc/*release\n/etc/os-release:PRETTY_NAME\u003d\"Ubuntu 24.04.2 LTS\"\nvagrant@saio:~$ python3 --version\nPython 3.12.3\nvagrant@saio:~$ pytest swift/test/probe/test_reconstructor_revert.py -x\n...\nFAILED swift/test/probe/test_reconstructor_revert.py::TestReconstructorRevert::test_delete_propagate - subprocess.CalledProcessError: Command \u0027[\u0027sudo\u0027, \u0027umount\u0027, \u0027/srv/node3/sdb3\u0027]\u0027 returned non-zero exit status 32.\n```\n\nthis change:\n\n```\nvagrant@saio:~$ pytest swift/test/probe/test_reconstructor_revert.py -x\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d test session starts \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nplatform linux -- Python 3.12.3, pytest-9.0.2, pluggy-1.6.0 -- /usr/bin/python3\ncachedir: .pytest_cache\nrootdir: /home/vagrant/swift\nconfigfile: tox.ini\nplugins: cov-7.1.0\ncollected 4 items                                                                                                                                                                                                                                      \n\nswift/test/probe/test_reconstructor_revert.py::TestReconstructorRevert::test_delete_propagate PASSED                                                                                                                                             [ 25%]\nswift/test/probe/test_reconstructor_revert.py::TestReconstructorRevert::test_handoff_non_durable PASSED                                                                                                                                          [ 50%]\nswift/test/probe/test_reconstructor_revert.py::TestReconstructorRevert::test_reconstruct_from_reverted_fragment_archive PASSED                                                                                                                   [ 75%]\nswift/test/probe/test_reconstructor_revert.py::TestReconstructorRevert::test_revert_object PASSED                                                                                                                                                [100%]\n\n...\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d 4 passed, 1 warning in 60.15s (0:01:00) \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n```\n\nI think it needs more tests - maybe even something in probe that goes after specifically the GC/unmount problem of the container/account server using `lsof`","commit_id":"fcc316a95c46412860b9d577657f2b5f1065f594"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"030c9244787950890190bd6ed2749eac4b2df212","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e81ef763_b0cc3891","updated":"2026-03-25 21:01:49.000000000","message":"I can\u0027t decide whether these new tests are incredibly clever or just insane. But they helped me notice a subtle issue with sub-brokers!","commit_id":"be9c050e48feefc3cf21744297dd32abcf3411c3"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c4dc9499e7300936fc43478dc249de2e7a099971","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"755beca2_fba7876a","updated":"2026-03-26 16:45:34.000000000","message":"I run into the same with probe tests on feature/threaded and had a similar wip patch. Will look more closer later, but happy to see this one!","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b143836e_6d427844","updated":"2026-03-30 23:35:11.000000000","message":"I would prefer we apply the context manager thoroughly and surgically in JUST the servers FIRST before we start throwing \"use brokers as context managers\" around willy-nilly b/c it will make it easier to verify that we\u0027re adding appropriate test coverage during review.\n\nAlthough it\u0027s possible this is already the the minimum change if the goal is to ensure container.server can successfully unmount drives the common.db and common.backend might need to support the sub_broker context manager pattern.\n\nWe could try to downscope even further to just start with the account server while we figure out the most clear story for testing sharding-container sub-broker connection handling in container.backend\n\nI think a targeted probe test that - or even just some `assert_can_unmount_drives` in existing account/container tests would go a long way to ensuring test coverage and maintaining correctness.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fbfdfef93a4c7ca362c03688073ff43f453a8c74","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c6d970f8_428fd0f8","in_reply_to":"99f653ad_f360bddc","updated":"2026-04-09 22:58:08.000000000","message":"I don\u0027t think EC probe tests using unmount drive and tangentially failing if/when it happens to also have the container being used in the test is sufficient to meet MY confidence bar that we can maintain this new/required behavior sufficiently to claim \"support\" of \u003e\u003dpy3.11\n\n... but I could be wrong and other\u0027s might agree this is good enough for now; if I wanted to get \"really comfortable that this is good enough\" - my next step would be working on the probe test.  I\u0027ll try to loop around to that next chance I get and don\u0027t feel terribly bad about continuing to use py3.10 in my dev (same as my prod) until we do the other \"needed\" work.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d142b8231d0ed32a006aeb5ca0e64dcce7315335","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"df3c1662_4ff5631d","in_reply_to":"ac0a78be_10af7a2c","updated":"2026-04-13 19:44:44.000000000","message":"Oh, and FWIW: https://governance.openstack.org/tc/reference/runtimes/2026.2.html#python says we should be targetting py311-py313, with an eye toward getting py314 support this cycle. We can definitely keep py310 support, but trying to walk back and say we\u0027ll **not** support py311+ until we feel comfortable running it in *our* prod just doesn\u0027t seem like a reasonable option.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"117b05eae98b5bd885d4c327f931662fce2c1b4e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"99f653ad_f360bddc","in_reply_to":"b143836e_6d427844","updated":"2026-04-09 18:12:54.000000000","message":"\u003e I would prefer we apply the context manager thoroughly and surgically in JUST the servers FIRST\n\nNot sure that\u0027ll do -- and the tests demonstrate it. Backing out just `swift/container/backend.py`, I see both `test/unit/container/test_server.py::TestContainerController::test_deleted_headers` and `test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_deleted_headers` fail like\n```\n\u003e           assert CloseTracker.open_conns \u003d\u003d [], CloseTracker.open_conns\n                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\nE           AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf2718c772940\u003e created at\nE             File \"/vagrant/swift/swift/common/swob.py\", line 1154, in get_response\nE               status, headers, app_iter \u003d self.call_application(application)\nE             File \"/vagrant/swift/swift/common/swob.py\", line 1138, in call_application\nE               app_iter \u003d application(self.environ, start_response)\nE             File \"/vagrant/swift/swift/container/server.py\", line 1032, in __call__\nE               res \u003d getattr(self, req.method)(req)\nE             File \"/vagrant/swift/swift/common/base_storage_server.py\", line 71, in _timing_stats\nE               resp \u003d func(ctrl, *args, **kwargs)\nE             File \"/vagrant/swift/swift/container/server.py\", line 385, in DELETE\nE               if not broker.empty():\nE             File \"/vagrant/swift/swift/container/backend.py\", line 736, in empty\nE               if not all(broker._empty() for broker in self.get_brokers()):\nE             File \"/vagrant/swift/swift/container/backend.py\", line 736, in \u003cgenexpr\u003e\nE               if not all(broker._empty() for broker in self.get_brokers()):\nE             File \"/vagrant/swift/swift/container/backend.py\", line 711, in _empty\nE               with self.get() as conn:\nE             File \"/usr/lib/python3.12/contextlib.py\", line 137, in __enter__\nE               return next(self.gen)\nE             File \"/vagrant/swift/swift/common/db.py\", line 550, in get\nE               self.conn \u003d get_db_connection(self.db_file, self.timeout,\nE             File \"/vagrant/swift/swift/common/db.py\", line 204, in get_db_connection\nE               conn \u003d sqlite3.connect(path, check_same_thread\u003dFalse,\nE           \u003e]\n```\n\u003e I think a targeted probe test that - or even just some `assert_can_unmount_drives` in existing account/container tests would go a long way to ensuring test coverage and maintaining correctness.\n\nThe existing probe tests *already* fail on py311+ (due to an inability to unmount) -- that\u0027s what prompted the patch.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d6cfe853960fd7ecb6255406a93b1355892428ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ac0a78be_10af7a2c","in_reply_to":"c6d970f8_428fd0f8","updated":"2026-04-10 00:28:06.000000000","message":"\u003e we can maintain this new/required behavior\n\nIt\u0027s still not at all clear to me that it *is* required. Required for our tests to pass, yes; required for ops to get their jobs done? No. If they need a drive unmounted, they\u0027ll get that drive unmounted (likely with `--lazy`). Odds are there are plenty of open files *anyway*, because it\u0027s an active cluster.\n\n\u003e claim \"support\" of \u003e\u003dpy3.11\n\nShip has sailed there: https://github.com/openstack/swift/blob/master/setup.cfg#L29-L32\n\n\u003e don\u0027t feel terribly bad about continuing to use py3.10 in my dev (same as my prod)\n\nI\u0027m not worried about *your* dev env, I\u0027m worried about *mine*, and our ability to upgrade the gate, and new contributors\u0027 experience trying to onboard, and ...\n\nI\u0027ve seen Swift stuck in time on a python version before. I don\u0027t want to see it happen again.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fbfdfef93a4c7ca362c03688073ff43f453a8c74","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"84a51c1e_8594e378","updated":"2026-04-09 22:58:08.000000000","message":"I don\u0027t have any strong objections here;  I think the ctx-manager design is sound, and the unit-testing \"check close\" pattern acceptable.\n\nI would prefer to see a probe test - I think I\u0027d found some un-tested branches in container.backend (but you may have already fixed those) and I\u0027m not AWARE of any others - if i\u0027d had to go looking for them in order to +2 - I\u0027d probably write a probetest to smoke out a framework to sanity check the basics:\n\n```\nfor root, shard in sample_dbs:\n    for verb in HEAD, GET, PUT:\n        do_check_lsof_and_unmount(verb)\n```\n\nmaybe claude could write it!? No blockers from me otherwise!  GLHF","commit_id":"3baac2111c7c7a51b6019320ea18d0e236173df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d6cfe853960fd7ecb6255406a93b1355892428ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"7f6c63f3_07854b94","in_reply_to":"84a51c1e_8594e378","updated":"2026-04-10 00:28:06.000000000","message":"Man I hate touching probe tests -- it takes 30s to run *just one test!*\n\n\u003e I\u0027d probably write a probetest to smoke out a framework to sanity check the basics\n\nI trust unit tests to have much better coverage for that sort of all-verbs, all-db-states testing.","commit_id":"3baac2111c7c7a51b6019320ea18d0e236173df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"004a52933d9efa9ec616ec8fec13eac498a730d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7329d170_22425e7f","updated":"2026-04-10 16:20:16.000000000","message":"So here\u0027s a fun diff to apply ahead of running probe tests:\n```\ndiff --git a/swift/account/server.py b/swift/account/server.py\nindex 87ea71f8a..e6216915b 100644\n--- a/swift/account/server.py\n+++ b/swift/account/server.py\n@@ -304,6 +304,11 @@ class AccountController(BaseStorageServer):\n             return HTTPNoContent(request\u003dreq)\n\n     def __call__(self, env, start_response):\n+        from test.unit import check_db_connections_get_closed\n+        with check_db_connections_get_closed():\n+            return self.___call__(env, start_response)\n+\n+    def ___call__(self, env, start_response):\n         start_time \u003d time.time()\n         req \u003d Request(env)\n         self.logger.txn_id \u003d req.headers.get(\u0027x-trans-id\u0027, None)\ndiff --git a/swift/container/server.py b/swift/container/server.py\nindex ead8266ed..ce84ca630 100644\n--- a/swift/container/server.py\n+++ b/swift/container/server.py\n@@ -1018,6 +1018,11 @@ class ContainerController(BaseStorageServer):\n             return HTTPNoContent(request\u003dreq)\n\n     def __call__(self, env, start_response):\n+        from test.unit import check_db_connections_get_closed\n+        with check_db_connections_get_closed():\n+            return self.___call__(env, start_response)\n+\n+    def ___call__(self, env, start_response):\n         start_time \u003d time.time()\n         req \u003d Request(env)\n         self.logger.txn_id \u003d req.headers.get(\u0027x-trans-id\u0027, None)\n```\nLooks like we don\u0027t really exercise `REPLICATE` in the `test_server.py` tests!? \u0027Cause with that diff, I\u0027m still seeing open connections without something like\n```\ndiff --git a/swift/common/db_replicator.py b/swift/common/db_replicator.py\nindex 37403bd03..996c6940a 100644\n--- a/swift/common/db_replicator.py\n+++ b/swift/common/db_replicator.py\n@@ -980,8 +980,8 @@ class ReplicatorRpc(object):\n             mkdirs(os.path.join(self.root, drive, \u0027tmp\u0027))\n             if not self._db_file_exists(db_file):\n                 return HTTPNotFound()\n-            return getattr(self, op)(\n-                self.broker_class(db_file, logger\u003dself.logger), args)\n+            with self.broker_class(db_file, logger\u003dself.logger) as broker:\n+                return getattr(self, op)(broker, args)\n\n     @contextmanager\n     def debug_timing(self, name):\n```","commit_id":"85cbe43bf6d900c4a8fa2b49ea7495d6847b1888"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9c9a293d69b39a04e5686789a6e7a8dd3c428187","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"597a7775_805edef6","in_reply_to":"7329d170_22425e7f","updated":"2026-04-10 22:07:02.000000000","message":"Ugh -- but it spews false-positives unless you get the conn-tracking into corolocals... doable, but annoying.","commit_id":"85cbe43bf6d900c4a8fa2b49ea7495d6847b1888"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9c9a293d69b39a04e5686789a6e7a8dd3c428187","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"daee2287_d29984d4","updated":"2026-04-10 22:07:02.000000000","message":"Man, even trying to limit this to just the WSGI servers reveals there\u0027s a *lot* of surface there...","commit_id":"024f114d977d836999cdf6f4e50aff9ab426e4e0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4766d4382e12ea1b54fe9f90c44aca83a245ed75","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"076f8b9e_ce2d4ee9","updated":"2026-04-13 15:28:07.000000000","message":"recheck\n\nFailures in `tempest.api.identity.admin.v3.test_roles.RolesV3TestJSON.test_role_create_update_show_list` and `tempest.api.identity.admin.v3.test_services.ServicesTestJSON.test_create_update_get_service` -- nothing to do with us.","commit_id":"4d7825132682549e479d80cf7d1896ba46a72a09"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cb3319e1815bf14704eb0b91421712bc678be5a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"16332bb8_8425210b","updated":"2026-04-23 04:28:37.000000000","message":"On my Ubuntu 24 box,\n```\nvagrant@saio:~/swift$ python3 --version\nPython 3.12.3\n```\n\nwith the master branch, probe tests are running into failures due to device umount failure (caused by those lingering open DB connections) \n```\npytest test/probe/test_reconstructor_rebuild.py::TestReconstructorRebuild::test_sync_unexpired_object_metadata\n....\n\u003e       self.kill_drive(post_fail_path)\n\ntest/probe/test_reconstructor_rebuild.py:333:\n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _\ntest/probe/common.py:505: in kill_drive\n    subprocess.run([\"sudo\", \"umount\", device], check\u003dTrue)\n....\nFAILED test/probe/test_reconstructor_rebuild.py::TestReconstructorRebuild::test_sync_unexpired_object_metadata - subprocess.CalledProcessError: Command \u0027[\u0027sudo\u0027, \u0027umount\u0027, \u0027/srv/node2/sdb2\u0027]\u0027 returned non-zero exit status 32.\n```\n\nand this patch fixed this for me, nice!!\n```\ntest/probe/test_reconstructor_rebuild.py::TestReconstructorRebuild::test_sync_unexpired_object_metadata PASSED           [100%]\n```","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"991bef9dd922013dde4191a5391ab390fc0e4150","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"1f949e0d_089989c9","updated":"2026-05-07 07:13:52.000000000","message":"Looks clean to me. And like how its cleans up some open connection leaks.\n\nMaybe a follow up to update other broker usages throughout the code, like sharder etc. But targeting the wgi servers to me is a good targetted scope.","commit_id":"ab106e5f65561748c8bd8c0e4a1a3771f5f80aa2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e0bfacc8baef833dcc76640d4e9ab4617c8c3fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"83434068_7dc05eef","updated":"2026-05-29 12:21:23.000000000","message":"I think this is a helpful improvement to stabilize probetests!","commit_id":"05c11f44beef85a357ed5a2d98097da79a59dcdf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"61effcb7ccf066ae90da55cc7f31a848fc713129","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"ad61f455_acdce0cb","updated":"2026-06-01 15:12:34.000000000","message":"i have been running some \u003epy3.10 vsaio vms again lately and this is starting to annoy me; probably better to merge than not?\n\n```\n• Findings\n\n  No blocking findings in review 980785 (vsaio-2, Close DB connections in metadata servers).\n  \n  One residual risk: the changed probe was flaky here.\n```\n\nOtherwise, I may retreat back to an older ubuntu:\n\n```\n\n• Ran the flaky probe 10 times in slot vsaio-2 on review 980785.\n\n  Result: 7 passed, 3 failed. Observed reliability in this sample: 70% pass / 30% fail.\n\n  All three failures were the same assertion:\n\n  test/probe/test_account_failures.py:112\n  expected x-account-container-count \u003d\u003d \u00271\u0027, got \u00272\u0027\n\n  Failed runs: 02, 03, 10.\n\n  Command used:\n  poetry run vsaio-pool --slot vsaio-2 vagrant ssh -c \u0027cd /vagrant/swift \u0026\u0026 ... pytest -q --tb\u003dshort --disable-warnings test/probe/test_account_failures.py::TestAccountFailures::test_main ...\u0027\n```\n\nor for humans:\n\n```\nfor i in {1..5}; do pytest --disable-warnings swift/test/probe/test_account_failures.py::TestAccountFailures::test_main -vsx; if [ $? -ne 0 ]; then break; fi; done\n```\n\n\n```\n        # Assert account level knows container1 is gone but doesn\u0027t know about\n        #   container2/object2 yet\n        headers, containers \u003d client.get_account(self.url, self.token)\n\u003e       self.assertEqual(headers[\u0027x-account-container-count\u0027], \u00271\u0027)\nE       AssertionError: \u00272\u0027 !\u003d \u00271\u0027\nE       - 2\nE       + 1\n\ntest/probe/test_account_failures.py:112: AssertionError\n```","commit_id":"05c11f44beef85a357ed5a2d98097da79a59dcdf"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b6b3e8097e746693b416ac77447204f52fa68c23","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"35c250e8_8b3ab81b","updated":"2026-06-02 10:38:02.000000000","message":"We have three +2 now, merging this one.","commit_id":"5e283c47182ae365676d204569d72d1eb7f42a8d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ce18ee39a12d1cb7d047607270ff15c7da42a12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"a28e9f15_eeecb498","updated":"2026-06-01 20:07:13.000000000","message":"was this test just already flaky on master?\n\n990984: sq? Stabilize account failure probe | https://review.opendev.org/c/openstack/swift/+/990984","commit_id":"5e283c47182ae365676d204569d72d1eb7f42a8d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2dfb255d4e5697799c5d18c37ca72a1096061598","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"62bc97ac_31f9a1dd","in_reply_to":"a28e9f15_eeecb498","updated":"2026-06-02 22:46:12.000000000","message":"This patch maybe made things worse -- taking down the disk also takes down a container server. But you seemed so adamant about wanting a probe test to exercise this directly and explicitly!","commit_id":"5e283c47182ae365676d204569d72d1eb7f42a8d"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9a1e71164b0fccd2925d19779d772f5395cdb787","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"c8af9db5_3c15375d","updated":"2026-06-26 16:23:56.000000000","message":"As mentioned earlier - got multiple +2s, and this becomes even more important without eventlet. Let\u0027s merge it eventually!","commit_id":"245ee33880477e61c5efc267adb17f44c11d153b"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"303d96364f13a6aebb73d4476b2f53c728db760f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"17b6c0b0_4917c495","updated":"2026-06-26 12:21:35.000000000","message":"recheck","commit_id":"245ee33880477e61c5efc267adb17f44c11d153b"}],"swift/account/server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"13a66701e90ccca22d34b818ff9cc39faf247ce9","unresolved":true,"context_lines":[{"line_number":137,"context_line":"            if broker.is_deleted():"},{"line_number":138,"context_line":"                return self._deleted_response(broker, req, HTTPNotFound)"},{"line_number":139,"context_line":"            broker.delete_db(req_timestamp.internal)"},{"line_number":140,"context_line":"            return self._deleted_response(broker, req, HTTPNoContent)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def _update_metadata(self, req, broker, req_timestamp):"},{"line_number":143,"context_line":"        metadata \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"45441928_da6e7c57","line":140,"updated":"2026-03-25 18:33:21.000000000","message":"not clear from this diff is `self._deleted_repsonse` is *using* the broker or not?\n\nI worry that I can write either of these diffs:\n\n```\ndiff --git a/swift/account/server.py b/swift/account/server.py\nindex 87ea71f8aa..83f144c06f 100644\n--- a/swift/account/server.py\n+++ b/swift/account/server.py\n@@ -136,8 +136,8 @@ class AccountController(BaseStorageServer):\n         with self._get_account_broker(drive, part, account) as broker:\n             if broker.is_deleted():\n                 return self._deleted_response(broker, req, HTTPNotFound)\n-            broker.delete_db(req_timestamp.internal)\n-            return self._deleted_response(broker, req, HTTPNoContent)\n+        broker.delete_db(req_timestamp.internal)\n+        return self._deleted_response(broker, req, HTTPNoContent)\n \n     def _update_metadata(self, req, broker, req_timestamp):\n         metadata \u003d {\n```\n\n\n... but this one does not:\n\n```\ndiff --git a/swift/account/server.py b/swift/account/server.py\nindex 87ea71f8aa..4eea37446d 100644\n--- a/swift/account/server.py\n+++ b/swift/account/server.py\n@@ -137,7 +137,7 @@ class AccountController(BaseStorageServer):\n             if broker.is_deleted():\n                 return self._deleted_response(broker, req, HTTPNotFound)\n             broker.delete_db(req_timestamp.internal)\n-            return self._deleted_response(broker, req, HTTPNoContent)\n+        return self._deleted_response(broker, req, HTTPNoContent)\n \n     def _update_metadata(self, req, broker, req_timestamp):\n         metadata \u003d {\n```\n\n... and in neither case do your unittests blow up?\n\nMaybe try something like this:\n\n```\ndiff --git a/swift/common/db.py b/swift/common/db.py\nindex 3d0f2f8c4b..050c9f6a76 100644\n--- a/swift/common/db.py\n+++ b/swift/common/db.py\n@@ -341,11 +341,18 @@ class DatabaseBroker(object):\n         self.container \u003d container\n         self._db_version \u003d -1\n         self.skip_commits \u003d skip_commits\n+        # _is_closed is trinary state\n+        # None means \"not a context manager\"\n+        # True means under-the-context-manager\n+        # False means broker is done-zo\n+        self._is_closed \u003d None\n \n     def __enter__(self):\n+        self._is_closed \u003d False\n         return self\n \n     def __exit__(self, *a, **kw):\n+        self._is_closed \u003d True\n         if self.conn:\n             self.conn.close()\n             self.conn \u003d None\n@@ -545,6 +552,8 @@ class DatabaseBroker(object):\n     @contextmanager\n     def get(self):\n         \"\"\"Use with the \"with\" statement; returns a database connection.\"\"\"\n+        if self._is_closed is True:\n+            raise RuntimeError(\u0027You have exited the context manager\u0027)\n         if not self.conn:\n             if os.path.exists(self.db_file):\n                 try:\n```\n\n... with associated tests to help maintain code that has started to adopt the context manager pattern?","commit_id":"fcc316a95c46412860b9d577657f2b5f1065f594"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"117b05eae98b5bd885d4c327f931662fce2c1b4e","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            if broker.is_deleted():"},{"line_number":138,"context_line":"                return self._deleted_response(broker, req, HTTPNotFound)"},{"line_number":139,"context_line":"            broker.delete_db(req_timestamp.internal)"},{"line_number":140,"context_line":"            return self._deleted_response(broker, req, HTTPNoContent)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def _update_metadata(self, req, broker, req_timestamp):"},{"line_number":143,"context_line":"        metadata \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"0becece2_51d3d1d6","line":140,"in_reply_to":"45441928_da6e7c57","updated":"2026-04-09 18:12:54.000000000","message":"Both those diffs are caught by unit tests now. FWIW, `_deleted_response` *does* use the broker, calling `broker.is_status_deleted()`.\n\nAs to the tri-state: in general, I find `var is True` or `var is False` a code smell. I\u0027m also hesitant to add a `RuntimeError` to brokers -- do we really want to risk only catching them in prod?","commit_id":"fcc316a95c46412860b9d577657f2b5f1065f594"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                req.headers.get(\u0027X-Backend-Storage-Policy-Index\u0027, 0)"},{"line_number":169,"context_line":"            if \u0027x-trans-id\u0027 in req.headers:"},{"line_number":170,"context_line":"                pending_timeout \u003d 3"},{"line_number":171,"context_line":"            with self._get_account_broker("},{"line_number":172,"context_line":"                    drive, part, account,"},{"line_number":173,"context_line":"                    pending_timeout\u003dpending_timeout) as broker:"},{"line_number":174,"context_line":"                if account.startswith(self.auto_create_account_prefix) and \\"}],"source_content_type":"text/x-python","patch_set":15,"id":"a87430f6_aad1c081","line":171,"updated":"2026-05-29 04:55:44.000000000","message":"here is another opportunity to improve readability, similar to the pre-factoring patch to the container server delete path: https://review.opendev.org/c/openstack/swift/+/985972/1\n\nI pushed up a refactoring patch as follow-up at: https://review.opendev.org/c/openstack/swift/+/990388/","commit_id":"ab106e5f65561748c8bd8c0e4a1a3771f5f80aa2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1e41e18ba5e81c5dd5d2975b639a6553095f4d85","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                req.headers.get(\u0027X-Backend-Storage-Policy-Index\u0027, 0)"},{"line_number":169,"context_line":"            if \u0027x-trans-id\u0027 in req.headers:"},{"line_number":170,"context_line":"                pending_timeout \u003d 3"},{"line_number":171,"context_line":"            with self._get_account_broker("},{"line_number":172,"context_line":"                    drive, part, account,"},{"line_number":173,"context_line":"                    pending_timeout\u003dpending_timeout) as broker:"},{"line_number":174,"context_line":"                if account.startswith(self.auto_create_account_prefix) and \\"}],"source_content_type":"text/x-python","patch_set":15,"id":"d3f21cad_7a7780ad","line":171,"in_reply_to":"a87430f6_aad1c081","updated":"2026-05-29 23:25:53.000000000","message":"Love it, thanks!","commit_id":"ab106e5f65561748c8bd8c0e4a1a3771f5f80aa2"}],"swift/common/db_replicator.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cb3319e1815bf14704eb0b91421712bc678be5a2","unresolved":true,"context_lines":[{"line_number":681,"context_line":"        responses \u003d []"},{"line_number":682,"context_line":"        broker \u003d None"},{"line_number":683,"context_line":"        try:"},{"line_number":684,"context_line":"            broker \u003d self.brokerclass(object_file, pending_timeout\u003d30,"},{"line_number":685,"context_line":"                                      logger\u003dself.logger)"},{"line_number":686,"context_line":"            self._reclaim(broker, now)"},{"line_number":687,"context_line":"            info \u003d broker.get_replication_info()"}],"source_content_type":"text/x-python","patch_set":13,"id":"f1f124b8_8ea2c472","line":684,"updated":"2026-04-23 04:28:37.000000000","message":"seems like this place of new ``self.brokerclass`` was missed?","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":false,"context_lines":[{"line_number":681,"context_line":"        responses \u003d []"},{"line_number":682,"context_line":"        broker \u003d None"},{"line_number":683,"context_line":"        try:"},{"line_number":684,"context_line":"            broker \u003d self.brokerclass(object_file, pending_timeout\u003d30,"},{"line_number":685,"context_line":"                                      logger\u003dself.logger)"},{"line_number":686,"context_line":"            self._reclaim(broker, now)"},{"line_number":687,"context_line":"            info \u003d broker.get_replication_info()"}],"source_content_type":"text/x-python","patch_set":13,"id":"719d06b8_d549da4f","line":684,"in_reply_to":"0acc0696_5c983e6b","updated":"2026-05-29 04:55:44.000000000","message":"yeah, makes sense, the scope of this patch is \"Close DB connections in metadata servers\".","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"37bb98ea31db308792c297320dfe03a3cd2938d9","unresolved":true,"context_lines":[{"line_number":681,"context_line":"        responses \u003d []"},{"line_number":682,"context_line":"        broker \u003d None"},{"line_number":683,"context_line":"        try:"},{"line_number":684,"context_line":"            broker \u003d self.brokerclass(object_file, pending_timeout\u003d30,"},{"line_number":685,"context_line":"                                      logger\u003dself.logger)"},{"line_number":686,"context_line":"            self._reclaim(broker, now)"},{"line_number":687,"context_line":"            info \u003d broker.get_replication_info()"}],"source_content_type":"text/x-python","patch_set":13,"id":"0acc0696_5c983e6b","line":684,"in_reply_to":"f1f124b8_8ea2c472","updated":"2026-04-23 15:13:11.000000000","message":"Was trying to limit the scope to just what the WSGI servers can actually hit. This is only used by the replicator.\n\nThough I suppose I should work on a follow-up to cover everybody... sharder will surely also need updates... seems like testing will be a pain, though.","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":false,"context_lines":[{"line_number":980,"context_line":"            mkdirs(os.path.join(self.root, drive, \u0027tmp\u0027))"},{"line_number":981,"context_line":"            if not self._db_file_exists(db_file):"},{"line_number":982,"context_line":"                return HTTPNotFound()"},{"line_number":983,"context_line":"            with self.broker_class(db_file, logger\u003dself.logger) as broker:"},{"line_number":984,"context_line":"                return getattr(self, op)(broker, args)"},{"line_number":985,"context_line":""},{"line_number":986,"context_line":"    @contextmanager"}],"source_content_type":"text/x-python","patch_set":15,"id":"e2763cd9_5643ffe0","line":983,"updated":"2026-05-29 04:55:44.000000000","message":"``ReplicatorRpc`` is used in WSGI servers, e.g. account server REPLICATE function.","commit_id":"ab106e5f65561748c8bd8c0e4a1a3771f5f80aa2"}],"swift/container/backend.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":2504,"context_line":"                        shard_size, last_shard_upper)"},{"line_number":2505,"context_line":"                except (sqlite3.OperationalError, LockTimeout):"},{"line_number":2506,"context_line":"                    self.logger.exception("},{"line_number":2507,"context_line":"                        \"Problem finding shard upper in %r: \" % self.db_file)"},{"line_number":2508,"context_line":"                    break"},{"line_number":2509,"context_line":""},{"line_number":2510,"context_line":"            if (next_shard_upper is None or"}],"source_content_type":"text/x-python","patch_set":5,"id":"d09a3cc9_98f7b2f8","side":"PARENT","line":2507,"updated":"2026-03-30 23:35:11.000000000","message":"it\u0027s a LOT harder to notice this formatting drive-by when the whole wall is RED","commit_id":"020312b8ad30bcb7838198db72ce6166f9833724"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"37bb98ea31db308792c297320dfe03a3cd2938d9","unresolved":false,"context_lines":[{"line_number":2504,"context_line":"                        shard_size, last_shard_upper)"},{"line_number":2505,"context_line":"                except (sqlite3.OperationalError, LockTimeout):"},{"line_number":2506,"context_line":"                    self.logger.exception("},{"line_number":2507,"context_line":"                        \"Problem finding shard upper in %r: \" % self.db_file)"},{"line_number":2508,"context_line":"                    break"},{"line_number":2509,"context_line":""},{"line_number":2510,"context_line":"            if (next_shard_upper is None or"}],"source_content_type":"text/x-python","patch_set":5,"id":"b189360b_3d50bee6","side":"PARENT","line":2507,"in_reply_to":"d09a3cc9_98f7b2f8","updated":"2026-04-23 15:13:11.000000000","message":"Discarded.","commit_id":"020312b8ad30bcb7838198db72ce6166f9833724"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        for broker in self.get_brokers():"},{"line_number":738,"context_line":"            with broker:"},{"line_number":739,"context_line":"                all_empty \u0026\u003d broker._empty()"},{"line_number":740,"context_line":"        if not all_empty:"},{"line_number":741,"context_line":"            return False"},{"line_number":742,"context_line":"        if self.is_root_container() and self.sharding_initiated():"},{"line_number":743,"context_line":"            # sharded shards don\u0027t get updates from their shards so their shard"}],"source_content_type":"text/x-python","patch_set":5,"id":"06babaa3_a5e8eab3","line":740,"updated":"2026-03-30 23:35:11.000000000","message":"this is kind of interesting, I wonder if a `broker.empty()` would just be:\n\n```\nwith self:\n    return self._empty()\n```","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"117b05eae98b5bd885d4c327f931662fce2c1b4e","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        for broker in self.get_brokers():"},{"line_number":738,"context_line":"            with broker:"},{"line_number":739,"context_line":"                all_empty \u0026\u003d broker._empty()"},{"line_number":740,"context_line":"        if not all_empty:"},{"line_number":741,"context_line":"            return False"},{"line_number":742,"context_line":"        if self.is_root_container() and self.sharding_initiated():"},{"line_number":743,"context_line":"            # sharded shards don\u0027t get updates from their shards so their shard"}],"source_content_type":"text/x-python","patch_set":5,"id":"da0e9653_fa8d31ff","line":740,"in_reply_to":"06babaa3_a5e8eab3","updated":"2026-04-09 18:12:54.000000000","message":"Trouble is the non-`self` broker coming out of `self.get_brokers()`.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"07d69bce74dc6f359af0bfcd0d7937cd36a1648a","unresolved":false,"context_lines":[{"line_number":737,"context_line":"        for broker in self.get_brokers():"},{"line_number":738,"context_line":"            with broker:"},{"line_number":739,"context_line":"                all_empty \u0026\u003d broker._empty()"},{"line_number":740,"context_line":"        if not all_empty:"},{"line_number":741,"context_line":"            return False"},{"line_number":742,"context_line":"        if self.is_root_container() and self.sharding_initiated():"},{"line_number":743,"context_line":"            # sharded shards don\u0027t get updates from their shards so their shard"}],"source_content_type":"text/x-python","patch_set":5,"id":"e7e94c1a_26afb641","line":740,"in_reply_to":"46adb1a3_07704bf4","updated":"2026-04-23 15:38:16.000000000","message":"Done","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"37bb98ea31db308792c297320dfe03a3cd2938d9","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        for broker in self.get_brokers():"},{"line_number":738,"context_line":"            with broker:"},{"line_number":739,"context_line":"                all_empty \u0026\u003d broker._empty()"},{"line_number":740,"context_line":"        if not all_empty:"},{"line_number":741,"context_line":"            return False"},{"line_number":742,"context_line":"        if self.is_root_container() and self.sharding_initiated():"},{"line_number":743,"context_line":"            # sharded shards don\u0027t get updates from their shards so their shard"}],"source_content_type":"text/x-python","patch_set":5,"id":"46adb1a3_07704bf4","line":740,"in_reply_to":"8633f327_d2841950","updated":"2026-04-23 15:13:11.000000000","message":"#willfix","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cb3319e1815bf14704eb0b91421712bc678be5a2","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        for broker in self.get_brokers():"},{"line_number":738,"context_line":"            with broker:"},{"line_number":739,"context_line":"                all_empty \u0026\u003d broker._empty()"},{"line_number":740,"context_line":"        if not all_empty:"},{"line_number":741,"context_line":"            return False"},{"line_number":742,"context_line":"        if self.is_root_container() and self.sharding_initiated():"},{"line_number":743,"context_line":"            # sharded shards don\u0027t get updates from their shards so their shard"}],"source_content_type":"text/x-python","patch_set":5,"id":"8633f327_d2841950","line":740,"in_reply_to":"da0e9653_fa8d31ff","updated":"2026-04-23 04:28:37.000000000","message":"in previous code\n```\nif not all(broker._empty() for broker in in self.get_brokers()):\n    return False\n```\nif first broker is not empty, then ``all()`` stops and return ``False``. but the new code will iterate through all the brokers no matter what. I feel this behavior should be kept same.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f420f2e_89c6e7dc","line":949,"updated":"2026-03-30 23:35:11.000000000","message":"so I just picked this diff hunk at random and reverted it:\n\n```\ndiff --git a/swift/container/backend.py b/swift/container/backend.py\nindex 67638368fc..fa556b66d8 100644\n--- a/swift/container/backend.py\n+++ b/swift/container/backend.py\n@@ -945,8 +945,7 @@ class ContainerBroker(DatabaseBroker):\n     def _get_alternate_object_stats(self):\n         db_state \u003d self.get_db_state()\n         if db_state \u003d\u003d SHARDING:\n-            with self.get_brokers()[0] as sub_broker:\n-                other_info \u003d sub_broker._get_info()\n+            other_info \u003d self.get_brokers()[0]._get_info()\n             stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],\n                      \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}\n         elif db_state \u003d\u003d SHARDED and self.is_root_container():\n```\n\nnothing in `swift/test/unit/container` fails\n\nI\u0027m unclear on how we\u0027re deciding what code we can trust to use brokers as context-managers reliably and want to ensure that new pattern is maintained?  IIUC for this diff hunk our unittests would NOT help us maintain this behavior - does that mean we don\u0027t care if this uses a context manager or not?  If we don\u0027t care enough to test it I would say don\u0027t change it (yet?)\n\nupdate: I think we probably DO care, and if we wrote a probe test that tried to do a HEAD request on a shared container and then kill_drive it would find the sub-broker connection is not closed w/o this necessary change.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e0bfacc8baef833dcc76640d4e9ab4617c8c3fe","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"2a99c22e_73b6a31e","line":949,"in_reply_to":"4309edde_f05a3b03","updated":"2026-05-29 12:21:23.000000000","message":"\u003e I\u0027m on the fence here. I get wanting to have tests to validate that the new connection-tracking is actually working appropriately, but I also don\u0027t really see a good place for them.\n\nI\u0027m with Tim on this one. If there is a nice way to actually test this, it could be done in a follow up. Let\u0027s get this one merged to improve probetest stability.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"654f197f_e07dc346","line":949,"in_reply_to":"4309edde_f05a3b03","updated":"2026-05-29 04:55:44.000000000","message":"\u003e\u003e nothing in swift/test/unit/container fails\n\nI did the same revert again, now I am seeing lots of failed unit test cases\n```\nFAILED test/unit/container/test_server.py::TestContainerController::test_GET_auto_record_type - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf67f5d27a540\u003e created at\nFAILED test/unit/container/test_server.py::TestContainerController::test_PUT_GET_shard_ranges - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf67f5d279040\u003e created at\nFAILED test/unit/container/test_server.py::TestContainerController::test_PUT_GET_to_sharding_container - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf67f5d27a440\u003e created at\nFAILED test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_GET_auto_record_type - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf67f5c3c7440\u003e created at\nFAILED test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_PUT_GET_shard_ranges - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf67f5d278c40\u003e created at\nFAILED test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_PUT_GET_to_sharding_container - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0xf67f5d279640\u003e created at\nFAILED test/unit/container/test_sync.py::TestContainerSync::test_init - AssertionError: \u0027/etc/swift/internal-client.conf\u0027 is not an instance of \u003cclass \u0027swift.common.wsgi.ConfigString\u0027\u003e\n```\n\nand this probe test has been changed to a regression test as well: https://review.opendev.org/c/openstack/swift/+/980785/16/test/probe/test_account_failures.py. as a matter of fact, quite a few existing probe tests actually failed with my python 3.12 on master branch.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5fcadaf98e120baa4f22f7811aede418718972e0","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"b4e78026_0c217ff2","line":949,"in_reply_to":"513af299_61b38931","updated":"2026-04-13 19:40:12.000000000","message":"\u003e I wonder if we need a way to test for still open connections.\n\nI mean, that\u0027s basically the existing test plan. I\u0027m not sure I want to commit to fixing all the tests to say `with broker:` everywhere, though -- I looked into updating `BaseUnitTestCase` to use my new `CloseTracker` connection wrapper all over the place, and *mostly* I felt like I needed to go fixing tests, not swift code.\n\nFWIW, that particular hunk *is* covered by tests now; `test_GET_auto_record_type`, `test_PUT_GET_shard_ranges`, and `test_PUT_GET_to_sharding_container` all fail without it.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b58c0a9ff06139f8ee202225de2fd655b4dc3464","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"513af299_61b38931","line":949,"in_reply_to":"55fdbee3_4ea25324","updated":"2026-04-13 03:28:01.000000000","message":"I mean:\n\n```\ndiff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py\nindex 86b63c4c3..24e246a59 100644\n--- a/test/unit/container/test_sharder.py\n+++ b/test/unit/container/test_sharder.py\n@@ -50,7 +50,7 @@ from swift.common.utils import ShardRange, Timestamp, hash_path, \\\n from test.debug_logger import debug_logger, debug_labeled_statsd_client\n from test.unit import FakeRing, make_timestamp_iter, unlink_files, \\\n     mocked_http_conn, mock_timestamp_now, mock_timestamp_now_with_iter, \\\n-    attach_fake_replication_rpc\n+    attach_fake_replication_rpc, check_open_connections\n```\nBecause I added connections and files.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b8925a0faf590ca2b6c4c275ee30f932b6e5c07a","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"55fdbee3_4ea25324","line":949,"in_reply_to":"7f420f2e_89c6e7dc","updated":"2026-04-13 03:26:21.000000000","message":"The idea of this patch is to not leave any tangling connections to sqlite. I wonder if we need a way to test for still open connections.\n\nLooking into pytest fixtures we can do something like:\n\n```\ndiff --git a/test/unit/__init__.py b/test/unit/__init__.py\nindex dd4815f6b..9cb08b04a 100644\n--- a/test/unit/__init__.py\n+++ b/test/unit/__init__.py\n@@ -35,6 +35,8 @@ from shutil import rmtree\n import signal\n import json\n import random\n+import psutil\n+import pytest\n import errno\n import xattr\n from io import BytesIO\n@@ -1647,3 +1649,18 @@ def set_node_errors(proxy_app, ring_node, value, last_error):\n     stats \u003d {\u0027errors\u0027: value,\n              \u0027last_error\u0027: last_error}\n     proxy_app.error_limiter.stats[node_key] \u003d stats\n+\n+\n+@pytest.fixture(autouse\u003dTrue)\n+def check_open_connections(request):\n+    p \u003d psutil.Process()\n+    procs_before \u003d set(p.open_files())\n+    conn_before \u003d set(p.connections())\n+    yield\n+    procs_after \u003d set(p.open_files())\n+    conn_after \u003d set(p.connections())\n+    open_files \u003d procs_after.difference(procs_before)\n+    open_connections \u003d conn_after.difference(conn_before)\n+    if open_files or open_connections:\n+        print(f\"\\n[Open Files/Connections] {request.node.name} open-files: \"\n+              f\"{open_files} open-connections: {open_connections}\")\ndiff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py\nindex 86b63c4c3..24e246a59 100644\n--- a/test/unit/container/test_sharder.py\n+++ b/test/unit/container/test_sharder.py\n@@ -50,7 +50,7 @@ from swift.common.utils import ShardRange, Timestamp, hash_path, \\\n from test.debug_logger import debug_logger, debug_labeled_statsd_client\n from test.unit import FakeRing, make_timestamp_iter, unlink_files, \\\n     mocked_http_conn, mock_timestamp_now, mock_timestamp_now_with_iter, \\\n-    attach_fake_replication_rpc\n+    attach_fake_replication_rpc, check_open_files\n\n```\n\nAnd just need to import it into tests and if pytest can see it, it\u0027ll run the test. In the above example I did it only to test_sharder.py. But need a `-s` to see the results.\n\nIt gave me a:\n\n```\n$ pytest -s test/unit/container/test_sharder.py |grep -i Open\nUnable to read test config /etc/swift/test.conf - file not found\n[Open Files/Connections] test_cleave_insufficient_replication open-files: set() open-connections: {pconn(fd\u003d10, family\u003d\u003cAddressFamily.AF_INET: 2\u003e, type\u003d\u003cSocketKind.SOCK_STREAM: 1\u003e, laddr\u003daddr(ip\u003d\u0027127.0.0.1\u0027, port\u003d36047), raddr\u003daddr(ip\u003d\u0027127.0.0.1\u0027, port\u003d35138), status\u003d\u0027ESTABLISHED\u0027), pconn(fd\u003d9, family\u003d\u003cAddressFamily.AF_INET: 2\u003e, type\u003d\u003cSocketKind.SOCK_STREAM: 1\u003e, laddr\u003daddr(ip\u003d\u0027127.0.0.1\u0027, port\u003d35138), raddr\u003daddr(ip\u003d\u0027127.0.0.1\u0027, port\u003d36047), status\u003d\u0027ESTABLISHED\u0027)}\n```\n\nSo might need some tweaking. But could use it to check for dangling connections in all tests and swift.\n\nin one output a patch behind this I saw an open db connection:\n\n```\n[Open Files/Connections] test_audit_shard_root_ranges_merge_while_sharding open-files: {popenfile(path\u003d\u0027/tmp/tmp_0yhh5_t/sda/containers/0/49c/f982f73d4d88ebef231d2c12bfa7c49c/f982f73d4d88ebef231d2c12bfa7c49c_1776050098.00000.db\u0027, fd\u003d6, position\u003d0, mode\u003d\u0027r+\u0027, flags\u003d688130)} open-connections: set()\n```","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee8102cf7b5f5449e0faf965043efbda62a384b0","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"4309edde_f05a3b03","line":949,"in_reply_to":"a7c1346e_d9a6b57c","updated":"2026-05-27 17:57:32.000000000","message":"I\u0027m on the fence here. I get wanting to have tests to validate that the new connection-tracking is actually working appropriately, but I also don\u0027t really see a good place for them. We don\u0027t have any similar tests for `patch_policies`, or `FakeRing`/`FabricatedRing`, or `FakeMemcache`, or `fake_http_connect`...","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy","status":"I\u0027m a Software Engineer at Red Hat and I love Open Source and connect with people! Feel free to DM through IRC, I\u0027ll be delighted to chat"},"change_message_id":"6b9bc9b4a4562d3f5e147472ba91161da930e7bd","unresolved":true,"context_lines":[{"line_number":946,"context_line":"        db_state \u003d self.get_db_state()"},{"line_number":947,"context_line":"        if db_state \u003d\u003d SHARDING:"},{"line_number":948,"context_line":"            with self.get_brokers()[0] as sub_broker:"},{"line_number":949,"context_line":"                other_info \u003d sub_broker._get_info()"},{"line_number":950,"context_line":"            stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],"},{"line_number":951,"context_line":"                     \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}"},{"line_number":952,"context_line":"        elif db_state \u003d\u003d SHARDED and self.is_root_container():"}],"source_content_type":"text/x-python","patch_set":5,"id":"a7c1346e_d9a6b57c","line":949,"in_reply_to":"b4e78026_0c217ff2","updated":"2026-05-07 13:06:36.000000000","message":"Yeah so Clay\u0027s concern is also mine. We should have a way of checking regressions later and if updating lots of unit tests causes too much trouble, maybe at least a probe test could be added - although I wish we could have a few unit tests testing tests for the context manager contract and if the expected exceptions are being raised.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":2549,"context_line":""},{"line_number":2550,"context_line":"                progress +\u003d shard_size"},{"line_number":2551,"context_line":"                last_shard_upper \u003d next_shard_upper"},{"line_number":2552,"context_line":"                index +\u003d 1"},{"line_number":2553,"context_line":""},{"line_number":2554,"context_line":"            return found_ranges, False"}],"source_content_type":"text/x-python","patch_set":5,"id":"9e8697da_49881e24","line":2552,"updated":"2026-03-30 23:35:11.000000000","message":"I\u0027d prefer we do a method extract instead of the additional indention.\n\ne.g.\n\n```\n@@ -2491,6 +2496,18 @@ class ContainerBroker(DatabaseBroker):\n \n         found_ranges \u003d []\n         sub_broker \u003d self.get_brokers()[0]\n+        with sub_broker:\n+            return self._find_shard_range(progress, progress_reliable,\n+                                          found_ranges, existing_ranges,\n+                                          limit, shard_size,\n+                                          minimum_shard_size, object_count,\n+                                          sub_broker, last_shard_upper,\n+                                          own_shard_range)\n+\n+    def _find_shard_range(self, progress, progress_reliable, found_ranges,\n+                          existing_ranges, limit, shard_size,\n+                          minimum_shard_size, object_count, sub_broker,\n+                          last_shard_upper, own_shard_range):\n         index \u003d len(existing_ranges)\n         while limit is None or limit \u003c 0 or len(found_ranges) \u003c limit:\n             if progress + shard_size + minimum_shard_size \u003e object_count:\n@@ -2504,7 +2521,7 @@ class ContainerBroker(DatabaseBroker):\n                         shard_size, last_shard_upper)\n                 except (sqlite3.OperationalError, LockTimeout):\n                     self.logger.exception(\n-                        \"Problem finding shard upper in %r: \" % self.db_file)\n+                        \"Problem finding shard upper in %r: \", self.db_file)\n                     break\n \n             if (next_shard_upper is None or\n```\n\nN.B. it might be possible to cleanup the order of the `_find_shard_range` args and add a helpful doc string.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"37bb98ea31db308792c297320dfe03a3cd2938d9","unresolved":false,"context_lines":[{"line_number":2549,"context_line":""},{"line_number":2550,"context_line":"                progress +\u003d shard_size"},{"line_number":2551,"context_line":"                last_shard_upper \u003d next_shard_upper"},{"line_number":2552,"context_line":"                index +\u003d 1"},{"line_number":2553,"context_line":""},{"line_number":2554,"context_line":"            return found_ranges, False"}],"source_content_type":"text/x-python","patch_set":5,"id":"d95ec46b_da1fda2c","line":2552,"in_reply_to":"9e8697da_49881e24","updated":"2026-04-23 15:13:11.000000000","message":"Done","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9c9a293d69b39a04e5686789a6e7a8dd3c428187","unresolved":true,"context_lines":[{"line_number":2495,"context_line":"            last_shard_upper \u003d own_shard_range.lower"},{"line_number":2496,"context_line":""},{"line_number":2497,"context_line":"        found_ranges \u003d []"},{"line_number":2498,"context_line":"        with self.get_brokers()[0] as sub_broker:"},{"line_number":2499,"context_line":"            return self._find_shard_ranges(progress, progress_reliable,"},{"line_number":2500,"context_line":"                                           found_ranges, existing_ranges,"},{"line_number":2501,"context_line":"                                           limit, shard_size,"}],"source_content_type":"text/x-python","patch_set":10,"id":"fdf94d1a_f7e65241","line":2498,"updated":"2026-04-10 22:07:02.000000000","message":"Technically not needed to fix up the WSGI servers; this is only used in sharder/manage-shard-ranges workflows. But once I saw the `.get_brokers()` pattern, I wanted to fix all the occurrences in `backend.py`.","commit_id":"85cbe43bf6d900c4a8fa2b49ea7495d6847b1888"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":false,"context_lines":[{"line_number":2495,"context_line":"            last_shard_upper \u003d own_shard_range.lower"},{"line_number":2496,"context_line":""},{"line_number":2497,"context_line":"        found_ranges \u003d []"},{"line_number":2498,"context_line":"        with self.get_brokers()[0] as sub_broker:"},{"line_number":2499,"context_line":"            return self._find_shard_ranges(progress, progress_reliable,"},{"line_number":2500,"context_line":"                                           found_ranges, existing_ranges,"},{"line_number":2501,"context_line":"                                           limit, shard_size,"}],"source_content_type":"text/x-python","patch_set":10,"id":"24dd1283_a0e0321a","line":2498,"in_reply_to":"fdf94d1a_f7e65241","updated":"2026-05-29 04:55:44.000000000","message":"Acknowledged","commit_id":"85cbe43bf6d900c4a8fa2b49ea7495d6847b1888"}],"swift/container/server.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cb3319e1815bf14704eb0b91421712bc678be5a2","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        # auto create accounts)"},{"line_number":365,"context_line":"        obj_policy_index \u003d self.get_and_validate_policy_index(req) or 0"},{"line_number":366,"context_line":"        with self._get_container_broker("},{"line_number":367,"context_line":"                drive, part, account, container) as broker:"},{"line_number":368,"context_line":"            if obj:"},{"line_number":369,"context_line":"                self._maybe_autocreate(broker, req_timestamp, account,"},{"line_number":370,"context_line":"                                       obj_policy_index, req)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5857aad1_aeb7865c","line":367,"updated":"2026-04-23 04:28:37.000000000","message":"will be great if there are two internal helper function\n```\n        with self._get_container_broker(\n                drive, part, account, container) as broker:\n            if obj:\n                self._delete_obj(....)\n            else:\n                self._delete_container(....)\n```","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"37bb98ea31db308792c297320dfe03a3cd2938d9","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        # auto create accounts)"},{"line_number":365,"context_line":"        obj_policy_index \u003d self.get_and_validate_policy_index(req) or 0"},{"line_number":366,"context_line":"        with self._get_container_broker("},{"line_number":367,"context_line":"                drive, part, account, container) as broker:"},{"line_number":368,"context_line":"            if obj:"},{"line_number":369,"context_line":"                self._maybe_autocreate(broker, req_timestamp, account,"},{"line_number":370,"context_line":"                                       obj_policy_index, req)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5b56fada_c118c3ee","line":367,"in_reply_to":"5857aad1_aeb7865c","updated":"2026-04-23 15:13:11.000000000","message":"I\u0027ll get a pre-factor together.","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"07d69bce74dc6f359af0bfcd0d7937cd36a1648a","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        # auto create accounts)"},{"line_number":365,"context_line":"        obj_policy_index \u003d self.get_and_validate_policy_index(req) or 0"},{"line_number":366,"context_line":"        with self._get_container_broker("},{"line_number":367,"context_line":"                drive, part, account, container) as broker:"},{"line_number":368,"context_line":"            if obj:"},{"line_number":369,"context_line":"                self._maybe_autocreate(broker, req_timestamp, account,"},{"line_number":370,"context_line":"                                       obj_policy_index, req)"}],"source_content_type":"text/x-python","patch_set":13,"id":"635aa00c_4aa9bce3","line":367,"in_reply_to":"5b56fada_c118c3ee","updated":"2026-04-23 15:38:16.000000000","message":"Done","commit_id":"292f09cbb84ac1361fe1d56286daa7cab04a8de0"}],"test/probe/test_account_failures.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        # Assert account level knows container1 is gone but doesn\u0027t know about"},{"line_number":110,"context_line":"        #   container2/object2 yet"},{"line_number":111,"context_line":"        headers, containers \u003d client.get_account(self.url, self.token)"},{"line_number":112,"context_line":"        self.assertEqual(headers[\u0027x-account-container-count\u0027], \u00271\u0027)"},{"line_number":113,"context_line":"        self.assertEqual(headers[\u0027x-account-object-count\u0027], \u00271\u0027)"},{"line_number":114,"context_line":"        self.assertEqual(headers[\u0027x-account-bytes-used\u0027], \u00274\u0027)"},{"line_number":115,"context_line":"        found1 \u003d False"}],"source_content_type":"text/x-python","patch_set":16,"id":"b7cd6f24_677f612a","line":112,"updated":"2026-05-29 04:55:44.000000000","message":"with master branch, I am seeing this failure because of unclosed db connection\n\n```\n        headers, containers \u003d client.get_account(self.url, self.token)\n\u003e       self.assertEqual(headers[\u0027x-account-container-count\u0027], \u00271\u0027)\nE       AssertionError: \u00272\u0027 !\u003d \u00271\u0027\nE       - 2\nE       + 1\n\ntest/probe/test_account_failures.py:112: AssertionError\n```","commit_id":"05c11f44beef85a357ed5a2d98097da79a59dcdf"}],"test/unit/__init__.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":1662,"context_line":"            if not any("},{"line_number":1663,"context_line":"                x in fs.filename"},{"line_number":1664,"context_line":"                for x in (\u0027unittest\u0027, \u0027pytest\u0027, \u0027pluggy\u0027, \u0027test/unit/\u0027)"},{"line_number":1665,"context_line":"            )"},{"line_number":1666,"context_line":"        ]"},{"line_number":1667,"context_line":"        CloseTracker.open_conns.append(self)"},{"line_number":1668,"context_line":"        CloseTracker.ever_called \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"3697266a_7b44a6b4","line":1665,"updated":"2026-03-30 23:35:11.000000000","message":"what\u0027s `pluggy`\n\ncould this filter lead to \"gaps\" as opposed to just triming the fat on the outside?","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"117b05eae98b5bd885d4c327f931662fce2c1b4e","unresolved":true,"context_lines":[{"line_number":1662,"context_line":"            if not any("},{"line_number":1663,"context_line":"                x in fs.filename"},{"line_number":1664,"context_line":"                for x in (\u0027unittest\u0027, \u0027pytest\u0027, \u0027pluggy\u0027, \u0027test/unit/\u0027)"},{"line_number":1665,"context_line":"            )"},{"line_number":1666,"context_line":"        ]"},{"line_number":1667,"context_line":"        CloseTracker.open_conns.append(self)"},{"line_number":1668,"context_line":"        CloseTracker.ever_called \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"a217b70f_60468eca","line":1665,"in_reply_to":"3697266a_7b44a6b4","updated":"2026-04-09 18:12:54.000000000","message":"It\u0027s how pytest manages its plugins. It could *absolutely* lead to weird gaps -- say, where we\u0027ve patched out (some part of?) a broker for a test; there may be some frames of `test/unit/` code that are relevant.\n\nBut this is more about leaving breadcrumbs pointing us toward how the broker got instantiated than tracking *exactly* what happened -- the long and short of it is that I found full stacks to be too much chaff for the wheat. If we find it\u0027s pruned too much, can always edit the filter in a local dev environment.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":true,"context_lines":[{"line_number":1662,"context_line":"            if not any("},{"line_number":1663,"context_line":"                x in fs.filename"},{"line_number":1664,"context_line":"                for x in (\u0027unittest\u0027, \u0027pytest\u0027, \u0027pluggy\u0027, \u0027test/unit/\u0027)"},{"line_number":1665,"context_line":"            )"},{"line_number":1666,"context_line":"        ]"},{"line_number":1667,"context_line":"        CloseTracker.open_conns.append(self)"},{"line_number":1668,"context_line":"        CloseTracker.ever_called \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"d9c330a6_bc31ab5d","line":1665,"in_reply_to":"a217b70f_60468eca","updated":"2026-05-29 04:55:44.000000000","message":"```\npluggy is pytest\u0027s plugin/hook dispatcher (_pytest calls into pluggy._hookexec which then\n calls back into user plugin code). Frames from pluggy/, _pytest/, and stdlib unittest/\n normally sit at the bottom of the stack (test runner → fixture → test body)\n```\n\nbut, what\u0027s ``test/unit/asdf``?","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1e41e18ba5e81c5dd5d2975b639a6553095f4d85","unresolved":true,"context_lines":[{"line_number":1662,"context_line":"            if not any("},{"line_number":1663,"context_line":"                x in fs.filename"},{"line_number":1664,"context_line":"                for x in (\u0027unittest\u0027, \u0027pytest\u0027, \u0027pluggy\u0027, \u0027test/unit/\u0027)"},{"line_number":1665,"context_line":"            )"},{"line_number":1666,"context_line":"        ]"},{"line_number":1667,"context_line":"        CloseTracker.open_conns.append(self)"},{"line_number":1668,"context_line":"        CloseTracker.ever_called \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a376912_2ea18fe2","line":1665,"in_reply_to":"d9c330a6_bc31ab5d","updated":"2026-05-29 23:25:53.000000000","message":"Good catch! I forgot I\u0027d done that -- I was debating about just dropping `test/unit`, and probably should.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":1699,"context_line":"@contextmanager"},{"line_number":1700,"context_line":"def check_db_connections_get_closed():"},{"line_number":1701,"context_line":"    with mocklib.patch(\u0027swift.common.db.GreenDBConnection\u0027, CloseTracker):"},{"line_number":1702,"context_line":"        yield"},{"line_number":1703,"context_line":"    CloseTracker.assert_called_and_all_closed()"}],"source_content_type":"text/x-python","patch_set":5,"id":"902b4762_f66c8a31","line":1702,"updated":"2026-03-30 23:35:11.000000000","message":"I wonder if we could adapt this patch for use with patcher.start and addCleanup so that we could bake it into `BaseDBCloseTrackingTestCase.setup()`","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"117b05eae98b5bd885d4c327f931662fce2c1b4e","unresolved":true,"context_lines":[{"line_number":1699,"context_line":"@contextmanager"},{"line_number":1700,"context_line":"def check_db_connections_get_closed():"},{"line_number":1701,"context_line":"    with mocklib.patch(\u0027swift.common.db.GreenDBConnection\u0027, CloseTracker):"},{"line_number":1702,"context_line":"        yield"},{"line_number":1703,"context_line":"    CloseTracker.assert_called_and_all_closed()"}],"source_content_type":"text/x-python","patch_set":5,"id":"c761b56b_6638bf42","line":1702,"in_reply_to":"902b4762_f66c8a31","updated":"2026-04-09 18:12:54.000000000","message":"Or throw it on `BaseUnitTestCase` and commit to fixing it everywhere!\n\nThough that\u0027d\n\n1. be *way* less surgical and\n2. commit us to fixing our usage *in tests*, which is going to be a lot more churn for no practical benefit (beyond, I suppose, ensuring that tests demonstrate the *proper* way to use a broker).","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":1700,"context_line":"def check_db_connections_get_closed():"},{"line_number":1701,"context_line":"    with mocklib.patch(\u0027swift.common.db.GreenDBConnection\u0027, CloseTracker):"},{"line_number":1702,"context_line":"        yield"},{"line_number":1703,"context_line":"    CloseTracker.assert_called_and_all_closed()"}],"source_content_type":"text/x-python","patch_set":5,"id":"6d66e77c_c1716542","line":1703,"updated":"2026-03-30 23:35:11.000000000","message":"this seems more \"opt-in\" in the tests - as opposed to \"just a property of the broker when used as a context manager\"\n\nWhat\u0027s the downside of saying \"once code opts into using a broker as a context manager it must not get connections that won\u0027t automatically close\"\n\nWhat\u0027s better about saying \"for the limited portion of the tested unit under the new opt-in test helper context; any opened connection was closed\"\n\n... like do we NEED to support test code that wants by-pass the native `DatabaseBroker.__exit__` to use explicit `finally: broker.close()`?","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"117b05eae98b5bd885d4c327f931662fce2c1b4e","unresolved":true,"context_lines":[{"line_number":1700,"context_line":"def check_db_connections_get_closed():"},{"line_number":1701,"context_line":"    with mocklib.patch(\u0027swift.common.db.GreenDBConnection\u0027, CloseTracker):"},{"line_number":1702,"context_line":"        yield"},{"line_number":1703,"context_line":"    CloseTracker.assert_called_and_all_closed()"}],"source_content_type":"text/x-python","patch_set":5,"id":"f00e29cd_c3f08b63","line":1703,"in_reply_to":"6d66e77c_c1716542","updated":"2026-04-09 18:12:54.000000000","message":"\u003e this seems more \"opt-in\" in the tests\n\nI\u0027ll look at wrapping `self.controller` to check every request. Strikes me as better scoped than trying to do something in `setUp`.\n\n\u003e What\u0027s the downside of saying \"once code opts into using a broker as a context manager it must not get connections that won\u0027t automatically close\"\n\nShort of a probe test, how do you demonstrate that the server actually is using the context manager? We\u0027ll not have demonstrated that anything actually gets closed.\n\n\u003e What\u0027s better about saying \"for the limited portion of the tested unit under the new opt-in test helper context; any opened connection was closed\"\n\nThis is the behavior we actually *need*, isn\u0027t it? That all connections created during this request get closed?","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee8102cf7b5f5449e0faf965043efbda62a384b0","unresolved":false,"context_lines":[{"line_number":1700,"context_line":"def check_db_connections_get_closed():"},{"line_number":1701,"context_line":"    with mocklib.patch(\u0027swift.common.db.GreenDBConnection\u0027, CloseTracker):"},{"line_number":1702,"context_line":"        yield"},{"line_number":1703,"context_line":"    CloseTracker.assert_called_and_all_closed()"}],"source_content_type":"text/x-python","patch_set":5,"id":"62d2c9b5_bad88582","line":1703,"in_reply_to":"f00e29cd_c3f08b63","updated":"2026-05-27 17:57:32.000000000","message":"\u003e I\u0027ll look at wrapping `self.controller` to check every request.\n\nDone.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"5ddd5d5828e8638f0b96b599c8ed4799a6d90285","unresolved":false,"context_lines":[{"line_number":1718,"context_line":"    def __setattr__(self, name, value):"},{"line_number":1719,"context_line":"        if name in (\u0027app\u0027):"},{"line_number":1720,"context_line":"            return object.__setattr__(self, name, value)"},{"line_number":1721,"context_line":"        return setattr(self.app, name, value)"}],"source_content_type":"text/x-python","patch_set":12,"id":"e684decb_3ad7f34f","line":1721,"updated":"2026-04-14 07:25:30.000000000","message":"Cool idea!","commit_id":"4d7825132682549e479d80cf7d1896ba46a72a09"}],"test/unit/account/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":944,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027POST\u0027,"},{"line_number":945,"context_line":"                                                  \u0027HTTP_X_TIMESTAMP\u0027: \u00270\u0027},"},{"line_number":946,"context_line":"                            headers\u003d{\u0027X-Timestamp\u0027: \u0027not-float\u0027})"},{"line_number":947,"context_line":"        resp \u003d req.get_response(self.controller)"},{"line_number":948,"context_line":"        self.assertEqual(resp.status_int, 400)"},{"line_number":949,"context_line":""},{"line_number":950,"context_line":"    def test_POST_after_DELETE_not_found(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"988d571f_d0ad66f2","line":947,"updated":"2026-03-30 23:35:11.000000000","message":"I\u0027m confused, is it ok if this request leaves connections option?  What is the correct amount of tests to decorate with the new helper - only the ones where we want to maintain the behavior of \"always use broker as a context manager\"???","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"37bb98ea31db308792c297320dfe03a3cd2938d9","unresolved":false,"context_lines":[{"line_number":944,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027POST\u0027,"},{"line_number":945,"context_line":"                                                  \u0027HTTP_X_TIMESTAMP\u0027: \u00270\u0027},"},{"line_number":946,"context_line":"                            headers\u003d{\u0027X-Timestamp\u0027: \u0027not-float\u0027})"},{"line_number":947,"context_line":"        resp \u003d req.get_response(self.controller)"},{"line_number":948,"context_line":"        self.assertEqual(resp.status_int, 400)"},{"line_number":949,"context_line":""},{"line_number":950,"context_line":"    def test_POST_after_DELETE_not_found(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ce2af17f_ecbcfb74","line":947,"in_reply_to":"988d571f_d0ad66f2","updated":"2026-04-23 15:13:11.000000000","message":"`self.controller` now wraps the real controller to always enforce connections get closed.","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ae1c0aa0d64e7744946f8d03dc45520f9cce5e3f","unresolved":true,"context_lines":[{"line_number":135,"context_line":"        self.assertEqual(resp.status_int, 404)"},{"line_number":136,"context_line":"        self.assertNotIn(\u0027X-Account-Status\u0027, resp.headers)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_DELETE_empty(self):"},{"line_number":139,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027,"},{"line_number":140,"context_line":"                                                  \u0027HTTP_X_TIMESTAMP\u0027: \u00270\u0027})"},{"line_number":141,"context_line":"        resp \u003d req.get_response(self.controller)"}],"source_content_type":"text/x-python","patch_set":16,"id":"6fa29582_9ebae9b5","line":138,"updated":"2026-05-29 04:55:44.000000000","message":"I ran this test case on master, with only changes in this ``test_server.py`` and also ``test/unit/__init__.py``. I was hoping that I could see traceback from ``assert open_conns \u003d\u003d [], open_conns``, but instead I saw this traceback\n\n```\n______________________________________ TestAccountController.test_DELETE_empty ______________________________________\n\nself \u003d \u003ctest.unit.account.test_server.TestAccountController testMethod\u003dtest_DELETE_empty\u003e\n\n    def test_DELETE_empty(self):\n        req \u003d Request.blank(\u0027/sda1/p/a\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027,\n                                                  \u0027HTTP_X_TIMESTAMP\u0027: \u00270\u0027})\n        resp \u003d req.get_response(self.controller)\n\u003e       self.assertEqual(resp.status_int, 201)\nE       AssertionError: 500 !\u003d 201\n\ntest/unit/account/test_server.py:142: AssertionError\n----------------------------------------------- Captured stdout call ------------------------------------------------\ntest ERROR: ERROR __call__ error with PUT /sda1/p/a :\nTraceback (most recent call last):\n  File \"/vagrant/swift/swift/account/server.py\", line 316, in __call__\n    res \u003d getattr(self, req.method)(req)\n          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/vagrant/swift/swift/common/base_storage_server.py\", line 71, in _timing_stats\n    resp \u003d func(ctrl, *args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/vagrant/swift/swift/account/server.py\", line 198, in PUT\n    broker.initialize(timestamp.internal)\n  File \"/vagrant/swift/swift/common/db.py\", line 367, in initialize\n    conn \u003d sqlite3.connect(tmp_db_file, check_same_thread\u003dFalse,\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/vagrant/swift/test/unit/__init__.py\", line 1691, in __init__\n    self.conn \u003d GreenDBConnection(*a, **kw)\n                ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/vagrant/swift/swift/common/db.py\", line 126, in __init__\n    super(GreenDBConnection, self).__init__(\n    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\nTypeError: super(type, obj): obj must be an instance or subtype of type\ntest INFO: - - - [29/May/2026:04:41:53 +0000] \"PUT /sda1/p/a\" 500 1044 \"-\" \"-\" \"-\" 0.0026 \"-\" 53495 -\n```","commit_id":"05c11f44beef85a357ed5a2d98097da79a59dcdf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1e41e18ba5e81c5dd5d2975b639a6553095f4d85","unresolved":true,"context_lines":[{"line_number":135,"context_line":"        self.assertEqual(resp.status_int, 404)"},{"line_number":136,"context_line":"        self.assertNotIn(\u0027X-Account-Status\u0027, resp.headers)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_DELETE_empty(self):"},{"line_number":139,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027,"},{"line_number":140,"context_line":"                                                  \u0027HTTP_X_TIMESTAMP\u0027: \u00270\u0027})"},{"line_number":141,"context_line":"        resp \u003d req.get_response(self.controller)"}],"source_content_type":"text/x-python","patch_set":16,"id":"c7d13f26_1207563c","line":138,"in_reply_to":"6fa29582_9ebae9b5","updated":"2026-05-29 23:25:53.000000000","message":"You need part of that diff against `db.py`; in particular, it should be `super().__init__(database, timeout\u003d0, *args, **kwargs)` rather than `super(GreenDBConnection, self).__init__(...)`\n\nMaybe I coulda done something different to have `CloseTracker` inherit from (rather than wrap) `GreenDBConnection`? But then I think I was bumping into `RecursionError`s. \\*shrug\\*","commit_id":"05c11f44beef85a357ed5a2d98097da79a59dcdf"}],"test/unit/container/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0e0e2b1ecf095d8414739518bf561be0ece16d95","unresolved":true,"context_lines":[{"line_number":4053,"context_line":""},{"line_number":4054,"context_line":"        def assert_GET_shard_ranges(req, expected_shard_ranges):"},{"line_number":4055,"context_line":"            with mock_timestamp_now(ts_now):"},{"line_number":4056,"context_line":"                resp \u003d req.get_response(self.controller)"},{"line_number":4057,"context_line":"            self.assertEqual(resp.status_int, 200)"},{"line_number":4058,"context_line":"            self.assertEqual(resp.content_type, \u0027application/json\u0027)"},{"line_number":4059,"context_line":"            expected \u003d ["}],"source_content_type":"text/x-python","patch_set":5,"id":"3a52fb43_994770a0","line":4056,"updated":"2026-03-30 23:35:11.000000000","message":"FWIW it would be trivial to catch that sub-broker change:\n\n```\ndiff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py\nindex 7cc555feed..07ca67fe9f 100644\n--- a/test/unit/container/test_server.py\n+++ b/test/unit/container/test_server.py\n@@ -4052,7 +4052,7 @@ class TestContainerController(BaseUnitTestCase):\n             return resp\n \n         def assert_GET_shard_ranges(req, expected_shard_ranges):\n-            with mock_timestamp_now(ts_now):\n+            with mock_timestamp_now(ts_now), check_db_connections_get_closed():\n                 resp \u003d req.get_response(self.controller)\n             self.assertEqual(resp.status_int, 200)\n             self.assertEqual(resp.content_type, \u0027application/json\u0027)\n```\n\nwith the sub_broker as context manager \"fix\" reverted and this existing change sort of arbitrially selected to double as a \"check db connection context manager\" diff in place I get a test failure like this:\n\n```\nFAILED swift/test/unit/container/test_server.py::TestContainerController::test_GET_auto_record_type - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x70944614f240\u003e created at\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 1154, in get_response\n    status, headers, app_iter \u003d self.call_application(application)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 1138, in call_application\n    app_iter \u003d application(self.environ, start_response)\n  File \"/home/vagrant/swift/swift/container/server.py\", line 1032, in __call__\n    res \u003d getattr(self, req.method)(req)\n  File \"/home/vagrant/swift/swift/common/base_storage_server.py\", line 71, in _timing_stats\n    resp \u003d func(ctrl, *args, **kwargs)\n  File \"/home/vagrant/swift/swift/container/server.py\", line 767, in GET\n    info, is_deleted \u003d broker.get_info_is_deleted()\n  File \"/home/vagrant/swift/swift/container/backend.py\", line 890, in get_info_is_deleted\n    info \u003d self.get_info()\n  File \"/home/vagrant/swift/swift/container/backend.py\", line 970, in get_info\n    state, stats \u003d self._get_alternate_object_stats()\n  File \"/home/vagrant/swift/swift/container/backend.py\", line 948, in _get_alternate_object_stats\n    other_info \u003d self.get_brokers()[0]._get_info()\n  File \"/home/vagrant/swift/swift/container/backend.py\", line 936, in _get_info\n    with self.get() as conn:\n  File \"/usr/lib/python3.12/contextlib.py\", line 137, in __enter__\n    return next(self.gen)\n  File \"/home/vagrant/swift/swift/common/db.py\", line 550, in get\n    self.conn \u003d get_db_connection(self.db_file, self.timeout,\n  File \"/home/vagrant/swift/swift/common/db.py\", line 204, in get_db_connection\n    conn \u003d sqlite3.connect(path, check_same_thread\u003dFalse,\n\u003e]\n```\n\n... which DOES point you at:\n\n```\nE             File \"/home/vagrant/swift/swift/container/backend.py\", line 948, in _get_alternate_object_stats\nE               other_info \u003d self.get_brokers()[0]._get_info()\n```\n\n... albiet some frames up","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee8102cf7b5f5449e0faf965043efbda62a384b0","unresolved":false,"context_lines":[{"line_number":4053,"context_line":""},{"line_number":4054,"context_line":"        def assert_GET_shard_ranges(req, expected_shard_ranges):"},{"line_number":4055,"context_line":"            with mock_timestamp_now(ts_now):"},{"line_number":4056,"context_line":"                resp \u003d req.get_response(self.controller)"},{"line_number":4057,"context_line":"            self.assertEqual(resp.status_int, 200)"},{"line_number":4058,"context_line":"            self.assertEqual(resp.content_type, \u0027application/json\u0027)"},{"line_number":4059,"context_line":"            expected \u003d ["}],"source_content_type":"text/x-python","patch_set":5,"id":"9899e678_b9c8f061","line":4056,"in_reply_to":"3a52fb43_994770a0","updated":"2026-05-27 17:57:32.000000000","message":"Applying\n```\ndiff --git a/swift/container/backend.py b/swift/container/backend.py\nindex 6f42846fe..eddb8c643 100644\n--- a/swift/container/backend.py\n+++ b/swift/container/backend.py\n@@ -946,8 +946,7 @@ class ContainerBroker(DatabaseBroker):\n     def _get_alternate_object_stats(self):\n         db_state \u003d self.get_db_state()\n         if db_state \u003d\u003d SHARDING:\n-            with self.get_brokers()[0] as sub_broker:\n-                other_info \u003d sub_broker._get_info()\n+            other_info \u003d self.get_brokers()[0]._get_info()\n             stats \u003d {\u0027object_count\u0027: other_info[\u0027object_count\u0027],\n                      \u0027bytes_used\u0027: other_info[\u0027bytes_used\u0027]}\n         elif db_state \u003d\u003d SHARDED and self.is_root_container():\n```\ncurrently causes the following tests to fail:\n```\nFAILED test/unit/container/test_server.py::TestContainerController::test_GET_auto_record_type - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x10a4d8e40\u003e created at\nFAILED test/unit/container/test_server.py::TestContainerController::test_PUT_GET_shard_ranges - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x10b184d40\u003e created at\nFAILED test/unit/container/test_server.py::TestContainerController::test_PUT_GET_to_sharding_container - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x10b186840\u003e created at\nFAILED test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_GET_auto_record_type - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x10b209140\u003e created at\nFAILED test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_PUT_GET_shard_ranges - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x10a345640\u003e created at\nFAILED test/unit/container/test_server.py::TestNonLegacyDefaultStoragePolicy::test_PUT_GET_to_sharding_container - AssertionError: [\u003cCloseTracker: \u003cswift.common.db.GreenDBConnection object at 0x10a346640\u003e created at\n```","commit_id":"6871e28ed6807838605f26a21583ba0e0466dd9d"}]}