)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c6fedf9d_413674f9","updated":"2026-09-11 14:37:18.000000000","message":"I removed sq? from commit title since parent patch is merging.\n\nthe *general* rule for exception handling is \"as tight as possible\"\n\nthe *exception* to the general rule about exceptions is for `except Exception` - I call them \"pokemon handlers\" (because you gotta catch \u0027em all)\n\nFor pokemon handlers you want to them to be \"around the whole block you want to save\" - in this case not *just* the \"quarantine_dir_reanmer\" but the whole inner loop:\n\n```\nfor hash in hashes:\n    try:\n        cleanup_hash_and_maybe_suffix()\n    except:\n        # the loop will continue!!!\n        ...\n```\n\n^ I think this change more or less does that but it could be more obvious with a refactor\n\nI struggled with the test and think the subtest/different-exception-type-assert made it harder than it had to be, and didn\u0027t add much over `Exception(\u0027kaboom!\u0027)`","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f3dcbd42_ffffdaee","in_reply_to":"c6fedf9d_413674f9","updated":"2026-09-15 02:33:28.000000000","message":"This is great. Thanks for the guidance.","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"11566454_39c824d9","updated":"2026-09-15 02:33:28.000000000","message":"Thanks @clay.gerrard@gmail.com for the review. I reworked the test code and tried to absorb as many of your suggestions as possible. I also realized (after reading your top-level comment) thatwe can/should tighten the exceptions being caught, so I did that too.","commit_id":"4932bf1e50357fd7d5418b4244a10c88ea7dda4f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d468a396_8903847b","updated":"2026-09-15 22:33:52.000000000","message":"I think this is correct and useful - that\u0027s good enough for me!\n\n... but also I think it\u0027s not the best way to fix the bug b/c it leaves some obvious stuff on the table:\n\n1005811: replicator: defer robust suffix cleanup | https://review.opendev.org/c/openstack/swift/+/1005811\n\n... and the new test infra could do less and support more DAMP","commit_id":"bd664103928dde998f7b79ec73c572153c8b1b52"}],"swift/obj/replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":599,"context_line":"                             job[\u0027policy\u0027])"},{"line_number":600,"context_line":"            self.logger.increment(\u0027partition.lock-failure.count\u0027)"},{"line_number":601,"context_line":"        except (Exception, Timeout):"},{"line_number":602,"context_line":"            self.logger.exception(\"Error syncing handoff partition\")"},{"line_number":603,"context_line":"        finally:"},{"line_number":604,"context_line":"            stats.add_failure_stats(failure_devs_info)"},{"line_number":605,"context_line":"            target_devs_info \u003d set([(target_dev[\u0027replication_ip\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"515235e1_fb2b34b3","line":602,"updated":"2026-09-11 14:37:18.000000000","message":"the quarantine exception *would* be caught here - i\u0027m not sure we\u0027re missing much","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":false,"context_lines":[{"line_number":599,"context_line":"                             job[\u0027policy\u0027])"},{"line_number":600,"context_line":"            self.logger.increment(\u0027partition.lock-failure.count\u0027)"},{"line_number":601,"context_line":"        except (Exception, Timeout):"},{"line_number":602,"context_line":"            self.logger.exception(\"Error syncing handoff partition\")"},{"line_number":603,"context_line":"        finally:"},{"line_number":604,"context_line":"            stats.add_failure_stats(failure_devs_info)"},{"line_number":605,"context_line":"            target_devs_info \u003d set([(target_dev[\u0027replication_ip\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"3dbb890b_82cac03d","line":602,"in_reply_to":"515235e1_fb2b34b3","updated":"2026-09-15 02:33:28.000000000","message":"Acknowledged","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":623,"context_line":"                raise"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"    def delete_handoff_objs(self, job, delete_objs):"},{"line_number":626,"context_line":"        for object_hash in delete_objs:"},{"line_number":627,"context_line":"            object_path \u003d storage_directory(job[\u0027obj_path\u0027], job[\u0027partition\u0027],"},{"line_number":628,"context_line":"                                            object_hash)"},{"line_number":629,"context_line":"            tpool.execute(shutil.rmtree, object_path, ignore_errors\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"160d7147_e362971d","line":626,"updated":"2026-09-11 14:37:18.000000000","message":"oic; you\u0027re trying to continue *this* loop\n\nthat\u0027s probably helpful\n\nmaybe worth a method extract:\n\n```\nfor hash in hash:\n    remove_hash\n    try_remove_empty_suffix_quarantine_when_needed_and_do_not_die()\n```","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":true,"context_lines":[{"line_number":637,"context_line":"                    try:"},{"line_number":638,"context_line":"                        quarantine_dir_renamer(dirname(job[\u0027obj_path\u0027]),"},{"line_number":639,"context_line":"                                               suffix_dir)"},{"line_number":640,"context_line":"                    except (OSError, LockTimeout) as e:"},{"line_number":641,"context_line":"                        self.logger.error(\"Failed to quarantine %r (%s)\","},{"line_number":642,"context_line":"                                          suffix_dir, e)"},{"line_number":643,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"e57999b2_d1902755","line":640,"updated":"2026-09-15 22:33:52.000000000","message":"ok, so the idea is that any fs error is likely going to be `OSError`\n\nI guess we used to have to deal with `IOError` but they fixed it py3.3+\n\nhttps://peps.python.org/pep-3151/\n\nso this isn\u0027t trying to guarantee \"if there is any problem removing (and then maybe quarantining) the suffix we will continue onto the next object_hash (and subsequent suffix)\" ... it\u0027s a narrower guarantee:\n\n*just* for these *specific* errors that show up in the new exception handling branch we will continue\n\n... seems slightly less interesting","commit_id":"bd664103928dde998f7b79ec73c572153c8b1b52"}],"test/unit/obj/test_replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1812,"context_line":"        diskfiles \u003d []"},{"line_number":1813,"context_line":"        paths \u003d []"},{"line_number":1814,"context_line":"        ordered_hashes \u003d []"},{"line_number":1815,"context_line":"        candidates \u003d {}"},{"line_number":1816,"context_line":"        for obj in (\u0027o1\u0027, \u0027o2\u0027):"},{"line_number":1817,"context_line":"            df \u003d self.df_mgr.get_diskfile(\u0027sda\u0027, \u00271\u0027, \u0027a\u0027, \u0027c\u0027, obj,"},{"line_number":1818,"context_line":"                                          policy\u003dPOLICIES.legacy)"}],"source_content_type":"text/x-python","patch_set":1,"id":"664c6b48_e15a4421","line":1815,"updated":"2026-09-11 14:37:18.000000000","message":"i think if you used an ordered dict here to control the delete_objs order you wouldn\u0027t have to mock out the unit-under-test inorder to ensure consistent order","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":true,"context_lines":[{"line_number":1812,"context_line":"        diskfiles \u003d []"},{"line_number":1813,"context_line":"        paths \u003d []"},{"line_number":1814,"context_line":"        ordered_hashes \u003d []"},{"line_number":1815,"context_line":"        candidates \u003d {}"},{"line_number":1816,"context_line":"        for obj in (\u0027o1\u0027, \u0027o2\u0027):"},{"line_number":1817,"context_line":"            df \u003d self.df_mgr.get_diskfile(\u0027sda\u0027, \u00271\u0027, \u0027a\u0027, \u0027c\u0027, obj,"},{"line_number":1818,"context_line":"                                          policy\u003dPOLICIES.legacy)"}],"source_content_type":"text/x-python","patch_set":1,"id":"904e79ef_c3ff6aa1","line":1815,"in_reply_to":"664c6b48_e15a4421","updated":"2026-09-15 02:33:28.000000000","message":"Apparently the set intersection result ends up reassigned to set:\nhttps://github.com/NVIDIA/swift/blob/00fa2c9/swift/obj/replicator.py#L564\nThis what I see in PDB:\n```sh\nvagrant@saio:/vagrant/swift$ pytest test/unit/obj/test_replicator.py -k continues\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 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\nplatform linux -- Python 3.12.3, pytest-9.0.2, pluggy-1.6.0 -- /usr/bin/python3\ncachedir: .pytest_cache\nrootdir: /vagrant/swift\nconfigfile: tox.ini\nplugins: cov-7.0.0\ncollected 75 items / 73 deselected / 2 selected\n\ntest/unit/obj/test_replicator.py::TestObjectReplicator::test_delete_partition_ssync_quarantine_failure_continues\n\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e PDB set_trace (IO-capturing turned off) \u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\n\u003e /vagrant/swift/swift/obj/replicator.py(560)revert()\n-\u003e for cand_objs in synced_remote_regions.values():\n(Pdb) p type(delete_objs)\n\u003cclass \u0027set\u0027\u003e\n(Pdb) p type(cand_objs)\n\u003cclass \u0027odict_keys\u0027\u003e\n(Pdb)\n```\nwith this diff:\n```py\n--- a/test/unit/obj/test_replicator.py\n+++ b/test/unit/obj/test_replicator.py\n@@ -1835,4 +1835,3 @@ class TestObjectReplicator(BaseUnitTestCase):\n         self.replicator.logger \u003d mock_logger \u003d debug_logger(\u0027test-replicator\u0027)\n-        ordered_hashes \u003d []\n-        candidates \u003d {}\n+        ordered_hashes \u003d collections.OrderedDict()\n         for obj in (\u0027o1\u0027, \u0027o2\u0027):\n@@ -1845,16 +1844,9 @@ class TestObjectReplicator(BaseUnitTestCase):\n             ohash \u003d hash_path(\u0027a\u0027, \u0027c\u0027, obj)\n-            ordered_hashes.append(ohash)\n-            candidates[ohash] \u003d ts\n+            ordered_hashes[ohash] \u003d ts\n+        failed_hash, other_hash \u003d ordered_hashes\n         failed_quarantine_suffix \u003d os.path.join(\n-            self.objects, \u00271\u0027, ordered_hashes[0][-3:])\n-        other_suffix \u003d os.path.join(self.objects, \u00271\u0027, ordered_hashes[1][-3:])\n+            self.objects, \u00271\u0027, failed_hash[-3:])\n+        other_suffix \u003d os.path.join(self.objects, \u00271\u0027, other_hash[-3:])\n         self.replicator.sync_method \u003d \u0027ssync\u0027\n\n-        delete_handoff_objs \u003d self.replicator.delete_handoff_objs\n-\n-        def ordered_delete_handoff_objs(job, delete_objs):\n-            self.assertEqual(set(ordered_hashes), set(delete_objs))\n-            # Use fixture order to attempt the failing suffix first.\n-            return delete_handoff_objs(job, ordered_hashes)\n-\n         rmdir_func \u003d os.rmdir\n@@ -1869,6 +1861,3 @@ class TestObjectReplicator(BaseUnitTestCase):\n                 mock.patch.object(self.replicator, \u0027sync_method_fn\u0027,\n-                                  return_value\u003d(True, candidates)), \\\n-                mock.patch.object(\n-                    self.replicator, \u0027delete_handoff_objs\u0027,\n-                    side_effect\u003dordered_delete_handoff_objs), \\\n+                                  return_value\u003d(True, ordered_hashes)), \\\n                 mock.patch(\u0027os.rmdir\u0027, raise_exception_rmdir), \\\n```","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[{"line_number":1812,"context_line":"        diskfiles \u003d []"},{"line_number":1813,"context_line":"        paths \u003d []"},{"line_number":1814,"context_line":"        ordered_hashes \u003d []"},{"line_number":1815,"context_line":"        candidates \u003d {}"},{"line_number":1816,"context_line":"        for obj in (\u0027o1\u0027, \u0027o2\u0027):"},{"line_number":1817,"context_line":"            df \u003d self.df_mgr.get_diskfile(\u0027sda\u0027, \u00271\u0027, \u0027a\u0027, \u0027c\u0027, obj,"},{"line_number":1818,"context_line":"                                          policy\u003dPOLICIES.legacy)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3d2cce5d_fb01a94b","line":1815,"in_reply_to":"904e79ef_c3ff6aa1","updated":"2026-09-15 22:33:52.000000000","message":"%^\u0026*(ing `synced_remote_regions` - i hate that code 😠\n\nAFAICT you avoided the problem with the nonlocal - kudos","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1820,"context_line":"            diskfiles.append((df, ts))"},{"line_number":1821,"context_line":"            ohash \u003d hash_path(\u0027a\u0027, \u0027c\u0027, obj)"},{"line_number":1822,"context_line":"            ordered_hashes.append(ohash)"},{"line_number":1823,"context_line":"            paths.append(storage_directory(self.objects, 1, ohash))"},{"line_number":1824,"context_line":"            candidates[ohash] \u003d ts"},{"line_number":1825,"context_line":"        failed_quarantine_suffix \u003d os.path.dirname(paths[0])"},{"line_number":1826,"context_line":"        other_suffix \u003d os.path.dirname(paths[1])"}],"source_content_type":"text/x-python","patch_set":1,"id":"51c8a00d_8848ac49","line":1823,"updated":"2026-09-11 14:37:18.000000000","message":"maybe less setup/state if we just grab the suffix off the ohash?","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":false,"context_lines":[{"line_number":1820,"context_line":"            diskfiles.append((df, ts))"},{"line_number":1821,"context_line":"            ohash \u003d hash_path(\u0027a\u0027, \u0027c\u0027, obj)"},{"line_number":1822,"context_line":"            ordered_hashes.append(ohash)"},{"line_number":1823,"context_line":"            paths.append(storage_directory(self.objects, 1, ohash))"},{"line_number":1824,"context_line":"            candidates[ohash] \u003d ts"},{"line_number":1825,"context_line":"        failed_quarantine_suffix \u003d os.path.dirname(paths[0])"},{"line_number":1826,"context_line":"        other_suffix \u003d os.path.dirname(paths[1])"}],"source_content_type":"text/x-python","patch_set":1,"id":"d6d9bc2b_51009100","line":1823,"in_reply_to":"51c8a00d_8848ac49","updated":"2026-09-15 02:33:28.000000000","message":"Acknowledged","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"            paths.append(storage_directory(self.objects, 1, ohash))"},{"line_number":1824,"context_line":"            candidates[ohash] \u003d ts"},{"line_number":1825,"context_line":"        failed_quarantine_suffix \u003d os.path.dirname(paths[0])"},{"line_number":1826,"context_line":"        other_suffix \u003d os.path.dirname(paths[1])"},{"line_number":1827,"context_line":"        self.conf[\u0027sync_method\u0027] \u003d \u0027ssync\u0027"},{"line_number":1828,"context_line":""},{"line_number":1829,"context_line":"        def _fake_ssync(node, job, suffixes, **kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7586057a_08456c6f","line":1826,"updated":"2026-09-11 14:37:18.000000000","message":"so `other_suffix` still gets cleaned up even if `failed_quarantine_suffix` trips the inner exception","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"            paths.append(storage_directory(self.objects, 1, ohash))"},{"line_number":1824,"context_line":"            candidates[ohash] \u003d ts"},{"line_number":1825,"context_line":"        failed_quarantine_suffix \u003d os.path.dirname(paths[0])"},{"line_number":1826,"context_line":"        other_suffix \u003d os.path.dirname(paths[1])"},{"line_number":1827,"context_line":"        self.conf[\u0027sync_method\u0027] \u003d \u0027ssync\u0027"},{"line_number":1828,"context_line":""},{"line_number":1829,"context_line":"        def _fake_ssync(node, job, suffixes, **kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"40d1f7d7_00760eae","line":1826,"in_reply_to":"7586057a_08456c6f","updated":"2026-09-15 02:33:28.000000000","message":"Acknowledged","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1829,"context_line":"        def _fake_ssync(node, job, suffixes, **kwargs):"},{"line_number":1830,"context_line":"            return True, candidates"},{"line_number":1831,"context_line":""},{"line_number":1832,"context_line":"        self.replicator.sync_method \u003d _fake_ssync"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"        delete_handoff_objs \u003d self.replicator.delete_handoff_objs"},{"line_number":1835,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"02017f1e_dacee52f","line":1832,"updated":"2026-09-11 14:37:18.000000000","message":"wow, ok and here\u0027s *more* mocking up here (less idiomatic)","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":true,"context_lines":[{"line_number":1829,"context_line":"        def _fake_ssync(node, job, suffixes, **kwargs):"},{"line_number":1830,"context_line":"            return True, candidates"},{"line_number":1831,"context_line":""},{"line_number":1832,"context_line":"        self.replicator.sync_method \u003d _fake_ssync"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"        delete_handoff_objs \u003d self.replicator.delete_handoff_objs"},{"line_number":1835,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5aa39a02_1f089e89","line":1832,"in_reply_to":"02017f1e_dacee52f","updated":"2026-09-15 02:33:28.000000000","message":"I followed the setup of `test_delete_partition_ssync_with_sync_failure`. What would be a better way of doing this idiomatically?","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[{"line_number":1829,"context_line":"        def _fake_ssync(node, job, suffixes, **kwargs):"},{"line_number":1830,"context_line":"            return True, candidates"},{"line_number":1831,"context_line":""},{"line_number":1832,"context_line":"        self.replicator.sync_method \u003d _fake_ssync"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"        delete_handoff_objs \u003d self.replicator.delete_handoff_objs"},{"line_number":1835,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"949e8d87_2dec2fb8","line":1832,"in_reply_to":"5aa39a02_1f089e89","updated":"2026-09-15 22:33:52.000000000","message":"you found it: using mock","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1846,"context_line":""},{"line_number":1847,"context_line":"        rmdir_func \u003d os.rmdir"},{"line_number":1848,"context_line":""},{"line_number":1849,"context_line":"        def raise_exception_rmdir(exception_class, error_no):"},{"line_number":1850,"context_line":"            def func(directory, dir_fd\u003dNone):"},{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ff00ba5_e0dde7a6","line":1849,"updated":"2026-09-11 14:37:18.000000000","message":"this mock would be a lot easier if it was *just* a replacement/fallthrough for rmdir instead of a function that *creates* the rmdir replacement/fallthrough","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":false,"context_lines":[{"line_number":1846,"context_line":""},{"line_number":1847,"context_line":"        rmdir_func \u003d os.rmdir"},{"line_number":1848,"context_line":""},{"line_number":1849,"context_line":"        def raise_exception_rmdir(exception_class, error_no):"},{"line_number":1850,"context_line":"            def func(directory, dir_fd\u003dNone):"},{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"}],"source_content_type":"text/x-python","patch_set":1,"id":"74367125_aa4b9cab","line":1849,"in_reply_to":"3ff00ba5_e0dde7a6","updated":"2026-09-15 02:33:28.000000000","message":"Acknowledged","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1848,"context_line":""},{"line_number":1849,"context_line":"        def raise_exception_rmdir(exception_class, error_no):"},{"line_number":1850,"context_line":"            def func(directory, dir_fd\u003dNone):"},{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"},{"line_number":1853,"context_line":"                else:"},{"line_number":1854,"context_line":"                    rmdir_func(directory, dir_fd\u003ddir_fd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c1c557fe_0f6bbabd","line":1851,"updated":"2026-09-11 14:37:18.000000000","message":"another way to spell this would be just *observe* the order and make the \"first\" one \"failed\" and then assert the *2nd* one is removed.","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[{"line_number":1848,"context_line":""},{"line_number":1849,"context_line":"        def raise_exception_rmdir(exception_class, error_no):"},{"line_number":1850,"context_line":"            def func(directory, dir_fd\u003dNone):"},{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"},{"line_number":1853,"context_line":"                else:"},{"line_number":1854,"context_line":"                    rmdir_func(directory, dir_fd\u003ddir_fd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e78b8423_eac8f4f5","line":1851,"in_reply_to":"69ad26a3_9b845f50","updated":"2026-09-15 22:33:52.000000000","message":"idk, it just seemd to remove some dependency on the ORDER of the *inputs* to the mock controling the order of the *outcomes* of the mock (which is what we\u0027re interested in)\n\nI feel like any language with a nonlocal closure could avoid the inversion of control pretty equally ... do you not like the spelling you came up with?","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":true,"context_lines":[{"line_number":1848,"context_line":""},{"line_number":1849,"context_line":"        def raise_exception_rmdir(exception_class, error_no):"},{"line_number":1850,"context_line":"            def func(directory, dir_fd\u003dNone):"},{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"},{"line_number":1853,"context_line":"                else:"},{"line_number":1854,"context_line":"                    rmdir_func(directory, dir_fd\u003ddir_fd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"69ad26a3_9b845f50","line":1851,"in_reply_to":"c1c557fe_0f6bbabd","updated":"2026-09-15 02:33:28.000000000","message":"Interesting! Is this a common idiom in testing Python code? At first the bots suggested (a variant of) this approach but I brushed it off. Now that it\u0027s coming from you, I went ahead and implemented my understanding of the suggestion.","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"},{"line_number":1853,"context_line":"                else:"},{"line_number":1854,"context_line":"                    rmdir_func(directory, dir_fd\u003ddir_fd)"},{"line_number":1855,"context_line":"            return func"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"        for err in (make_exception(OSError, EPERM),"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a1db872_584f1ec6","line":1854,"updated":"2026-09-11 14:37:18.000000000","message":"makes `assertFalse(os.path.exists)` feel a little on the nose","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"},{"line_number":1853,"context_line":"                else:"},{"line_number":1854,"context_line":"                    rmdir_func(directory, dir_fd\u003ddir_fd)"},{"line_number":1855,"context_line":"            return func"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"        for err in (make_exception(OSError, EPERM),"}],"source_content_type":"text/x-python","patch_set":1,"id":"37b7082b_beb5e9f7","line":1854,"in_reply_to":"3bde45d2_b598ba1f","updated":"2026-09-15 22:33:52.000000000","message":"pretty sure I was being petty - AFAICT this is the best way to express this:\n\n\"imagine suffix rmdir raised ENOTDIR, so that we can fail *again* at quarantine: the second suffix is still processed\"","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":true,"context_lines":[{"line_number":1851,"context_line":"                if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1852,"context_line":"                    raise make_exception(exception_class, error_no)"},{"line_number":1853,"context_line":"                else:"},{"line_number":1854,"context_line":"                    rmdir_func(directory, dir_fd\u003ddir_fd)"},{"line_number":1855,"context_line":"            return func"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"        for err in (make_exception(OSError, EPERM),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3bde45d2_b598ba1f","line":1854,"in_reply_to":"5a1db872_584f1ec6","updated":"2026-09-15 02:33:28.000000000","message":"I need more help understanding why this is the case.","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1855,"context_line":"            return func"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"        for err in (make_exception(OSError, EPERM),"},{"line_number":1858,"context_line":"                    LockTimeout(None, \u0027quarantine-lock\u0027)):"},{"line_number":1859,"context_line":"            with self.subTest(error\u003dtype(err).__name__):"},{"line_number":1860,"context_line":"                mock_logger.clear()"},{"line_number":1861,"context_line":"                for df, ts in diskfiles:"}],"source_content_type":"text/x-python","patch_set":1,"id":"51694f5c_6b94dc7e","line":1858,"updated":"2026-09-11 14:37:18.000000000","message":"I don\u0027t think this sub-test loop is great; would the `quarnatine_dir_renamer` even raise either of these exceptions?  Does this test/code really *care* what kind of error is raised from `quarnatine_dir_renamer`","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":true,"context_lines":[{"line_number":1855,"context_line":"            return func"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"        for err in (make_exception(OSError, EPERM),"},{"line_number":1858,"context_line":"                    LockTimeout(None, \u0027quarantine-lock\u0027)):"},{"line_number":1859,"context_line":"            with self.subTest(error\u003dtype(err).__name__):"},{"line_number":1860,"context_line":"                mock_logger.clear()"},{"line_number":1861,"context_line":"                for df, ts in diskfiles:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c2d5fb8_4ac22b77","line":1858,"in_reply_to":"51694f5c_6b94dc7e","updated":"2026-09-15 02:33:28.000000000","message":"\u003e would the quarnatine_dir_renamer even raise either of these exceptions\n\nIt does (not sure about EPERM though - but that\u0027s not the point):\n- `OSError` through `os.rename`.\n- `LockTimeout` through `invalidate_hash` which acquires the partition lock.\n\nThe test doesn\u0027t care about the nature of the exception but the production code does because the exceptions are caught separately.","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[{"line_number":1855,"context_line":"            return func"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"        for err in (make_exception(OSError, EPERM),"},{"line_number":1858,"context_line":"                    LockTimeout(None, \u0027quarantine-lock\u0027)):"},{"line_number":1859,"context_line":"            with self.subTest(error\u003dtype(err).__name__):"},{"line_number":1860,"context_line":"                mock_logger.clear()"},{"line_number":1861,"context_line":"                for df, ts in diskfiles:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a9311e68_a9aedd4f","line":1858,"in_reply_to":"8c2d5fb8_4ac22b77","updated":"2026-09-15 22:33:52.000000000","message":"\u003e because the exceptions are caught separately^Wneedlessly narrowly\n\nftfy","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9512079ed191fe86c20ad61575f96946fa3833e","unresolved":true,"context_lines":[{"line_number":1862,"context_line":"                    mkdirs(df._datadir)"},{"line_number":1863,"context_line":"                    with open(os.path.join(df._datadir, ts + \u0027.data\u0027),"},{"line_number":1864,"context_line":"                              \u0027wb\u0027) as f:"},{"line_number":1865,"context_line":"                        f.write(b\u00270\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"                with mock.patch(\u0027swift.obj.replicator.http_connect\u0027,"},{"line_number":1868,"context_line":"                                mock_http_connect(200)), \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"63102062_82064d87","line":1865,"updated":"2026-09-11 14:37:18.000000000","message":"really strange for this \"re\"-setup to happen *again* in the inner loop - better as:\n\n\n```\ndef test_delete_partition_ssync_quarantine_failure_continues():\n    # setup\n    with mocks():\n        replicate\n    self.assertFalse(os.path.exists(other_suffix))\n```","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"20d29ec4497a51783f35c62f42f97009e4163da8","unresolved":true,"context_lines":[{"line_number":1862,"context_line":"                    mkdirs(df._datadir)"},{"line_number":1863,"context_line":"                    with open(os.path.join(df._datadir, ts + \u0027.data\u0027),"},{"line_number":1864,"context_line":"                              \u0027wb\u0027) as f:"},{"line_number":1865,"context_line":"                        f.write(b\u00270\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"                with mock.patch(\u0027swift.obj.replicator.http_connect\u0027,"},{"line_number":1868,"context_line":"                                mock_http_connect(200)), \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"ca6c032e_60522d14","line":1865,"in_reply_to":"63102062_82064d87","updated":"2026-09-15 02:33:28.000000000","message":"We need the fixtures once for each exception type we\u0027re testing. I abandoned the `subTest` approach - does that help address the concern? Am I just missing the point here?","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":false,"context_lines":[{"line_number":1862,"context_line":"                    mkdirs(df._datadir)"},{"line_number":1863,"context_line":"                    with open(os.path.join(df._datadir, ts + \u0027.data\u0027),"},{"line_number":1864,"context_line":"                              \u0027wb\u0027) as f:"},{"line_number":1865,"context_line":"                        f.write(b\u00270\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"                with mock.patch(\u0027swift.obj.replicator.http_connect\u0027,"},{"line_number":1868,"context_line":"                                mock_http_connect(200)), \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"af0253c4_6aba94f7","line":1865,"in_reply_to":"ca6c032e_60522d14","updated":"2026-09-15 22:33:52.000000000","message":"\u003e need the fixtures once for each exception type we\u0027re testing\n\ncorrect\n\n\u003e I abandoned the subTest approach\n\nwhen each subtest needs to repeat the outer loop setup inline: correct","commit_id":"9a2d198e0130860bc8403f83e96bdefe68858fa9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":true,"context_lines":[{"line_number":1829,"context_line":""},{"line_number":1830,"context_line":"    def test_delete_partition_ssync_quarantine_lock_timeout_continues(self):"},{"line_number":1831,"context_line":"        self._test_delete_partition_ssync_quarantine_failure_continues("},{"line_number":1832,"context_line":"            LockTimeout(None, \u0027quarantine-lock\u0027))"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"    def _test_delete_partition_ssync_quarantine_failure_continues(self, err):"},{"line_number":1835,"context_line":"        self.replicator.logger \u003d mock_logger \u003d debug_logger(\u0027test-replicator\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"09897518_0e04495b","line":1832,"updated":"2026-09-15 22:33:52.000000000","message":"there is a missing test that rounds out the story - it should say \"what happens if you `Exception(\u0027kaboom!\u0027)`\"","commit_id":"bd664103928dde998f7b79ec73c572153c8b1b52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":true,"context_lines":[{"line_number":1844,"context_line":"            ohash \u003d hash_path(\u0027a\u0027, \u0027c\u0027, obj)"},{"line_number":1845,"context_line":"            candidates[ohash] \u003d ts"},{"line_number":1846,"context_line":"        suffixes \u003d {os.path.join(self.objects, \u00271\u0027, ohash[-3:])"},{"line_number":1847,"context_line":"                    for ohash in candidates}"},{"line_number":1848,"context_line":"        failed_quarantine_suffix \u003d None"},{"line_number":1849,"context_line":"        self.replicator.sync_method \u003d \u0027ssync\u0027"},{"line_number":1850,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"43fc3d93_f0f59280","line":1847,"updated":"2026-09-15 22:33:52.000000000","message":"`suffix_paths`","commit_id":"bd664103928dde998f7b79ec73c572153c8b1b52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":true,"context_lines":[{"line_number":1854,"context_line":"            nonlocal failed_quarantine_suffix"},{"line_number":1855,"context_line":"            if directory in suffixes and failed_quarantine_suffix is None:"},{"line_number":1856,"context_line":"                failed_quarantine_suffix \u003d directory"},{"line_number":1857,"context_line":"            if directory \u003d\u003d failed_quarantine_suffix:"},{"line_number":1858,"context_line":"                raise OSError(ENOTDIR, os.strerror(ENOTDIR))"},{"line_number":1859,"context_line":"            return rmdir_func(directory, dir_fd\u003ddir_fd)"},{"line_number":1860,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"819f7dac_70b2c646","line":1857,"updated":"2026-09-15 22:33:52.000000000","message":"i like the spelling of \"this is a suffix and I didn\u0027t already fail\"\n\n... we should just raise directly after the nonlocal capture\n\n\n```\nDIFF:\ndiff --git a/test/unit/obj/test_replicator.py b/test/unit/obj/test_replicator.py\nindex ae8161e4e..f2b56e833 100644\n--- a/test/unit/obj/test_replicator.py\n+++ b/test/unit/obj/test_replicator.py\n@@ -1854,7 +1854,6 @@ class TestObjectReplicator(BaseUnitTestCase):\n             nonlocal failed_quarantine_suffix\n             if directory in suffixes and failed_quarantine_suffix is None:\n                 failed_quarantine_suffix \u003d directory\n-            if directory \u003d\u003d failed_quarantine_suffix:\n                 raise OSError(ENOTDIR, os.strerror(ENOTDIR))\n             return rmdir_func(directory, dir_fd\u003ddir_fd)\n \n\nCOMMAND:\ncd /vagrant/swift\npytest test/unit/obj/test_replicator.py::TestObjectReplicator::test_delete_partition_ssync_quarantine_failure_continues\n\nOUTPUT:\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 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\nplatform linux -- Python 3.12.3, pytest-9.1.1, pluggy-1.6.0 -- /usr/bin/python3\ncachedir: .pytest_cache\nrootdir: /vagrant/swift\nconfigfile: tox.ini\nplugins: cov-7.1.0\ncollecting ... collected 1 item\n\ntest/unit/obj/test_replicator.py::TestObjectReplicator::test_delete_partition_ssync_quarantine_failure_continues PASSED [100%]\n```","commit_id":"bd664103928dde998f7b79ec73c572153c8b1b52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d3a3cb2c9fce944b92da4c39b0a3f4a9fd933f33","unresolved":true,"context_lines":[{"line_number":1872,"context_line":"        self.assertEqual(mock_logger.get_lines_for_level(\u0027error\u0027), ["},{"line_number":1873,"context_line":"            \u0027Failed to delete %r ([Errno 20] Not a directory);\u0027"},{"line_number":1874,"context_line":"            \u0027 quarantining.\u0027 % failed_quarantine_suffix,"},{"line_number":1875,"context_line":"            \u0027Failed to quarantine %r (%s)\u0027 % (failed_quarantine_suffix, err),"},{"line_number":1876,"context_line":"        ])"},{"line_number":1877,"context_line":"        self.assertTrue(os.path.isdir(failed_quarantine_suffix))"},{"line_number":1878,"context_line":"        self.assertFalse(os.path.exists(other_suffix))"}],"source_content_type":"text/x-python","patch_set":4,"id":"42f38e4c_365c6a34","line":1875,"updated":"2026-09-15 22:33:52.000000000","message":"this is a pretty good story:\n\n```\nfailed to delete \u003cpath\u003e (not a dir)\nfailed to quarantine \u003cpath\u003e (\u003csome magic string\u003e)\n```\n\nI think the helper would be stronger *with out* the assertions, consider if you wanted:\n\n```\n+    def test_delete_partition_ssync_quarantine_other_error(self):\n+        self._test_delete_partition_ssync_quarantine_failure_continues(\n+            Exception(\u0027kaboom!\u0027))\n+\n```\n\nthe interesting assertion isn\u0027t the logging:\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 FAILURES \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\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___ TestObjectReplicator.test_delete_partition_ssync_quarantine_other_error ____\n\nself \u003d \u003ctest.unit.obj.test_replicator.TestObjectReplicator testMethod\u003dtest_delete_partition_ssync_quarantine_other_error\u003e\n\n    def test_delete_partition_ssync_quarantine_other_error(self):\n\u003e       self._test_delete_partition_ssync_quarantine_failure_continues(\n            Exception(\u0027kaboom!\u0027))\n\ntest/unit/obj/test_replicator.py:1835: \n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \ntest/unit/obj/test_replicator.py:1876: in _test_delete_partition_ssync_quarantine_failure_continues\n    self.assertEqual(mock_logger.get_lines_for_level(\u0027error\u0027), [\nE   AssertionError: Lists differ: [\"Fai[101 chars]g.\", \u0027Error syncing handoff partition: \u0027] !\u003d [\"Fai[101 chars]g.\", \"Failed to quarantine \u0027/mnt/tmp/tmpmx_leq[33 chars]m!)\"]\nE   \nE   First differing element 1:\nE   \u0027Error syncing handoff partition: \u0027\nE   \"Failed to quarantine \u0027/mnt/tmp/tmpmx_leqh1/node/sda/objects/1/061\u0027 (kaboom!)\"\nE   \nE     [\"Failed to delete \u0027/mnt/tmp/tmpmx_leqh1/node/sda/objects/1/061\u0027 ([Errno 20] \"\nE      \u0027Not a directory); quarantining.\u0027,\nE   -  \u0027Error syncing handoff partition: \u0027]\nE   +  \"Failed to quarantine \u0027/mnt/tmp/tmpmx_leqh1/node/sda/objects/1/061\u0027 (kaboom!)\"]\n----------------------------- Captured stdout call -----------------------------\ntest-replicator INFO: Removing partition: /mnt/tmp/tmpmx_leqh1/node/sda/objects-1/1\ntest-replicator INFO: Removing 2 objects\ntest-replicator ERROR: Failed to delete \u0027/mnt/tmp/tmpmx_leqh1/node/sda/objects/1/061\u0027 ([Errno 20] Not a directory); quarantining.\ntest-replicator ERROR: Error syncing handoff partition: \nTraceback (most recent call last):\n  File \"/vagrant/swift/swift/obj/replicator.py\", line 629, in delete_handoff_objs\n    os.rmdir(suffix_dir)\n  File \"/vagrant/swift/test/unit/obj/test_replicator.py\", line 1862, in raise_exception_rmdir\n    raise OSError(ENOTDIR, os.strerror(ENOTDIR))\nNotADirectoryError: [Errno 20] Not a directory\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"/vagrant/swift/swift/obj/replicator.py\", line 585, in revert\n    self.delete_handoff_objs(job, delete_objs)\n  File \"/vagrant/swift/swift/obj/replicator.py\", line 638, in delete_handoff_objs\n    quarantine_dir_renamer(dirname(job[\u0027obj_path\u0027]),\n  File \"/usr/lib/python3.12/unittest/mock.py\", line 1134, in __call__\n    return self._mock_call(*args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3.12/unittest/mock.py\", line 1138, in _mock_call\n    return self._execute_mock_call(*args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3.12/unittest/mock.py\", line 1193, in _execute_mock_call\n    raise effect\nException: kaboom!\ntest-replicator INFO: 8/8 (100.00%) partitions replicated in 0.02s (446.82/sec, 0s remaining)\ntest-replicator INFO: 18 successes, 0 failures\n```\n\nit\u0027s `self.assertTrue(os.path.exists(other_suffix))`\n\nalso, LOL @\n\n```\n\u0027Failed to quarantine %r ([Errno 1] Operation not permitted)\u0027\n```\n\nvs\n\n```\nFailed to quarantine %r (: quarantine-lock)\n```","commit_id":"bd664103928dde998f7b79ec73c572153c8b1b52"}]}