)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"869badeb9ebb9e564a69965fa4c3cddafb298c20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"28973bf2_89dd4a14","updated":"2022-11-28 11:13:57.000000000","message":"I\u0027m happy to rename the method if we can agree 😊\n\nOther ideas:\n\n  _conflicts_with_existing\n  _blocked_by_existing\n","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"6d2e097e44413628a2a587bd5f9ecb84387957f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d40e5a9c_6b40f633","updated":"2022-11-18 23:22:13.000000000","message":"LGTM. Mat, Could you please take a look at this patch again?","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2385cac3_71785522","updated":"2022-11-30 20:04:48.000000000","message":"So, assuming we carry this -- I guess we\u0027d need to be careful to only pass the new option on new-enough swift? Otherwise, we exit 2 with some message about unrecognized arguments. Would it be better/simpler/cheaper to just treat EOFError as a negative response?","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"e507e945c311a3b181fab5e296fad458b886aea6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3b7b63de_8fb1c8c5","updated":"2022-11-28 03:50:42.000000000","message":"Yeah ok. I agree that the true or false returning in a method like this needs to read nice based of the function name.\n\nrequired_disallowed_deletes, is correct, but somehow doesn\u0027t sound right to me.. but I\u0027m not sure I can think of anything better.. maybe: are_deletes_disabled, is_deletes_disabled or deletes_disabled?\n\nHappy to be over turned and change to a +2. Cause maybe I\u0027m just Nit picking here. ","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"57f4ebadc25db4f5304e00408f51cc4513288057","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e0176f85_6b04d6a5","updated":"2022-11-18 14:18:47.000000000","message":"thanks for the reviews","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"737458dc7861ad08c83694f9f71ebce0cda343bf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b7940bdd_3aec22de","updated":"2022-12-01 11:04:56.000000000","message":"I liked Tim\u0027s proposal in https://review.opendev.org/c/openstack/swift/+/866209 so I have pivoted to use that here (I wanted to add unit tests, but not for ringbuilder too, so just applied the idea in s-m-s-r).","commit_id":"eeaf423fdb26ec6d6880b11c47171b215dba1afc"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"00148034096a93abac4ba9d7bde7da40a3d03e8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ae79053b_ec532b9a","updated":"2022-12-01 16:57:17.000000000","message":"LOOK AT THAT TEST RATIO!!!","commit_id":"07b909c260937916843dfe797b95f929bbedbb60"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"db7a4eaaf9b7dc11ee1509ab8f7d424764c38cf6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b0a43a57_cd7cc32e","updated":"2022-12-01 16:17:18.000000000","message":"Love it, though I might be biased ;-)","commit_id":"07b909c260937916843dfe797b95f929bbedbb60"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"697f58343fe4349a5ae91b5b125be09a2a56a40a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4982e23c_ef38a1e6","updated":"2022-12-01 18:06:11.000000000","message":"recheck\n\nLooks like some flakey tempest tests involving non-idempotent APIs.","commit_id":"07b909c260937916843dfe797b95f929bbedbb60"}],"swift/cli/manage_shard_ranges.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"63cb8447f6e122028181b54dcbbf748df2a3db9b","unresolved":true,"context_lines":[{"line_number":515,"context_line":"    if args.no_delete and shard_ranges:"},{"line_number":516,"context_line":"        print(\u0027WARNING: %d existing shard ranges found.\u0027 % len(shard_ranges))"},{"line_number":517,"context_line":"        print(\u0027No changes applied.\u0027)"},{"line_number":518,"context_line":"        return False"},{"line_number":519,"context_line":"    return True"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"11dbb551_6ad55c4d","line":518,"updated":"2022-11-17 00:29:37.000000000","message":"I would return True here, and return False at line 519, since this function name is \"_check_no_delete\".","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"57f4ebadc25db4f5304e00408f51cc4513288057","unresolved":false,"context_lines":[{"line_number":515,"context_line":"    if args.no_delete and shard_ranges:"},{"line_number":516,"context_line":"        print(\u0027WARNING: %d existing shard ranges found.\u0027 % len(shard_ranges))"},{"line_number":517,"context_line":"        print(\u0027No changes applied.\u0027)"},{"line_number":518,"context_line":"        return False"},{"line_number":519,"context_line":"    return True"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8fbb8dc0_70075fca","line":518,"in_reply_to":"11dbb551_6ad55c4d","updated":"2022-11-18 14:18:47.000000000","message":"I tend to think of check_* methods as meaning \"check something is ok\" so in this case check that the --no-delete arg conditions are ok. I\u0027ll rename the method to have a negative sense.","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"63cb8447f6e122028181b54dcbbf748df2a3db9b","unresolved":true,"context_lines":[{"line_number":520,"context_line":""},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"def replace_shard_ranges(broker, args):"},{"line_number":523,"context_line":"    if _check_no_delete(broker, args):"},{"line_number":524,"context_line":"        shard_data \u003d _load_and_validate_shard_data(args)"},{"line_number":525,"context_line":"        return _replace_shard_ranges(broker, args, shard_data)"},{"line_number":526,"context_line":"    return EXIT_USER_QUIT"}],"source_content_type":"text/x-python","patch_set":1,"id":"6fe123d7_241cc3fe","line":523,"updated":"2022-11-17 00:29:37.000000000","message":"nit: I would advocate for using below flow, to reduce overall needed indentations in future.\n\n  if _check_no_delete(broker, args):\n      return EXIT_USER_QUIT\n  shard_data \u003d _load_and_validate_shard_data(args)\n  return _replace_shard_ranges(broker, args, shard_data)","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"57f4ebadc25db4f5304e00408f51cc4513288057","unresolved":false,"context_lines":[{"line_number":520,"context_line":""},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"def replace_shard_ranges(broker, args):"},{"line_number":523,"context_line":"    if _check_no_delete(broker, args):"},{"line_number":524,"context_line":"        shard_data \u003d _load_and_validate_shard_data(args)"},{"line_number":525,"context_line":"        return _replace_shard_ranges(broker, args, shard_data)"},{"line_number":526,"context_line":"    return EXIT_USER_QUIT"}],"source_content_type":"text/x-python","patch_set":1,"id":"e5c0b168_6664d415","line":523,"in_reply_to":"6fe123d7_241cc3fe","updated":"2022-11-18 14:18:47.000000000","message":"Done","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"63cb8447f6e122028181b54dcbbf748df2a3db9b","unresolved":true,"context_lines":[{"line_number":527,"context_line":""},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"def find_replace_shard_ranges(broker, args):"},{"line_number":530,"context_line":"    if _check_no_delete(broker, args):"},{"line_number":531,"context_line":"        shard_data, delta_t \u003d _find_ranges(broker, args, sys.stdout)"},{"line_number":532,"context_line":"        # Since we\u0027re trying to one-shot this, and the previous step probably"},{"line_number":533,"context_line":"        # took a while, make the timeout for writing *at least* that long"}],"source_content_type":"text/x-python","patch_set":1,"id":"c5ac020b_592f1bf9","line":530,"updated":"2022-11-17 00:29:37.000000000","message":"I would suggest the flow change here too.","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"57f4ebadc25db4f5304e00408f51cc4513288057","unresolved":false,"context_lines":[{"line_number":527,"context_line":""},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"def find_replace_shard_ranges(broker, args):"},{"line_number":530,"context_line":"    if _check_no_delete(broker, args):"},{"line_number":531,"context_line":"        shard_data, delta_t \u003d _find_ranges(broker, args, sys.stdout)"},{"line_number":532,"context_line":"        # Since we\u0027re trying to one-shot this, and the previous step probably"},{"line_number":533,"context_line":"        # took a while, make the timeout for writing *at least* that long"}],"source_content_type":"text/x-python","patch_set":1,"id":"311759ca_01b4b7f2","line":530,"in_reply_to":"c5ac020b_592f1bf9","updated":"2022-11-18 14:18:47.000000000","message":"Done","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"027fe9751bc65d57a7dd608f5989af35e03dec91","unresolved":true,"context_lines":[{"line_number":944,"context_line":""},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"def _add_force_delete_args(parser):"},{"line_number":947,"context_line":"    group \u003d parser.add_mutually_exclusive_group()"},{"line_number":948,"context_line":"    group.add_argument("},{"line_number":949,"context_line":"        \u0027--force\u0027, \u0027-f\u0027, action\u003d\u0027store_true\u0027, default\u003dFalse,"},{"line_number":950,"context_line":"        help\u003d\u0027Delete existing shard ranges; no questions asked.\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8d73175a_96b9fb33","line":947,"updated":"2022-11-14 04:28:34.000000000","message":"Oh cool, mutually exclusive group, didn\u0027t know that was a thing in the parser. Nice!","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"869badeb9ebb9e564a69965fa4c3cddafb298c20","unresolved":false,"context_lines":[{"line_number":944,"context_line":""},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"def _add_force_delete_args(parser):"},{"line_number":947,"context_line":"    group \u003d parser.add_mutually_exclusive_group()"},{"line_number":948,"context_line":"    group.add_argument("},{"line_number":949,"context_line":"        \u0027--force\u0027, \u0027-f\u0027, action\u003d\u0027store_true\u0027, default\u003dFalse,"},{"line_number":950,"context_line":"        help\u003d\u0027Delete existing shard ranges; no questions asked.\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f11eaa36_99e6db84","line":947,"in_reply_to":"8d73175a_96b9fb33","updated":"2022-11-28 11:13:57.000000000","message":"Ack","commit_id":"dec073300ad08c0cf9ba50dfccb76ca7bcc93f63"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":false,"context_lines":[{"line_number":395,"context_line":"        print(\"No shard ranges found to delete.\")"},{"line_number":396,"context_line":"        return EXIT_SUCCESS"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    while not args.force:"},{"line_number":399,"context_line":"        print(\u0027This will delete existing %d shard ranges.\u0027 % len(shard_ranges))"},{"line_number":400,"context_line":"        if broker.get_db_state() !\u003d UNSHARDED:"},{"line_number":401,"context_line":"            print(\u0027WARNING: Be very cautious about deleting existing shard \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"8277b7f2_a646285d","line":398,"updated":"2022-11-30 20:04:48.000000000","message":"It felt a little weird that we don\u0027t check for no_delete here (since it\u0027s kind-of-but-not-exactly an inverse of force), but I suppose it\u0027s not always present, and it\u0027d be much weirder to add --no-delete to our delete subparser.","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":true,"context_lines":[{"line_number":530,"context_line":"    if _requires_disallowed_deletes(broker, args):"},{"line_number":531,"context_line":"        return EXIT_USER_QUIT"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"    shard_data \u003d _load_and_validate_shard_data(args)"},{"line_number":534,"context_line":"    return _replace_shard_ranges(broker, args, shard_data)"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"6d4fd584_5b51c35b","line":533,"updated":"2022-11-30 20:04:48.000000000","message":"Might still want to do the validation of user input...","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":true,"context_lines":[{"line_number":536,"context_line":""},{"line_number":537,"context_line":"def find_replace_shard_ranges(broker, args):"},{"line_number":538,"context_line":"    if _requires_disallowed_deletes(broker, args):"},{"line_number":539,"context_line":"        return EXIT_USER_QUIT"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"    shard_data, delta_t \u003d _find_ranges(broker, args, sys.stdout)"},{"line_number":542,"context_line":"    # Since we\u0027re trying to one-shot this, and the previous step probably"}],"source_content_type":"text/x-python","patch_set":2,"id":"bec7cd9a_dc33644a","line":539,"updated":"2022-11-30 20:04:48.000000000","message":"So, exit 3 -- pretty sure our controller will still interpret this as a failed command, but at least there wouldn\u0027t be the same sort of traceback about an EOFError.\n\nSpeaking of, proposed https://review.opendev.org/c/openstack/swift/+/866209 to treat EOFErrors as negative responses in the handful of places we use input().","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":false,"context_lines":[{"line_number":538,"context_line":"    if _requires_disallowed_deletes(broker, args):"},{"line_number":539,"context_line":"        return EXIT_USER_QUIT"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"    shard_data, delta_t \u003d _find_ranges(broker, args, sys.stdout)"},{"line_number":542,"context_line":"    # Since we\u0027re trying to one-shot this, and the previous step probably"},{"line_number":543,"context_line":"    # took a while, make the timeout for writing *at least* that long"},{"line_number":544,"context_line":"    return _replace_shard_ranges(broker, args, shard_data, timeout\u003ddelta_t)"}],"source_content_type":"text/x-python","patch_set":2,"id":"45a69cb5_b57c9bc3","line":541,"updated":"2022-11-30 20:04:48.000000000","message":"Yeah, LBYL definitely seems right here.","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":false,"context_lines":[{"line_number":959,"context_line":"        help\u003d\u0027Delete existing shard ranges; no questions asked.\u0027)"},{"line_number":960,"context_line":"    group.add_argument("},{"line_number":961,"context_line":"        \u0027--no-delete\u0027, action\u003d\u0027store_true\u0027, default\u003dFalse,"},{"line_number":962,"context_line":"        help\u003d\u0027Do not delete existing shard ranges; no questions asked.\u0027)"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":""},{"line_number":965,"context_line":"def _add_replace_args(parser):"}],"source_content_type":"text/x-python","patch_set":2,"id":"c7990db7_bba56d09","line":962,"updated":"2022-11-30 20:04:48.000000000","message":"Note that we *don\u0027t* want to add -n as a short arg here; we already use it for --dry-run.","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"071a249b4de2f330b6ccd0925cde4bc79b383c23","unresolved":true,"context_lines":[{"line_number":970,"context_line":"    parser.add_argument("},{"line_number":971,"context_line":"        \u0027--enable\u0027, action\u003d\u0027store_true\u0027, default\u003dFalse,"},{"line_number":972,"context_line":"        help\u003d\u0027Enable sharding after adding shard ranges.\u0027)"},{"line_number":973,"context_line":"    _add_force_delete_args(parser)"},{"line_number":974,"context_line":""},{"line_number":975,"context_line":""},{"line_number":976,"context_line":"def _add_enable_args(parser):"}],"source_content_type":"text/x-python","patch_set":2,"id":"179592de_3901372f","line":973,"updated":"2022-11-30 20:04:48.000000000","message":"This is the only place we call this -- do we really need the new helper?","commit_id":"ae06c6a9b6255918bc195a5288f678123e0928cd"}]}