)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":15,"context_line":"The audit step will quarantine a hash directory if and only if it is"},{"line_number":16,"context_line":"found in a PPI-ancestor of its expected partition, i.e. the partition"},{"line_number":17,"context_line":"it would have resided in before a series of PPIs. The length of this"},{"line_number":18,"context_line":"series is hard-coded today to 1. Hash directories that are misplaced for"},{"line_number":19,"context_line":"other reasons are logged as warnings and left untouched."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Additional changes include:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"8bfb7538_9e8ba06a","line":18,"updated":"2026-05-06 14:47:24.000000000","message":"since we know we\u0027re currently in the process of doing back-to-back PPI we should make this a config var.\n\nWhat I don\u0027t want is to make the tool essentially useless at it\u0027s one job (quarantine objects that hash to a different part) by being overly strict.\n\nAlternatively I\u0027m sure we could justify making ancestory unbounded, or even some sort of \"quarantine any mis-hash\" mode.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Additional changes include:"},{"line_number":22,"context_line":"- Replace the STEP_RELINK/STEP_CLEANUP constants with an enum."},{"line_number":23,"context_line":"- Ring-state precondition validation for each step."},{"line_number":24,"context_line":"- Only perform the cleanup step on the lower-half of the partition"},{"line_number":25,"context_line":"  space."},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"ae122a46_09cc474e","line":23,"updated":"2026-05-06 14:47:24.000000000","message":"I *think* some of this already existed?  the exitcode\u003d2 isn\u0027t new - is this JUST saying \"audit also expects ring in correct state; where correct is: any non-PPI-in-progress state\"","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":22,"context_line":"- Replace the STEP_RELINK/STEP_CLEANUP constants with an enum."},{"line_number":23,"context_line":"- Ring-state precondition validation for each step."},{"line_number":24,"context_line":"- Only perform the cleanup step on the lower-half of the partition"},{"line_number":25,"context_line":"  space."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I6d1de8caa1322168e5e26682a586d11dc606e19a"},{"line_number":28,"context_line":"Signed-off-by: Wael Halbawi \u003cwhalbawi@nvidia.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"d3d245f9_452ea3c3","line":25,"updated":"2026-05-06 14:47:24.000000000","message":"... wait this is new?  Or you\u0027re calling it out as \"we used to this based on state file *implying* cleanup; now we do it explicitly/always for cleanup\" ???","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"07bf5d189ff8d082a9facec1251906036008aedb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"424ecf3d_e19495e6","updated":"2026-02-19 05:46:40.000000000","message":"Rebased on top of `2c980ac94`","commit_id":"7d9ce3104aa7db412914bd52db0f9430d6ca6ca9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"2aee4f18_1a3ff69a","updated":"2026-04-09 19:55:24.000000000","message":"this is looking great!  nice find on that hashes list shadow mutation (!!)","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"b1ee163239f5102dde95ca11c2af414c78db4864","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"ae25f184_22656908","updated":"2026-04-16 16:34:09.000000000","message":"recheck","commit_id":"7b6770a647db6cf83f699f377dd01325e18d9b20"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"a0666a89_a338dc1d","updated":"2026-05-06 14:47:24.000000000","message":"1) go ahead and do the plumbing for `max_num_ppi` - probably `max_audit_history_quarantine_threshold\u003d2` (or 3?)  We\u0027ve not had this audit feature for a long time; who knows how many PPIs have gone by.  A conservative default of `\u003d1` is justifiable as long as we can configure it to `\u003d2` in PDX.\n2) I don\u0027t like the new RuntimeError for stale cleanup state in the part filter; I think we need to just consider the \"cleanup prev_part_power filter is wrong\" as a pre-existing bug and either a) maintain it as-is or b) fix it in a pre-factor\n3) I don\u0027t think we need a \"permissive audit all parts for mis-homed foreign hashes\" mode, that\u0027s starting to tread into object-auditor territory and the idea with re-using the relinker for audit was that we could make it more efficient and get it delivered sooner.  Let\u0027s reject it as scope creep and try and get the \"lower half audit for ppi orphans\" merged.\n4) I think that \"quarantined mis-homed ancestor\" path is *necessary* (if you relink and cleanup an ancestor older than reclaim you risk missing a tombstone in the real path) - so since we HAVE to write that path we should *deliver* it BEFORE we scope creep into \"yeah but if it WAS new enough we COULD relink \u0026 cleanup\" - YAGNI.\n\nI struggled to get used to reading:\n\n```\n_is_ppi_ancestor(n, 2n[+1])\n```\n\n... but at this point I think I\u0027ve got it and would advocate *against* changing the interface or name.  It is a useful abstraction worth understanding (if possible):\n\n```\n    def test_is_ppi_ancestor_exahustive(self):\n        def make_part_map(pp):\n            return {n: (2 * n, 2 * n + 1) for n in range(2 ** pp)}\n\n        # sanity\n        prev_pp_3 \u003d make_part_map(3)\n        self.assertEqual({\n            0: (0, 1),  # old n \u003d\u003e 2n, 2n+1\n            1: (2, 3),\n            2: (4, 5),\n            3: (6, 7),\n            4: (8, 9),\n            5: (10, 11),\n            6: (12, 13),\n            7: (14, 15),\n        }, prev_pp_3)\n\n        # if we have parts 0-15 (i.e. pp\u003d4) and we \"found\" a hash sitting in a\n        # part e.g. 2 but our hash shift says THAT hash belongs in 4 (or 5!) we\n        # say can still say \"yes, 2 is an ancestor of 4\"\n        for found, new_parts in prev_pp_3.items():\n            for p in new_parts:\n                self.assertTrue(relinker._is_ppi_ancestor(found, p))\n\n        # similarly for parts 0-31 (i.e. pp\u003d5) we mind find a hash in an\n        # ancestry part (0-16)\n        prev_pp_4 \u003d make_part_map(4)\n        for found, new_parts in prev_pp_4.items():\n            for p in new_parts:\n                self.assertTrue(relinker._is_ppi_ancestor(found, p))\n        # but notably, for any part 0-7, it could be an ancestor of any of\n        # it\u0027s descendants (if you look back 2 generations!)\n        for found, descendents in prev_pp_3.items():\n            for c in descendents:\n                for p in prev_pp_4[c]:\n                    if p \u003d\u003d 0:\n                        # for the hashes expected to be in part 0, the part 0\n                        # is both their parent AND grandparent\n                        self.assertEqual(found, 0)\n                    elif p \u003d\u003d 1:\n                        # for the hashes expected to be in part 1, the part 0\n                        # is both their parent AND grandparent\n                        self.assertEqual(found, 0)\n                    else:\n                        # for any \"normal\" part, including prev_pp_3[1] which\n                        # mapped to (2, 3) - generally it\u0027s grandchildren are\n                        # NOT it\u0027s children\n                        self.assertFalse(\n                            relinker._is_ppi_ancestor(found, p, max_num_ppi\u003d1))\n                    self.assertTrue(\n                        relinker._is_ppi_ancestor(found, p, max_num_ppi\u003d2))\n                    # N.B. default behavior is \"unbounded\"\n                    self.assertTrue(relinker._is_ppi_ancestor(found, p))\n                    self.assertTrue(relinker._is_ppi_ancestor(\n                        found, p, max_num_ppi\u003dNone))\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"ca0bb70d073faab4735a055e5759de98a87d22e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"53279fa3_3c6a6900","updated":"2026-05-05 18:00:36.000000000","message":"recheck","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"9d861f2ed5dfa1077db6cc1cff2265b925da4fed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"8bdd642e_d35db268","updated":"2026-05-01 21:35:17.000000000","message":"recheck","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"}],"swift/cli/relinker.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":257,"context_line":"            prev_part_power \u003d self.part_power - 1"},{"line_number":258,"context_line":"            if \"prev_part_power\" in self.states:"},{"line_number":259,"context_line":"                # this is a thing that CLEANUP tracks via state file"},{"line_number":260,"context_line":"                assert self.states[\"prev_part_power\"] \u003d\u003d prev_part_power"},{"line_number":261,"context_line":"            # All partitions in the upper half are new partitions and"},{"line_number":262,"context_line":"            # there is nothing to clean up (or audit) there"},{"line_number":263,"context_line":"            cutoff \u003d 2 ** prev_part_power"}],"source_content_type":"text/x-python","patch_set":2,"id":"1d1b7fcd_b101aaf7","line":260,"updated":"2026-04-09 19:55:24.000000000","message":"this was more of a sanity check - but bandit doesn\u0027t like the assert statement\n\nmaybe better to raise a RunTimeError - or even suggest deletion of the state file?   It might depend on how seriously we want to support `prev_part_power \u003c part_power - 1`","commit_id":"714d59901c1d23841ffba1e936ca63779904bb15"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":257,"context_line":"            prev_part_power \u003d self.part_power - 1"},{"line_number":258,"context_line":"            if \"prev_part_power\" in self.states:"},{"line_number":259,"context_line":"                # this is a thing that CLEANUP tracks via state file"},{"line_number":260,"context_line":"                assert self.states[\"prev_part_power\"] \u003d\u003d prev_part_power"},{"line_number":261,"context_line":"            # All partitions in the upper half are new partitions and"},{"line_number":262,"context_line":"            # there is nothing to clean up (or audit) there"},{"line_number":263,"context_line":"            cutoff \u003d 2 ** prev_part_power"}],"source_content_type":"text/x-python","patch_set":2,"id":"30c3ec5f_bd4ade84","line":260,"in_reply_to":"1d1b7fcd_b101aaf7","updated":"2026-04-15 20:25:10.000000000","message":"Acknowledged","commit_id":"714d59901c1d23841ffba1e936ca63779904bb15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":392,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"},{"line_number":393,"context_line":"            if fname \u003d\u003d replace_partition_in_path("},{"line_number":394,"context_line":"                    self.conf[\u0027devices\u0027], fname, self.next_part_power):"},{"line_number":395,"context_line":"                hashes.remove(hsh)"},{"line_number":396,"context_line":"        return hashes"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def do_relink(self, device, hash_path, new_hash_path, filename,"}],"source_content_type":"text/x-python","patch_set":7,"id":"605091d4_3d0bfed9","side":"PARENT","line":395,"updated":"2026-04-09 19:55:24.000000000","message":"holy crap, we create a copy with `hashes \u003d list(hashes)` but that\u0027s just shadowing - this should be blowing up in prod - WTF is going on!?\n\nI think this is probably very very bad news:\n\n```\n\u003e\u003e\u003e a \u003d list(range(5))\n\u003e\u003e\u003e for x in a:\n...     if x \u003d\u003d 2:\n...         a.remove(x)\n...     print(x, a)\n... \n0 [0, 1, 2, 3, 4]\n1 [0, 1, 2, 3, 4]\n2 [0, 1, 3, 4]\n4 [0, 1, 3, 4]\n```","commit_id":"9d684080daf73f2506655bf1a54c96b8685d7f93"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":392,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"},{"line_number":393,"context_line":"            if fname \u003d\u003d replace_partition_in_path("},{"line_number":394,"context_line":"                    self.conf[\u0027devices\u0027], fname, self.next_part_power):"},{"line_number":395,"context_line":"                hashes.remove(hsh)"},{"line_number":396,"context_line":"        return hashes"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def do_relink(self, device, hash_path, new_hash_path, filename,"}],"source_content_type":"text/x-python","patch_set":7,"id":"c2a3d6f9_63ad00e7","side":"PARENT","line":395,"in_reply_to":"605091d4_3d0bfed9","updated":"2026-04-15 20:25:10.000000000","message":"Acknowledged","commit_id":"9d684080daf73f2506655bf1a54c96b8685d7f93"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":659,"context_line":"            new_hash_path \u003d replace_partition_in_path("},{"line_number":660,"context_line":"                self.conf[\u0027devices\u0027], hash_path, self.next_part_power)"},{"line_number":661,"context_line":"            if new_hash_path \u003d\u003d hash_path:"},{"line_number":662,"context_line":"                continue"},{"line_number":663,"context_line":"            self.process_location(device, hash_path, new_hash_path)"},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"        # any unmounted devices don\u0027t trigger the pre_device trigger."}],"source_content_type":"text/x-python","patch_set":7,"id":"45d2596b_9b572619","side":"PARENT","line":662,"updated":"2026-04-09 19:55:24.000000000","message":"I find I\u0027m still confused about the *apparent* duplication this logic and `self.hashes_filter` - i can\u0027t tell if BOTH are needed for correctness or if EITHER would be fine?  Might be worth getting the bottom of - probably better as a pre-factor as opposed to \"cargo cult complexity\" that we \"try to ignore while review/understanding\" the correctness and completeness of the new tri-state audit hash engine.","commit_id":"9d684080daf73f2506655bf1a54c96b8685d7f93"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":659,"context_line":"            new_hash_path \u003d replace_partition_in_path("},{"line_number":660,"context_line":"                self.conf[\u0027devices\u0027], hash_path, self.next_part_power)"},{"line_number":661,"context_line":"            if new_hash_path \u003d\u003d hash_path:"},{"line_number":662,"context_line":"                continue"},{"line_number":663,"context_line":"            self.process_location(device, hash_path, new_hash_path)"},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"        # any unmounted devices don\u0027t trigger the pre_device trigger."}],"source_content_type":"text/x-python","patch_set":7,"id":"4800db97_9ed598e6","side":"PARENT","line":662,"in_reply_to":"45d2596b_9b572619","updated":"2026-04-15 20:25:10.000000000","message":"I think it\u0027s unintended duplication where the second one protected us from \"blowing up in prod\". First came the inline filter: [779566](https://review.opendev.org/c/openstack/swift/+/779566/8/swift/cli/relinker.py#396), then the buggy hook: [695344](https://review.opendev.org/c/openstack/swift/+/695344/9/swift/cli/relinker.py#137)","commit_id":"9d684080daf73f2506655bf1a54c96b8685d7f93"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":757,"context_line":"    \"\"\""},{"line_number":758,"context_line":"    Fork Relinker workers based on config and wait for them to finish."},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"    :param do_cleanup: boolean, if workers should perform cleanup step"},{"line_number":761,"context_line":"    :param conf: dict, config options"},{"line_number":762,"context_line":"    :param logger: SwiftLogAdapter instance"},{"line_number":763,"context_line":"    :kwarg device_list: list of strings, optionally limit to specific devices"}],"source_content_type":"text/x-python","patch_set":7,"id":"33633f06_b63d6fa1","side":"PARENT","line":760,"updated":"2026-04-09 19:55:24.000000000","message":"nice cleanup on this doc-string - the input param was already `step` from \n\n\u003e     return parallel_process(args.action, conf, logger, args.device_list)","commit_id":"9d684080daf73f2506655bf1a54c96b8685d7f93"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":757,"context_line":"    \"\"\""},{"line_number":758,"context_line":"    Fork Relinker workers based on config and wait for them to finish."},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"    :param do_cleanup: boolean, if workers should perform cleanup step"},{"line_number":761,"context_line":"    :param conf: dict, config options"},{"line_number":762,"context_line":"    :param logger: SwiftLogAdapter instance"},{"line_number":763,"context_line":"    :kwarg device_list: list of strings, optionally limit to specific devices"}],"source_content_type":"text/x-python","patch_set":7,"id":"dffe2b5a_2019ed07","side":"PARENT","line":760,"in_reply_to":"33633f06_b63d6fa1","updated":"2026-04-15 20:25:10.000000000","message":"Acknowledged","commit_id":"9d684080daf73f2506655bf1a54c96b8685d7f93"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":424,"context_line":"        else:"},{"line_number":425,"context_line":"            expected_part_power \u003d self.next_part_power"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        mishashed \u003d list(hashes)"},{"line_number":428,"context_line":"        for hsh in hashes:"},{"line_number":429,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"},{"line_number":430,"context_line":"            if fname \u003d\u003d replace_partition_in_path("}],"source_content_type":"text/x-python","patch_set":7,"id":"01580258_ed9a5ad3","line":427,"updated":"2026-04-09 19:55:24.000000000","message":"starting with an empty list also avoids the need to make a copy that we can mutate while iterating","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":424,"context_line":"        else:"},{"line_number":425,"context_line":"            expected_part_power \u003d self.next_part_power"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        mishashed \u003d list(hashes)"},{"line_number":428,"context_line":"        for hsh in hashes:"},{"line_number":429,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"},{"line_number":430,"context_line":"            if fname \u003d\u003d replace_partition_in_path("}],"source_content_type":"text/x-python","patch_set":7,"id":"b6b8d937_80ab90a7","line":427,"in_reply_to":"01580258_ed9a5ad3","updated":"2026-04-15 20:25:10.000000000","message":"Acknowledged","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":429,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"},{"line_number":430,"context_line":"            if fname \u003d\u003d replace_partition_in_path("},{"line_number":431,"context_line":"                    self.conf[\u0027devices\u0027], fname, expected_part_power):"},{"line_number":432,"context_line":"                mishashed.remove(hsh)"},{"line_number":433,"context_line":"        return mishashed"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"    def do_relink(self, device, hash_path, new_hash_path, filename,"}],"source_content_type":"text/x-python","patch_set":7,"id":"0d9561e4_139ea763","line":432,"updated":"2026-04-09 19:55:24.000000000","message":"I like `return mishashed` A LOT - having the semantic meaning of the returned list of strings encoded into the function\u0027s named return value is good intuition - KUDOS\n\nnit: I probably would have found this more *obvious* if `mishased \u003d []` started life empty and got \"filled\" with `fname !\u003d epected_path`","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":429,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"},{"line_number":430,"context_line":"            if fname \u003d\u003d replace_partition_in_path("},{"line_number":431,"context_line":"                    self.conf[\u0027devices\u0027], fname, expected_part_power):"},{"line_number":432,"context_line":"                mishashed.remove(hsh)"},{"line_number":433,"context_line":"        return mishashed"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"    def do_relink(self, device, hash_path, new_hash_path, filename,"}],"source_content_type":"text/x-python","patch_set":7,"id":"be8d9a84_e795fe0a","line":432,"in_reply_to":"0d9561e4_139ea763","updated":"2026-04-15 20:25:10.000000000","message":"Goes away entirely in [984664](https://review.opendev.org/c/openstack/swift/+/984664)","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":741,"context_line":"                new_hash_path \u003d replace_partition_in_path("},{"line_number":742,"context_line":"                    self.conf[\u0027devices\u0027], hash_path, self.next_part_power)"},{"line_number":743,"context_line":"                if new_hash_path \u003d\u003d hash_path:"},{"line_number":744,"context_line":"                    continue"},{"line_number":745,"context_line":"                self.process_location(device, hash_path, new_hash_path)"},{"line_number":746,"context_line":""},{"line_number":747,"context_line":"        # any unmounted devices don\u0027t trigger the pre_device trigger."}],"source_content_type":"text/x-python","patch_set":7,"id":"92c76b0d_3721911d","line":744,"updated":"2026-04-09 19:55:24.000000000","message":"one reason I think this distraction about `hashes_filter` \"jumps off the diff\" - is because we\u0027re not applying the \"double check new_hash_path\" logic for `step \u003d\u003d AUDIT` (why would we assume that is correct, if we trust the old code enough to maintain it\u0027s current behavior)","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":741,"context_line":"                new_hash_path \u003d replace_partition_in_path("},{"line_number":742,"context_line":"                    self.conf[\u0027devices\u0027], hash_path, self.next_part_power)"},{"line_number":743,"context_line":"                if new_hash_path \u003d\u003d hash_path:"},{"line_number":744,"context_line":"                    continue"},{"line_number":745,"context_line":"                self.process_location(device, hash_path, new_hash_path)"},{"line_number":746,"context_line":""},{"line_number":747,"context_line":"        # any unmounted devices don\u0027t trigger the pre_device trigger."}],"source_content_type":"text/x-python","patch_set":7,"id":"ff5a1ca9_d183193e","line":744,"in_reply_to":"92c76b0d_3721911d","updated":"2026-04-15 20:25:10.000000000","message":"Acknowledged","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":777,"context_line":"            if self.step in (Step.RELINK, Step.CLEANUP):"},{"line_number":778,"context_line":"                if not ring.next_part_power:"},{"line_number":779,"context_line":"                    continue"},{"line_number":780,"context_line":"                part_power_increased \u003d ring.next_part_power \u003d\u003d ring.part_power"},{"line_number":781,"context_line":"                do_cleanup \u003d self.step \u003d\u003d Step.CLEANUP"},{"line_number":782,"context_line":"                if do_cleanup !\u003d part_power_increased:"},{"line_number":783,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":7,"id":"bec2feca_22c01487","line":780,"updated":"2026-04-09 19:55:24.000000000","message":"How we think these ring attributes work:\n\n```\ns-r-b prepare_part_power \u003d\u003e ring.next_part_power gets set to your target\n\u003crun relink\u003e\ns-r-b increase_part_power \u003d\u003e ring.part_power gets set to ring.next_part_power\n  (N.B. next_part_power is non-None \u003d\u003d ring.part_power)\n\u003crun cleanup\u003e\ns-r-b finish_part_power \u003d\u003e ring.next_part_power \u003d None\n```","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":780,"context_line":"                part_power_increased \u003d ring.next_part_power \u003d\u003d ring.part_power"},{"line_number":781,"context_line":"                do_cleanup \u003d self.step \u003d\u003d Step.CLEANUP"},{"line_number":782,"context_line":"                if do_cleanup !\u003d part_power_increased:"},{"line_number":783,"context_line":"                    continue"},{"line_number":784,"context_line":"            elif self.step \u003d\u003d Step.AUDIT:"},{"line_number":785,"context_line":"                if ring.next_part_power is not None:"},{"line_number":786,"context_line":"                    self.logger.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"bfadf223_25e8820e","line":783,"updated":"2026-04-09 19:55:24.000000000","message":"perfect, so this is saying that \"you can only cleanup if the ring is in the `increase_part_power` step\"","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":780,"context_line":"                part_power_increased \u003d ring.next_part_power \u003d\u003d ring.part_power"},{"line_number":781,"context_line":"                do_cleanup \u003d self.step \u003d\u003d Step.CLEANUP"},{"line_number":782,"context_line":"                if do_cleanup !\u003d part_power_increased:"},{"line_number":783,"context_line":"                    continue"},{"line_number":784,"context_line":"            elif self.step \u003d\u003d Step.AUDIT:"},{"line_number":785,"context_line":"                if ring.next_part_power is not None:"},{"line_number":786,"context_line":"                    self.logger.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"e7cdc2bc_f0c5d8e5","line":783,"in_reply_to":"bfadf223_25e8820e","updated":"2026-04-15 20:25:10.000000000","message":"Acknowledged","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52817ff6fef95d13395b51e76cbc941990e4d65c","unresolved":true,"context_lines":[{"line_number":782,"context_line":"                if do_cleanup !\u003d part_power_increased:"},{"line_number":783,"context_line":"                    continue"},{"line_number":784,"context_line":"            elif self.step \u003d\u003d Step.AUDIT:"},{"line_number":785,"context_line":"                if ring.next_part_power is not None:"},{"line_number":786,"context_line":"                    self.logger.warning("},{"line_number":787,"context_line":"                        f\"policy\u003d{policy.name}: \""},{"line_number":788,"context_line":"                        \"Skipping audit as relink/cleanup is in progress\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"96bd9c3b_76b6fc4c","line":785,"updated":"2026-04-09 19:55:24.000000000","message":"ok, so there is some kind of (non-obvious?) mirroring between \"you can only relink/cleanup when you\u0027re in the right phase of a PPI\" and \"you can only audit when you\u0027re NOT in one of the PPI phases\"","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"f3869afb8cc00d7533825de05b3448e81ced7d64","unresolved":false,"context_lines":[{"line_number":782,"context_line":"                if do_cleanup !\u003d part_power_increased:"},{"line_number":783,"context_line":"                    continue"},{"line_number":784,"context_line":"            elif self.step \u003d\u003d Step.AUDIT:"},{"line_number":785,"context_line":"                if ring.next_part_power is not None:"},{"line_number":786,"context_line":"                    self.logger.warning("},{"line_number":787,"context_line":"                        f\"policy\u003d{policy.name}: \""},{"line_number":788,"context_line":"                        \"Skipping audit as relink/cleanup is in progress\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"58a4b4c0_f5374e6e","line":785,"in_reply_to":"96bd9c3b_76b6fc4c","updated":"2026-04-15 20:25:10.000000000","message":"Correct. Worth investigating maintaining explicit state somewhere (ring file?) rather than implicitly deducing from part_power and next_part_power.","commit_id":"806866426dde8c77f72fd85e3ae546f944a6e4e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":234,"context_line":"        # Remove all non partitions first (eg: auditor_status_ALL.json)"},{"line_number":235,"context_line":"        partitions \u003d [p for p in partitions if p.isdigit()]"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"        relinking \u003d (self.part_power !\u003d self.next_part_power)"},{"line_number":238,"context_line":"        if relinking:"},{"line_number":239,"context_line":"            # All partitions in the upper half are new partitions and there is"},{"line_number":240,"context_line":"            # nothing to relink there"}],"source_content_type":"text/x-python","patch_set":12,"id":"be523fae_5ef28a04","side":"PARENT","line":237,"updated":"2026-05-06 14:47:24.000000000","message":"IIUC this is no longer sufficient to exclusively identifiy step \u003d\u003d relink b/c during audit `self.next_part_power \u003d\u003d None !\u003d self.part_power`","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":240,"context_line":"            # nothing to relink there"},{"line_number":241,"context_line":"            partitions \u003d [part for part in partitions"},{"line_number":242,"context_line":"                          if int(part) \u003c 2 ** self.part_power]"},{"line_number":243,"context_line":"        elif \"prev_part_power\" in self.states:"},{"line_number":244,"context_line":"            # All partitions in the upper half are new partitions and there is"},{"line_number":245,"context_line":"            # nothing to clean up there"},{"line_number":246,"context_line":"            partitions \u003d [part for part in partitions"}],"source_content_type":"text/x-python","patch_set":12,"id":"e5291aef_fc31636c","side":"PARENT","line":243,"updated":"2026-05-06 14:47:24.000000000","message":"if you read `hook_pre_device` it pretty clearly reads that if the state-file doesn\u0027t match what we expect (`next_pp \u003d\u003d next_pp`) we unlink it and continue like it doesn\u0027t exist; so `prev_part_power in states` is clearly a kind of \"best effort\"\n\n... the fallback is \"cleanup all parts\" (inefficient, but not wrong)\n\nI can *imagine* a \"permissive mode audit\" that checks ALL parts for mis-homed hashes and quarntines any that don\u0027t belong there.  Similar to configurable max_ppi_quarantine_threshold \u003d 32 we might want to allow for audit to \"visit all parts\" as a *fallback*\n\nThat said, I think it would actually be nice improvement if the \"cleanup only lower half\" was less dependent on transient-state-file.  And also agree that AUDIT could (should?) sensibly (default?) to do \"just the lower half\"","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":244,"context_line":"            # All partitions in the upper half are new partitions and there is"},{"line_number":245,"context_line":"            # nothing to clean up there"},{"line_number":246,"context_line":"            partitions \u003d [part for part in partitions"},{"line_number":247,"context_line":"                          if int(part) \u003c 2 ** self.states[\"prev_part_power\"]]"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        # Format: { \u0027part\u0027: processed }"},{"line_number":250,"context_line":"        if self.states[\"state\"]:"}],"source_content_type":"text/x-python","patch_set":12,"id":"80e3a2e1_4dc9d5df","side":"PARENT","line":247,"updated":"2026-05-06 14:47:24.000000000","message":"I think using `self.states[\u0027prev_part_power\u0027]` is significantly worse than `self.part_power - 1`\n\nRunning cleanup with a stale `state_file[\u0027part_power\u0027] \u003d 10` while having `self.part_power \u003d 12` isn\u0027t just \"inefficient\" - it would skip relink/cleanup on parts 1024-2048 (??)\n\nI think this would be \"strictly better\" if `cleanup` just only/always filtered `parts \u003c self.part_power - 1`","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":46,"context_line":"EXIT_SUCCESS \u003d 0"},{"line_number":47,"context_line":"EXIT_NO_APPLICABLE_POLICY \u003d 2"},{"line_number":48,"context_line":"EXIT_ERROR \u003d 1"},{"line_number":49,"context_line":"DEFAULT_STATS_INTERVAL \u003d 300.0"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Step(Enum):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9e5cd0c2_1a68d99c","line":49,"updated":"2026-05-06 14:47:24.000000000","message":"if we\u0027re going to \"hard code\" something - this is where I would expect to see/change it","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"def _is_ppi_ancestor(found_part, expected_part, max_num_ppi\u003dNone):"},{"line_number":110,"context_line":"    \"\"\""},{"line_number":111,"context_line":"    Determine if `expected_part` descended from `found_part` via a sequence of"},{"line_number":112,"context_line":"    partition power increase operations. The search is limited to at most"},{"line_number":113,"context_line":"    `max_num_ppi` operations if it is not None, and is unlimited otherwise."},{"line_number":114,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"750ff2d8_2be37c4a","line":111,"updated":"2026-05-06 14:47:24.000000000","message":"I read \"descended\" as `(2n, 2n+1)` like if I found `5` it is an \"descendet\" of... `2` b/c `2(2)+1\u003d5`\n\nor \"2\" is an ancestor of \"5\"\n\nfound ancestor expected \u003d expect descended found","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                on_disk_part_power \u003d state_from_disk[\"part_power\"]"},{"line_number":229,"context_line":"                if on_disk_part_power !\u003d self.states[\"part_power\"]:"},{"line_number":230,"context_line":"                    self.states[\"prev_part_power\"] \u003d on_disk_part_power"},{"line_number":231,"context_line":"                    raise ValueError"},{"line_number":232,"context_line":"                self.states[\"state\"].update(state_from_disk[\"state\"])"},{"line_number":233,"context_line":"        except (ValueError, TypeError, KeyError):"},{"line_number":234,"context_line":"            # Bad state file: remove the file to restart from scratch"}],"source_content_type":"text/x-python","patch_set":12,"id":"a9819e8a_4d3894db","line":231,"updated":"2026-05-06 14:47:24.000000000","message":"this is gross - we pickup the state from disk, and then carry it forward.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":279,"context_line":"            partitions \u003d [part for part in partitions"},{"line_number":280,"context_line":"                          if int(part) \u003c 2 ** (self.part_power - 1)]"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            raise RuntimeError(f\"Invalid step: {self.step}\")"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        # Format: { \u0027part\u0027: processed }"},{"line_number":285,"context_line":"        if self.states[\"state\"]:"}],"source_content_type":"text/x-python","patch_set":12,"id":"dcfa9bb4_cc8623f8","line":282,"updated":"2026-05-06 14:47:24.000000000","message":"oic, all this `RuntimeError` stuff in the partitions filter is *new* - yuk\n\nCan we just not do this yet?","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":425,"context_line":"        if self.step \u003d\u003d Step.AUDIT:"},{"line_number":426,"context_line":"            expected_part_power \u003d self.part_power"},{"line_number":427,"context_line":"        else:"},{"line_number":428,"context_line":"            expected_part_power \u003d self.next_part_power"},{"line_number":429,"context_line":"        mismatched_hashes \u003d list()"},{"line_number":430,"context_line":"        for hsh in hashes:"},{"line_number":431,"context_line":"            fname \u003d os.path.join(suff_path, hsh)"}],"source_content_type":"text/x-python","patch_set":12,"id":"6cea5cea_8e998f41","line":428,"updated":"2026-05-06 14:47:24.000000000","message":"I think I might use this `expect_part_power` pattern in `partitions_filter` as well?","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":551,"context_line":"        expected_part \u003d get_partition_from_path(self.conf[\u0027devices\u0027],"},{"line_number":552,"context_line":"                                                expected_hash_path)"},{"line_number":553,"context_line":"        found_part \u003d get_partition_from_path(self.conf[\u0027devices\u0027], hash_path)"},{"line_number":554,"context_line":"        # TODO: Take in as a CLI argument"},{"line_number":555,"context_line":"        max_num_ppi \u003d 1"},{"line_number":556,"context_line":"        if not _is_ppi_ancestor(found_part,"},{"line_number":557,"context_line":"                                expected_part,"}],"source_content_type":"text/x-python","patch_set":12,"id":"73742103_8bf5871d","line":554,"updated":"2026-05-06 14:47:24.000000000","message":"bah, stupid CLI args vs config values\n\neven if it\u0027s configuable it will need a good default - so you could throw that in a constant at the top","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":560,"context_line":"                f\"hash_path\u003d{hash_path}: found_part\u003d{found_part} \" +"},{"line_number":561,"context_line":"                f\"not an ancestor of expected_part\u003d{expected_part}: \" +"},{"line_number":562,"context_line":"                \"Skipping quarantine\")"},{"line_number":563,"context_line":"            self.stats[\u0027warnings\u0027] \u003d self.stats.get(\u0027warnings\u0027, 0) + 1"},{"line_number":564,"context_line":"            return"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"8cdbceee_476906b3","line":563,"updated":"2026-05-06 14:47:24.000000000","message":"for me tests pass with this diff:\n\n```\ndiff --git a/swift/cli/relinker.py b/swift/cli/relinker.py\nindex 6db1490b5b..08d4b382e0 100644\n--- a/swift/cli/relinker.py\n+++ b/swift/cli/relinker.py\n@@ -560,7 +560,7 @@ class Relinker(object):\n                 f\"hash_path\u003d{hash_path}: found_part\u003d{found_part} \" +\n                 f\"not an ancestor of expected_part\u003d{expected_part}: \" +\n                 \"Skipping quarantine\")\n-            self.stats[\u0027warnings\u0027] \u003d self.stats.get(\u0027warnings\u0027, 0) + 1\n+            self.stats[\u0027warnings\u0027] +\u003d 1\n             return\n \n         try:\n```\n\nI think it\u0027s b/c they\u0027re functionally equivilent - but the `self.stats[\u0027hash_dirs\u0027] +\u003d 1` seems more idiomatic and in-line with surrounding style.\n\nIf there\u0027s  *reason* that it should be written differently (e.g. do we ever \"initialize stats from disk\" (!?) instead of `_zero_stats`) - then a test should help me maintain it to be correct.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":568,"context_line":"            to_dir \u003d diskfile.quarantine_renamer(dev_path, hash_path)"},{"line_number":569,"context_line":"            self.logger.info(f\"found_part\u003d{found_part} \" +"},{"line_number":570,"context_line":"                             f\"expected_part\u003d{expected_part}: \" +"},{"line_number":571,"context_line":"                             f\"{hash_path} moved to {to_dir}\")"},{"line_number":572,"context_line":"            self.stats[\u0027removed\u0027] +\u003d 1"},{"line_number":573,"context_line":"        except Exception as exc:"},{"line_number":574,"context_line":"            self.logger.exception(\"Could not quarantine \" +"}],"source_content_type":"text/x-python","patch_set":12,"id":"70ef7ca6_883ef4da","line":571,"updated":"2026-05-06 14:47:24.000000000","message":"I\u0027m realizing there\u0027s some case to be made that w/i a reclaim_age we *could* possibly do an immediate relink \u0026 cleanup - but the `self.[next_]part_power` attributes are going to be all messed up.\n\nOperationally I know that b/c `cleanup` typically takes longer than `relinker` and the proxy has been using the `next_part_power` the whole time; that by the time you get to `audit` (even if you do it RIGHT after cleanup) you really don\u0027t care much about the data in the old hash-space.  So I think we should stick with \"quarantine for now\" (as always correct and \"old data\" cases necessary) and keep \"recent enough to repair\" as a kind of \"future enhancement if needed\"","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":799,"context_line":"                    f\"policy\u003d{policy.name}: \""},{"line_number":800,"context_line":"                    \"Skipping step - PPI in progress: \""},{"line_number":801,"context_line":"                    f\"part_power\u003d{ring.part_power} \" +"},{"line_number":802,"context_line":"                    f\"next_part_power\u003d{ring.next_part_power}\")"},{"line_number":803,"context_line":"                continue"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":"            num_policies +\u003d 1"}],"source_content_type":"text/x-python","patch_set":12,"id":"3ed27b1e_06259351","line":802,"updated":"2026-05-06 14:47:24.000000000","message":"some of these feel more like debug messages","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"}],"test/unit/cli/test_relinker.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":381,"context_line":"                               return_value\u003d[\u0027sda\u0027, \u0027sdb\u0027]):"},{"line_number":382,"context_line":"                mock_relinker.return_value.run.return_value \u003d 0"},{"line_number":383,"context_line":"                call_capture.attach_mock(mock_dp, \u0027drop_privileges\u0027)"},{"line_number":384,"context_line":"                call_capture.attach_mock(mock_relinker, \u0027run\u0027)"},{"line_number":385,"context_line":"                yield call_capture"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        # no user option"}],"source_content_type":"text/x-python","patch_set":12,"id":"13a7cbdd_d1a082b0","side":"PARENT","line":384,"updated":"2026-05-06 14:47:24.000000000","message":"*maybe* it\u0027d be better if this was `__init__` - I\u0027ve never seen this `attach_mock` thing before:\n\nhttps://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.attach_mock\n\nit\u0027s possible that\u0027s the test is trying to make assertions about the order in which `drop_privileges` and `Relinker(...).run()` are called?","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":388,"context_line":"        with do_mocks() as capture:"},{"line_number":389,"context_line":"            self.assertEqual(0, relinker.main([command, \u0027--workers\u0027, \u00270\u0027]))"},{"line_number":390,"context_line":"        self.assertEqual([mock.call.run(mock.ANY, mock.ANY, [\u0027sda\u0027, \u0027sdb\u0027],"},{"line_number":391,"context_line":"                                        do_cleanup\u003d(command \u003d\u003d \u0027cleanup\u0027))],"},{"line_number":392,"context_line":"                         capture.method_calls)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        # cli option --user"}],"source_content_type":"text/x-python","patch_set":12,"id":"2bdf34e3_51642039","side":"PARENT","line":391,"updated":"2026-05-06 14:47:24.000000000","message":"this is a annoying change - it *reads* like a signature change (which it is!) but the pointer `mock.call.run` *seems* like it\u0027s trying to say \"the signature of run changed!\" ... but that\u0027s not it:\n\n```\n    def run(self):\n```\n\nwhat *really* changed is `__init__`:\n\n\n```\n-    def __init__(self, conf, logger, device_list\u003dNone, do_cleanup\u003dFalse):\n+    def __init__(self, conf, logger, device_list\u003dNone, step\u003dStep.RELINK):\n```\n\nThe reason `run` shows up is because of the way this `do_mocks` \"aggregates\" all of the mocks onto a single mock object - and explicitly says \"whatever the class mock_relinker is called with, name that `run`\"","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":1000,"context_line":"                     exp_old_specs, exp_new_specs):"},{"line_number":1001,"context_line":"        # force the rehash to not happen during relink so that we can inspect"},{"line_number":1002,"context_line":"        # files in the new partition hash dir before they are cleaned up"},{"line_number":1003,"context_line":"        self._setup_object(lambda part: part \u003c 2 ** (PART_POWER - 1))"},{"line_number":1004,"context_line":"        self.rb.prepare_increase_partition_power()"},{"line_number":1005,"context_line":"        self._save_ring()"},{"line_number":1006,"context_line":"        self._do_link_test(\u0027relink\u0027, old_file_specs, new_file_specs, None,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9f1c175b_c1b0ea16","side":"PARENT","line":1003,"updated":"2026-05-06 14:47:24.000000000","message":"I think what this is saying was that `_relink_test` was only ever called w/ relink phase from PART_POWER\u003d\u003ePART_POWER+1","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":1001,"context_line":"        # force the rehash to not happen during relink so that we can inspect"},{"line_number":1002,"context_line":"        # files in the new partition hash dir before they are cleaned up"},{"line_number":1003,"context_line":"        self._setup_object(lambda part: part \u003c 2 ** (PART_POWER - 1))"},{"line_number":1004,"context_line":"        self.rb.prepare_increase_partition_power()"},{"line_number":1005,"context_line":"        self._save_ring()"},{"line_number":1006,"context_line":"        self._do_link_test(\u0027relink\u0027, old_file_specs, new_file_specs, None,"},{"line_number":1007,"context_line":"                           exp_old_specs, exp_new_specs)"}],"source_content_type":"text/x-python","patch_set":12,"id":"e3383bba_11ac9bf9","side":"PARENT","line":1004,"updated":"2026-05-06 14:47:24.000000000","message":"... and in fact this helper was clearly driving you INTO relink phase, so using the ring\u0027s pre-existing PART_POWER is equivalent for this setup.","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":2992,"context_line":"            fcntl.flock(f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)"},{"line_number":2993,"context_line":""},{"line_number":2994,"context_line":"    def _test_state_file(self, pol, expected_recon_data):"},{"line_number":2995,"context_line":"        datadir \u003d \u0027objects\u0027"},{"line_number":2996,"context_line":"        device_path \u003d os.path.join(self.devices, self.existing_device)"},{"line_number":2997,"context_line":"        datadir_path \u003d os.path.join(device_path, datadir)"},{"line_number":2998,"context_line":"        state_file \u003d os.path.join(device_path, \u0027relink.%s.json\u0027 % datadir)"}],"source_content_type":"text/x-python","patch_set":12,"id":"c616b53f_2ad6ae13","side":"PARENT","line":2995,"updated":"2026-05-06 14:47:24.000000000","message":"existing test wonky-ness if `diskfile.get_data_dir(pol) !\u003d datadir`","commit_id":"453e2356ba92fdefbe9e0886028d8414a1063684"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":391,"context_line":"        with do_mocks() as capture:"},{"line_number":392,"context_line":"            self.assertEqual(0, relinker.main([command, \u0027--workers\u0027, \u00270\u0027]))"},{"line_number":393,"context_line":"        self.assertEqual([mock.call.run(mock.ANY, mock.ANY, [\u0027sda\u0027, \u0027sdb\u0027],"},{"line_number":394,"context_line":"                                        step\u003dstep)],"},{"line_number":395,"context_line":"                         capture.method_calls)"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        # cli option --user"}],"source_content_type":"text/x-python","patch_set":12,"id":"819b8794_1566accf","line":394,"updated":"2026-05-06 14:47:24.000000000","message":"so this is testing \"routing\" as well - on the CLI if you pass in `Step.value` it will get dispatched to `Relinker(..., step\u003dstep).run()`","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":926,"context_line":"        def make_filenames(specs):"},{"line_number":927,"context_line":"            filenames \u003d []"},{"line_number":928,"context_line":"            for ext, ts_delta in specs:"},{"line_number":929,"context_line":"                ts \u003d utils.Timestamp(float(self.obj_ts) + ts_delta)"},{"line_number":930,"context_line":"                filename \u003d \u0027.\u0027.join([ts.internal, ext])"},{"line_number":931,"context_line":"                filenames.append(filename)"},{"line_number":932,"context_line":"            return filenames"}],"source_content_type":"text/x-python","patch_set":12,"id":"26455e9b_a13a2d0e","line":929,"updated":"2026-05-06 14:47:24.000000000","message":"maybe un-related?","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":1003,"context_line":"                     exp_old_specs, exp_new_specs):"},{"line_number":1004,"context_line":"        # force the rehash to not happen during relink so that we can inspect"},{"line_number":1005,"context_line":"        # files in the new partition hash dir before they are cleaned up"},{"line_number":1006,"context_line":"        part_power \u003d self.rb.part_power"},{"line_number":1007,"context_line":"        self._setup_object(lambda part: part \u003c 2 ** (part_power - 1))"},{"line_number":1008,"context_line":"        self.rb.prepare_increase_partition_power()"},{"line_number":1009,"context_line":"        self._save_ring()"}],"source_content_type":"text/x-python","patch_set":12,"id":"f7fa1a37_31c968b9","line":1006,"updated":"2026-05-06 14:47:24.000000000","message":"this helper becomes more flexible - seems reasonable.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":3015,"context_line":"            get_prefixed_swift_logger(self.logger,"},{"line_number":3016,"context_line":"                                      f\u0027[step\u003d{step.value}] \u0027),"},{"line_number":3017,"context_line":"            [self.existing_device], step)"},{"line_number":3018,"context_line":"        r.datadir \u003d datadir"},{"line_number":3019,"context_line":"        r.part_power \u003d PART_POWER + 1 if step \u003d\u003d relinker.Step.CLEANUP \\"},{"line_number":3020,"context_line":"            else PART_POWER"},{"line_number":3021,"context_line":"        r.next_part_power \u003d None if step \u003d\u003d relinker.Step.AUDIT \\"}],"source_content_type":"text/x-python","patch_set":12,"id":"69938dc7_f2bfdad9","line":3018,"updated":"2026-05-06 14:47:24.000000000","message":"how is `r.datadata \u003d None` *ever* ok!?\n\nwell it\u0027s fine if you don\u0027t use any of the audit_location_generator -  `self.datadir` is used by the pre device hook, and notably statefile tracking.\n\nIf you are calling into basically any other relinker method - it doesn\u0027t care about this attribute\u0027s value\n\nIt\u0027s typically initialized to:\n\n```\n        self.datadir \u003d diskfile.get_data_dir(policy)\n```\n\n... and honestly that might be a good case for it not even be *possible* for this helper to initialize it to a inconsistent value.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":3019,"context_line":"        r.part_power \u003d PART_POWER + 1 if step \u003d\u003d relinker.Step.CLEANUP \\"},{"line_number":3020,"context_line":"            else PART_POWER"},{"line_number":3021,"context_line":"        r.next_part_power \u003d None if step \u003d\u003d relinker.Step.AUDIT \\"},{"line_number":3022,"context_line":"            else PART_POWER + 1"},{"line_number":3023,"context_line":"        r.policy \u003d pol"},{"line_number":3024,"context_line":"        r.pid \u003d 1234  # for recon workers stats"},{"line_number":3025,"context_line":"        r.diskfile_mgr \u003d DiskFileRouter({"}],"source_content_type":"text/x-python","patch_set":12,"id":"db2ebc2b_4d6646d7","line":3022,"updated":"2026-05-06 14:47:24.000000000","message":"i like it; `next_part_power` becomes \"dynamic\" in the same way that `part_power` was already dynamic based on the `step` argument passed in - so the caller can say say what `step` they want and the helper sets up a realistic fake ring.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4243,"context_line":"    def test_is_ppi_ancestor_partition_zero(self):"},{"line_number":4244,"context_line":"        self.assertTrue(relinker._is_ppi_ancestor(0, 255))"},{"line_number":4245,"context_line":"        self.assertTrue(relinker._is_ppi_ancestor(0, 1, max_num_ppi\u003d1))"},{"line_number":4246,"context_line":"        self.assertFalse(relinker._is_ppi_ancestor(0, 2, max_num_ppi\u003d1))"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"    def test_is_ppi_ancestor_invalid(self):"},{"line_number":4249,"context_line":"        with self.assertRaises(ValueError):"}],"source_content_type":"text/x-python","patch_set":12,"id":"e1f3abbb_e652fbc3","line":4246,"updated":"2026-05-06 14:47:24.000000000","message":"I might enjoy\n\n(0, 0), (0, 1)\n(1, 2), (1, 3)\n\njust to really drive home that (2n, 2n+1) mapping/decensdent","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4269,"context_line":"            r.audit_location(self.existing_device, hp)"},{"line_number":4270,"context_line":""},{"line_number":4271,"context_line":"        qr.assert_not_called()"},{"line_number":4272,"context_line":"        self.assertEqual(r.stats[\u0027hash_dirs\u0027], 0)"},{"line_number":4273,"context_line":"        self.assertEqual(r.stats[\u0027removed\u0027], 0)"},{"line_number":4274,"context_line":"        self.assertEqual(r.stats[\u0027errors\u0027], 0)"},{"line_number":4275,"context_line":"        self.assertEqual(r.stats[\u0027warnings\u0027], 0)"}],"source_content_type":"text/x-python","patch_set":12,"id":"6d77cf82_8b8c1d8d","line":4272,"updated":"2026-05-06 14:47:24.000000000","message":"maybe worth a comment for `# matching hash_path isn\u0027t counted` (???)\n\n... with some sort of justification on \"b/c that\u0027s what process_location did\"","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4321,"context_line":"            r.audit_location(self.existing_device, hp_correct)"},{"line_number":4322,"context_line":"            r.audit_location(self.existing_device, hp_ancestor)"},{"line_number":4323,"context_line":""},{"line_number":4324,"context_line":"        self.assertEqual(r.stats[\u0027hash_dirs\u0027], 1)"},{"line_number":4325,"context_line":"        self.assertEqual(r.stats[\u0027removed\u0027], 1)"},{"line_number":4326,"context_line":"        self.assertEqual(r.stats[\u0027errors\u0027], 0)"},{"line_number":4327,"context_line":"        self.assertEqual(r.stats[\u0027warnings\u0027], 0)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5ee97ad3_3cd6188b","line":4324,"updated":"2026-05-06 14:47:24.000000000","message":"I was expecting this to be \"2\" \n\nthe reason it\u0027s confusing is because `process_location` *always* increments hash_path (but the caller *filters* calls based on matching paths)\n\nwere as `audit_location` is *always* called (no filter) - but sometimes \"skips\" matching hash_paths\n\n... I think this is a weird combination of \"consistent but not\"\n\n1) `process_location` is weird, it\u0027s essentially \"always relink maybe cleanup\"\n2) it should do the right thing even if we don\u0027t pre-filter during cleanup\n3) I think it\u0027s an omission that we don\u0027t count skipped and filtered hashes\n\n^ i\u0027m not sure what that says about the ideal amount of code to have as part of this change.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4326,"context_line":"        self.assertEqual(r.stats[\u0027errors\u0027], 0)"},{"line_number":4327,"context_line":"        self.assertEqual(r.stats[\u0027warnings\u0027], 0)"},{"line_number":4328,"context_line":""},{"line_number":4329,"context_line":"    def test_audit_location_quarantine_renamer_raises(self):"},{"line_number":4330,"context_line":"        r \u003d self._create_relinker(relinker.Step.AUDIT)"},{"line_number":4331,"context_line":"        hsh \u003d utils.hash_path(\u0027a\u0027, \u0027c\u0027, \u0027o\u0027)"},{"line_number":4332,"context_line":"        expected_part \u003d utils.get_partition_for_hash(hsh, PART_POWER)"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a95a57e_8c90a7e1","line":4329,"updated":"2026-05-06 14:47:24.000000000","message":"... when I see `test_\u003csomething\u003e_raises` I\u0027m looking for a `self.assertRaises`\n\nmaybe rename this:  `test_audit_location_ancestor_quarantine_error`\n\nwhen I set `test_\u003csomething\u003e_error` I\u0027m typically looking for \"setup some kind of error - and assert we do the right thing\" (which is often: log it but don\u0027t blow up) ... this test is setting up a \"ancestor quarantine\" that *blows up* and the \"correct behavior\" it calls out is 1) don\u0027t blow up 2) increment error\n\n... presumably there\u0027s also something interesting in `self.logger.get_lines_for_level(\u0027error\u0027)`\n\n```\n(Pdb) !self.logger.get_lines_for_level(\u0027error\u0027)\n[\u0027[step\u003daudit] Could not quarantine hash_path\u003d/mnt/tmp/tmp6lcnfn9d/node/sda1/objects/48/609/609099584d4a31526e840b664cb0dbbe: : \u0027]\n(Pdb) !self.logger.log_dict\ndefaultdict(\u003cclass \u0027list\u0027\u003e, {\u0027error\u0027: [((\u0027[step\u003daudit] Could not quarantine hash_path\u003d/mnt/tmp/tmp6lcnfn9d/node/sda1/objects/48/609/609099584d4a31526e840b664cb0dbbe: : \u0027,), {\u0027exc_info\u0027: (\u003cclass \u0027OSError\u0027\u003e, OSError(), \u003ctraceback object at 0x7fd83d0623c0\u003e), \u0027extra\u0027: {\u0027server\u0027: \u0027test\u0027, \u0027txn_id\u0027: None, \u0027client_ip\u0027: None}})]})\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4361,"context_line":"        r.conf[\u0027policies\u0027] \u003d POLICIES"},{"line_number":4362,"context_line":"        r.conf[\u0027swift_dir\u0027] \u003d self.testdir"},{"line_number":4363,"context_line":""},{"line_number":4364,"context_line":"        self.assertEqual(r.run(), relinker.EXIT_NO_APPLICABLE_POLICY)"},{"line_number":4365,"context_line":""},{"line_number":4366,"context_line":"    def test_audit_run_hash_ok(self):"},{"line_number":4367,"context_line":"        self._setup_object(lambda part: part \u003c 2 ** (PART_POWER - 1))"}],"source_content_type":"text/x-python","patch_set":12,"id":"712c8c5c_07d86190","line":4364,"updated":"2026-05-06 14:47:24.000000000","message":"I really like how terse these new `test_\u003cphase\u003e_blocked_\u003cring_state` tests are - it would be nice to extend them to cover other conditions.\n\nI think the way this test is setup it can only be a negative test i.e. `run() \u003d\u003e EXIT_NO_APPLICABLE_POLICY` the \"positive\"/happy-path side would have to show some work of some kind and mostly we do that with `relinker.main()`\n\nmaybe these would look better unified with other existing tests:\n\n```\n    def test_relink_no_applicable_policy(self):\n        # NB do not prepare part power increase\n        self._save_ring()\n        with self._mock_relinker():\n            self.assertEqual(2, relinker.main([\n                \u0027relink\u0027,\n                \u0027--swift-dir\u0027, self.testdir,\n                \u0027--devices\u0027, self.devices,\n            ]))\n        self.assertEqual(\n            self.logger.get_lines_for_level(\u0027warning\u0027),\n            [\u0027[step\u003drelink] policy\u003dplatinum: Skipping step - \u0027 +\n             \u0027next_part_power !\u003d part_power + 1 \u0027 +\n             \u0027part_power\u003d8 next_part_power\u003dNone\u0027,\n             \u0027[step\u003drelink] No policy found to increase the partition power.\u0027])\n        self.assertEqual([], self.logger.get_lines_for_level(\u0027error\u0027))\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4364,"context_line":"        self.assertEqual(r.run(), relinker.EXIT_NO_APPLICABLE_POLICY)"},{"line_number":4365,"context_line":""},{"line_number":4366,"context_line":"    def test_audit_run_hash_ok(self):"},{"line_number":4367,"context_line":"        self._setup_object(lambda part: part \u003c 2 ** (PART_POWER - 1))"},{"line_number":4368,"context_line":"        self._save_ring()"},{"line_number":4369,"context_line":""},{"line_number":4370,"context_line":"        with self._mock_relinker():"}],"source_content_type":"text/x-python","patch_set":12,"id":"2abd085c_13bad024","line":4367,"updated":"2026-05-06 14:47:24.000000000","message":"if I try to remove this condition:\n\n```\n-        self._setup_object(lambda part: part \u003c 2 ** (PART_POWER - 1))\n+        self._setup_object()\n```\n\nthe test fails like:\n\n```\n\u003e       with open(state_file, \u0027rt\u0027) as f:\nE       FileNotFoundError: [Errno 2] No such file or directory: \u0027/mnt/tmp/tmp894czd2l/node/sda1/relink.objects.json\u0027\n```\n\n^ but only *sometimes*\n\nthe \"reason\" is b/c audit does \"nothing\" if the hash is in the \"uppper\" part of the hash space.","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4450,"context_line":""},{"line_number":4451,"context_line":"        # Ensures that placing `hash_fgn` in `part_qr` indeed"},{"line_number":4452,"context_line":"        # makes it foreign"},{"line_number":4453,"context_line":"        next_part_fgn \u003d utils.get_partition_for_hash(hash_fgn, PART_POWER + 1)"},{"line_number":4454,"context_line":"        self.assertFalse(relinker._is_ppi_ancestor(part_qr,"},{"line_number":4455,"context_line":"                                                   next_part_fgn,"},{"line_number":4456,"context_line":"                                                   max_num_ppi\u003d1))"}],"source_content_type":"text/x-python","patch_set":12,"id":"87866afd_172c4432","line":4453,"updated":"2026-05-06 14:47:24.000000000","message":"For me, with this diff the tests still pass:\n\n```\n-        next_part_fgn \u003d utils.get_partition_for_hash(hash_fgn, PART_POWER + 1)\n+        next_part_fgn \u003d utils.get_partition_for_hash(hash_fgn, PART_POWER)\n```\n\nThe point is the hash for `o2` belongs *somewhere else* i.e. NOT in the part for the hash of `o1`\n\n```\n(Pdb) !part_qr\n103\n(Pdb) !next_part_fgn\n158\n```\n\nI think having `part + 1` is \"also correct\" but having it more closely \"mirror\" the previous setup for `part_qr` might drive home the real magic comes next:\n\n```\nself._create_object(policy, part_qr, hash_fgn)\n```\n\nthis puts `hash_fgn` into `part_qr` which could *never* happen - that hash should be in `part_fgn` (or `next_part_fgn`)\n\n\nConsider maybe:\n\n```\n        # Object that must be quarantined\n        hash_qr \u003d utils.hash_path(\u0027a\u0027, \u0027c\u0027, \u0027o1\u0027)\n        part_qr \u003d utils.get_partition_for_hash(hash_qr, PART_POWER)\n        _, fname_qr, _ \u003d self._create_object(\n            policy, part_qr, hash_qr)\n\n        # foreign object that must remain in place\n        hash_fgn \u003d utils.hash_path(\u0027a\u0027, \u0027c\u0027, \u0027o2\u0027)\n        part_fgn \u003d utils.get_partition_for_hash(hash_fgn, PART_POWER)\n        # it\u0027s \"foreign\" becase we put it in o1\u0027s part\n        objdir_fgn, fname_fgn, _ \u003d self._create_object(\n            policy, part_qr, hash_fgn)\n\n        # sanity: placing `hash_fgn` in `part_qr` makes it \"foreign\"\n        self.assertNotEqual(part_qr, part_fgn)\n        self.assertFalse(relinker._is_ppi_ancestor(part_qr, part_fgn))\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4463,"context_line":"        self.rb.prepare_increase_partition_power()"},{"line_number":4464,"context_line":"        self.rb.increase_partition_power()"},{"line_number":4465,"context_line":"        self.rb.finish_increase_partition_power()"},{"line_number":4466,"context_line":"        self._save_ring()"},{"line_number":4467,"context_line":""},{"line_number":4468,"context_line":"        with self._mock_relinker():"},{"line_number":4469,"context_line":"            self.assertEqual(0, relinker.main(["}],"source_content_type":"text/x-python","patch_set":12,"id":"7e5a7985_2ae33db1","line":4466,"updated":"2026-05-06 14:47:24.000000000","message":"for funzies (after PPI) you might also try doing another:\n\n```\n        part_correct \u003d utils.get_partition_for_hash(\n            hash_fgn, self.rb.part_power)\n        _ \u003d self._create_object(policy, part_correct, hash_fgn)\n```\n\nor something to capture: same \"foreign\" hash in the *correct* part is ok/noop","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4498,"context_line":"                               relinker.RECON_RELINKER_FILE), \u0027rt\u0027) as f:"},{"line_number":4499,"context_line":"            recon_state \u003d json.load(f)"},{"line_number":4500,"context_line":"            stats \u003d recon_state[\"devices\"][self.existing_device][\"stats\"]"},{"line_number":4501,"context_line":"            self.assertEqual(stats[\"hash_dirs\"], 2)"},{"line_number":4502,"context_line":"            self.assertEqual(stats[\"removed\"], 1)"},{"line_number":4503,"context_line":"            self.assertEqual(stats[\"errors\"], 0)"},{"line_number":4504,"context_line":"            self.assertEqual(stats[\"warnings\"], 1)"}],"source_content_type":"text/x-python","patch_set":12,"id":"df6974a8_71686359","line":4501,"updated":"2026-05-06 14:47:24.000000000","message":"i\u0027m not sure the conditional increment of `hash_dirs` was intentional - maybe the best we could do now is add a counter for `skipped_hash_dirs` or something","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4503,"context_line":"            self.assertEqual(stats[\"errors\"], 0)"},{"line_number":4504,"context_line":"            self.assertEqual(stats[\"warnings\"], 1)"},{"line_number":4505,"context_line":""},{"line_number":4506,"context_line":"    def test_prev_part_power_mismatch_raises(self):"},{"line_number":4507,"context_line":"        r \u003d self._create_relinker(relinker.Step.CLEANUP)"},{"line_number":4508,"context_line":"        r.states \u003d {"},{"line_number":4509,"context_line":"            \"prev_part_power\": PART_POWER - 2,"}],"source_content_type":"text/x-python","patch_set":12,"id":"23b120a4_4a9f5e9d","line":4506,"updated":"2026-05-06 14:47:24.000000000","message":"ah, here we go - `test_\u003csomething\u003e_raises` ...\n\n... and it does have a `self.assertRaises`","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4504,"context_line":"            self.assertEqual(stats[\"warnings\"], 1)"},{"line_number":4505,"context_line":""},{"line_number":4506,"context_line":"    def test_prev_part_power_mismatch_raises(self):"},{"line_number":4507,"context_line":"        r \u003d self._create_relinker(relinker.Step.CLEANUP)"},{"line_number":4508,"context_line":"        r.states \u003d {"},{"line_number":4509,"context_line":"            \"prev_part_power\": PART_POWER - 2,"},{"line_number":4510,"context_line":"            \"part_power\": PART_POWER,"}],"source_content_type":"text/x-python","patch_set":12,"id":"a177ab26_6b5f6f26","line":4507,"updated":"2026-05-06 14:47:24.000000000","message":"step\u003dcleanup creates a relinker with `part_power \u003d PART_POWER + 1`\n\n```\n(Pdb) !r.part_power\n9\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4506,"context_line":"    def test_prev_part_power_mismatch_raises(self):"},{"line_number":4507,"context_line":"        r \u003d self._create_relinker(relinker.Step.CLEANUP)"},{"line_number":4508,"context_line":"        r.states \u003d {"},{"line_number":4509,"context_line":"            \"prev_part_power\": PART_POWER - 2,"},{"line_number":4510,"context_line":"            \"part_power\": PART_POWER,"},{"line_number":4511,"context_line":"            \"next_part_power\": PART_POWER,"},{"line_number":4512,"context_line":"            \"state\": {},"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff5e978a_83b229d2","line":4509,"updated":"2026-05-06 14:47:24.000000000","message":"oic, and the problem here is `partitions_filter` expected `prev_part_power \u003d PART_POWER` (i.e. `r.part_power - 1`)","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4508,"context_line":"        r.states \u003d {"},{"line_number":4509,"context_line":"            \"prev_part_power\": PART_POWER - 2,"},{"line_number":4510,"context_line":"            \"part_power\": PART_POWER,"},{"line_number":4511,"context_line":"            \"next_part_power\": PART_POWER,"},{"line_number":4512,"context_line":"            \"state\": {},"},{"line_number":4513,"context_line":"        }"},{"line_number":4514,"context_line":"        with self.assertRaises(RuntimeError) as cm:"}],"source_content_type":"text/x-python","patch_set":12,"id":"2ba049a5_117cf52c","line":4511,"updated":"2026-05-06 14:47:24.000000000","message":"it\u0027s not super clear to me what if any state the `relinker cleanup` would expect here\n\nwhen `relinker relink` ran `part_power\u003d8` and `next_part_power\u003d9`\nwhen `relinker cleanup` started `part_power\u003d9` and `next_part_power\u003d9`\n\n... probably in no case would `next_part_powever\u003d8`; so maybe we\u0027re trying to show \"none of this state file non-sense matters and shouldn\u0027t drive the relinker *behavior* because all the state tracking is just \"best effort\" anyway\"?","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4546,"context_line":"                                  relinker.STATE_FILE.format(datadir\u003ddatadir))"},{"line_number":4547,"context_line":"        with open(state_file, \u0027rt\u0027) as f:"},{"line_number":4548,"context_line":"            state \u003d json.load(f)[\"state\"]"},{"line_number":4549,"context_line":"            self.assertTrue(state[str(self.part)])"},{"line_number":4550,"context_line":""},{"line_number":4551,"context_line":"        self._relink_test(((\u0027data\u0027, 0),),"},{"line_number":4552,"context_line":"                          None,"}],"source_content_type":"text/x-python","patch_set":12,"id":"0178b10c_89f3ade6","line":4549,"updated":"2026-05-06 14:47:24.000000000","message":"should we try to say something about the *part_power values in the status dict?\n\n```\n(Pdb) !status\n{\u0027part_power\u0027: 9, \u0027next_part_power\u0027: None, \u0027state\u0027: {\u0027204\u0027: True}}\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fff178b9b476191e801a912e33e64b05f99a0dee","unresolved":true,"context_lines":[{"line_number":4551,"context_line":"        self._relink_test(((\u0027data\u0027, 0),),"},{"line_number":4552,"context_line":"                          None,"},{"line_number":4553,"context_line":"                          ((\u0027data\u0027, 0),),"},{"line_number":4554,"context_line":"                          ((\u0027data\u0027, 0),))"},{"line_number":4555,"context_line":""},{"line_number":4556,"context_line":""},{"line_number":4557,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"8a9539dd_9c432c56","line":4554,"updated":"2026-05-06 14:47:24.000000000","message":"what is the point of this?  inside of here under `_setup_object` we\u0027re doing a `shutil.rmtree(objects_dir, ignore_errors\u003dTrue)`\n\nas best I can tell the data here is completely disconnected from the object we just successfully audited:\n\n```\n+ {\u0027next_part_power\u0027: 10, \u0027part_power\u0027: 9, \u0027state\u0027: {\u0027165\u0027: True}}\n```","commit_id":"473e66886d6199d6fe673ce34c7154e80f412b16"}]}