)]}'
{"/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":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"}],"/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"}],"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"}],"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":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"}],"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":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":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"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"}],"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/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":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":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"}],"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"}]}