)]}'
{"swift/cli/relinker.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"59b41f87b6929c5d147aab6a376d9c3737150a2b","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    # Remove all non partitions first (eg: auditor_status_ALL.json)"},{"line_number":78,"context_line":"    partitions \u003d [p for p in partitions if p.isdigit()]"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    if not (step \u003d\u003d STEP_CLEANUP and part_power \u003d\u003d next_part_power):"},{"line_number":81,"context_line":"        # This is not a cleanup after cancel, partitions in the upper half are"},{"line_number":82,"context_line":"        # new partitions, there is nothing to relink/cleanup from there"},{"line_number":83,"context_line":"        partitions \u003d [p for p in partitions"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f15bb33_048805d6","line":80,"range":{"start_line":80,"start_character":4,"end_line":80,"end_character":68},"updated":"2021-01-15 19:31:28.000000000","message":"Off-topic: I think cleanup can only happen when part_power \u003d\u003d next_part_power -- if it\u0027s at the end of an increase, they\u0027ll both be old_part_power+1, while if the increase was canceled, they\u0027ll both be old_part_power.","commit_id":"24f422e0b3b8b480a31b722bc6cf2b9fde9ed3a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"59b41f87b6929c5d147aab6a376d9c3737150a2b","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        partitions \u003d [p for p in partitions"},{"line_number":84,"context_line":"                      if int(p) \u003c 2 ** next_part_power / 2]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    # Format: { \u0027part\u0027: [relinked, cleaned] }"},{"line_number":87,"context_line":"    if states[\"state\"]:"},{"line_number":88,"context_line":"        missing \u003d list(set(partitions) - set(states[\"state\"].keys()))"},{"line_number":89,"context_line":"        if missing:"}],"source_content_type":"text/x-python","patch_set":2,"id":"77f0e1de_bc4a8345","line":86,"range":{"start_line":86,"start_character":4,"end_line":86,"end_character":45},"updated":"2021-01-15 19:31:28.000000000","message":"Needs updating.","commit_id":"24f422e0b3b8b480a31b722bc6cf2b9fde9ed3a4"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"b7a02d2ffc65d3a526a292e2fbf4ad7802d21490","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            # cleaned. Just update the state file."},{"line_number":94,"context_line":"            for part in missing:"},{"line_number":95,"context_line":"                states[\"state\"][part] \u003d (step \u003d\u003d STEP_RELINK)"},{"line_number":96,"context_line":"        if step \u003d\u003d STEP_RELINK:"},{"line_number":97,"context_line":"            partitions \u003d [str(p) for p, r in states[\"state\"].items() if not r]"},{"line_number":98,"context_line":"        elif step \u003d\u003d STEP_CLEANUP:"},{"line_number":99,"context_line":"            partitions \u003d [str(p) for p, c in states[\"state\"].items() if not c]"}],"source_content_type":"text/x-python","patch_set":2,"id":"45071345_a9bd3d39","line":96,"updated":"2021-01-14 03:16:45.000000000","message":"In both branchs of the \"if\", the code is the same. It can be simplified to:\n    if step in (STEP_RELINK, STEP_CLEANUP):\n\nBut actually the condition is unecessary given there is no other type of steps.","commit_id":"24f422e0b3b8b480a31b722bc6cf2b9fde9ed3a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"59b41f87b6929c5d147aab6a376d9c3737150a2b","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            # cleaned. Just update the state file."},{"line_number":94,"context_line":"            for part in missing:"},{"line_number":95,"context_line":"                states[\"state\"][part] \u003d (step \u003d\u003d STEP_RELINK)"},{"line_number":96,"context_line":"        if step \u003d\u003d STEP_RELINK:"},{"line_number":97,"context_line":"            partitions \u003d [str(p) for p, r in states[\"state\"].items() if not r]"},{"line_number":98,"context_line":"        elif step \u003d\u003d STEP_CLEANUP:"},{"line_number":99,"context_line":"            partitions \u003d [str(p) for p, c in states[\"state\"].items() if not c]"}],"source_content_type":"text/x-python","patch_set":2,"id":"461b1a3e_3c3c11ad","line":96,"in_reply_to":"45071345_a9bd3d39","updated":"2021-01-15 19:31:28.000000000","message":"Done","commit_id":"24f422e0b3b8b480a31b722bc6cf2b9fde9ed3a4"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"268c4404bc7f3f92c4cd590b8468fd60b56271d4","unresolved":true,"context_lines":[{"line_number":85,"context_line":"        # nothing to relink there"},{"line_number":86,"context_line":"        partitions \u003d [part for part in partitions"},{"line_number":87,"context_line":"                      if int(part) \u003c 2 ** part_power]"},{"line_number":88,"context_line":"    elif \"prev_part_power\" in states:"},{"line_number":89,"context_line":"        # All partitions in the upper half are new partitions and there is"},{"line_number":90,"context_line":"        # nothing to relink there"},{"line_number":91,"context_line":"        partitions \u003d [part for part in partitions"}],"source_content_type":"text/x-python","patch_set":5,"id":"35c48be7_7a47245b","line":88,"updated":"2021-01-22 13:35:08.000000000","message":"So this is \"cleanup after relink\". The implicit else is \"cleanup for cancel\" where we cleanup everything. Looks fine.","commit_id":"0c84358f2c8df50259bd6db7bfe106db689335cb"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"268c4404bc7f3f92c4cd590b8468fd60b56271d4","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                      if int(part) \u003c 2 ** part_power]"},{"line_number":88,"context_line":"    elif \"prev_part_power\" in states:"},{"line_number":89,"context_line":"        # All partitions in the upper half are new partitions and there is"},{"line_number":90,"context_line":"        # nothing to relink there"},{"line_number":91,"context_line":"        partitions \u003d [part for part in partitions"},{"line_number":92,"context_line":"                      if int(part) \u003c 2 ** states[\"prev_part_power\"]]"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"69e01bb4_578b4bc2","line":90,"range":{"start_line":90,"start_character":21,"end_line":90,"end_character":27},"updated":"2021-01-22 13:35:08.000000000","message":"I think you meant cleanup here","commit_id":"0c84358f2c8df50259bd6db7bfe106db689335cb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e6516e19db036017eceb890d602349e3ee39aae4","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                      if int(part) \u003c 2 ** part_power]"},{"line_number":88,"context_line":"    elif \"prev_part_power\" in states:"},{"line_number":89,"context_line":"        # All partitions in the upper half are new partitions and there is"},{"line_number":90,"context_line":"        # nothing to relink there"},{"line_number":91,"context_line":"        partitions \u003d [part for part in partitions"},{"line_number":92,"context_line":"                      if int(part) \u003c 2 ** states[\"prev_part_power\"]]"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"72d70d0c_92cd78d5","line":90,"range":{"start_line":90,"start_character":21,"end_line":90,"end_character":27},"in_reply_to":"69e01bb4_578b4bc2","updated":"2021-01-26 18:23:25.000000000","message":"Good catch!","commit_id":"0c84358f2c8df50259bd6db7bfe106db689335cb"}],"test/unit/cli/test_relinker.py":[{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"b7a02d2ffc65d3a526a292e2fbf4ad7802d21490","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        relinker.cleanup(self.testdir, self.devices, True)"},{"line_number":190,"context_line":"        with open(state_file, \u0027rt\u0027) as f:"},{"line_number":191,"context_line":"            # NB: part_power/next_part_power tuple changed, so state was reset"},{"line_number":192,"context_line":"            # (though we can\u0027t really tell at this point...)"},{"line_number":193,"context_line":"            self.assertEqual(json.load(f), {"},{"line_number":194,"context_line":"                \"part_power\": PART_POWER + 1,"},{"line_number":195,"context_line":"                \"next_part_power\": PART_POWER + 1,"}],"source_content_type":"text/x-python","patch_set":2,"id":"0d57d9dd_c5ab9779","line":192,"updated":"2021-01-14 03:16:45.000000000","message":"I feel it\u0027s an important change of the patchset. Could you open (and keep it open) the file, then run cleanup() and ensure the file at the same path now have a different inode? Something like:\n    with open(state_file, \u0027rt\u0027) as f:\n        inode_orig \u003d os.stat(state_file).st_ino\n        relinker.cleanup(self.testdir, self.devices, True)\n        inode_new \u003d os.stat(state_file).st_ino\n        self.assertNotEqual(inode_orig, inode_new)\n    with open(state_file, \u0027rt\u0027) as f:\n        # normal assert on json\n\nAs long as the file stays open, its inode won\u0027t be released so the new state file will have to have a new inode.","commit_id":"24f422e0b3b8b480a31b722bc6cf2b9fde9ed3a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"59b41f87b6929c5d147aab6a376d9c3737150a2b","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        relinker.cleanup(self.testdir, self.devices, True)"},{"line_number":190,"context_line":"        with open(state_file, \u0027rt\u0027) as f:"},{"line_number":191,"context_line":"            # NB: part_power/next_part_power tuple changed, so state was reset"},{"line_number":192,"context_line":"            # (though we can\u0027t really tell at this point...)"},{"line_number":193,"context_line":"            self.assertEqual(json.load(f), {"},{"line_number":194,"context_line":"                \"part_power\": PART_POWER + 1,"},{"line_number":195,"context_line":"                \"next_part_power\": PART_POWER + 1,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7232a580_5f5e56ad","line":192,"in_reply_to":"0d57d9dd_c5ab9779","updated":"2021-01-15 19:31:28.000000000","message":"Done","commit_id":"24f422e0b3b8b480a31b722bc6cf2b9fde9ed3a4"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"b56dfd57dfe10b74ceff214f84a30b4f9ddb2e89","unresolved":true,"context_lines":[{"line_number":326,"context_line":"            \"state\": {}})"},{"line_number":327,"context_line":"        os.close(locks[0])  # Release the lock"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        self.assertEqual([\u0027312\u0027, \u0027227\u0027, \u002796\u0027],"},{"line_number":330,"context_line":"                         call_partition_filter(PART_POWER + 1, PART_POWER + 1,"},{"line_number":331,"context_line":"                                               [\u002796\u0027, \u0027227\u0027, \u0027312\u0027]))"},{"line_number":332,"context_line":"        # Ack partition 227"}],"source_content_type":"text/x-python","patch_set":4,"id":"80c1c1dc_05a9f217","line":329,"updated":"2021-01-20 21:12:39.000000000","message":"It seems to be a major improvement loss here. Partition 312 does not need to be cleaned because it\u0027s a new partition (cleaning only necessary in case of relink cancel). It means in the current proposal, it doubles the number of partitions to scan to do the cleanup.","commit_id":"73d63dffc9c332d9e3654fe9110e1b64fc77a3f4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c10a70ef0092ce23d36d3cd9f4c74ac1e0457741","unresolved":true,"context_lines":[{"line_number":326,"context_line":"            \"state\": {}})"},{"line_number":327,"context_line":"        os.close(locks[0])  # Release the lock"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        self.assertEqual([\u0027312\u0027, \u0027227\u0027, \u002796\u0027],"},{"line_number":330,"context_line":"                         call_partition_filter(PART_POWER + 1, PART_POWER + 1,"},{"line_number":331,"context_line":"                                               [\u002796\u0027, \u0027227\u0027, \u0027312\u0027]))"},{"line_number":332,"context_line":"        # Ack partition 227"}],"source_content_type":"text/x-python","patch_set":4,"id":"69b72190_77199584","line":329,"in_reply_to":"80c1c1dc_05a9f217","updated":"2021-01-20 21:57:52.000000000","message":"`swift-object-relinker cleanup` doesn\u0027t know what the previous partition power was, though -- are we running this because we were at PART_POWER and now we\u0027re at PART_POWER + 1? Or because we were at PART_POWER + 1, did some amount of relinking, then decided to cancel the increase?\n\nMaybe I could piece that together from the state file that gets discarded... 🤔","commit_id":"73d63dffc9c332d9e3654fe9110e1b64fc77a3f4"}]}