)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":36164,"name":"kim woo seok","display_name":"rladntjr4","email":"rladntjr4@gmail.com","username":"rladntjr4"},"change_message_id":"e51ce7a424d4c779b7b11dfecac182fa0726a904","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5ad82164_93c189a7","updated":"2023-09-23 14:59:55.000000000","message":"I\u0027ve made a new commit to address the points you raised.\nhttps://review.opendev.org/c/openstack/swift/+/896313\n\nLet\u0027s explore the scenarios involving multiple disks or policies, as you mentioned.\nThank you for your review! I am not good at English. Please understand if it\u0027s awkward.","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d7929dc5b942695ad7dfec31809ab0c7363ead03","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0b4c5901_0347369a","updated":"2023-09-22 19:32:09.000000000","message":"Nice! Looks like a pretty straight-forward extraction. I might be tempted to keep a\n```\nif __name__ \u003d\u003d \"__main__\":\n    main()\n```\nin `swift/cli/recon_cron.py` so you could invoke it as `python -m swift.cli.recon_cron`, but there are a bunch of other files under `swift/cli` without it, so I\u0027m not too worried -- we can always add it later if we really want.\n\nNew test makes sense, and gives us a good starting point if we wanted to expand to cover multiple disks or multiple policies (which might be a nice exercise if you wanted to learn more about how Swift structures data on disk).\n\nAll of that (and the handful of other little suggestions) could be done as future patches, though -- LGTM as-is.\n\nI should go rebase https://review.opendev.org/c/openstack/swift/+/895737 on top and add some more tests for it!","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"}],"swift/cli/recon_cron.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d7929dc5b942695ad7dfec31809ab0c7363ead03","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from swift.obj.diskfile import ASYNCDIR_BASE"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"def get_async_count(device_dir):"},{"line_number":27,"context_line":"    async_count \u003d 0"},{"line_number":28,"context_line":"    for i in os.listdir(device_dir):"},{"line_number":29,"context_line":"        device \u003d os.path.join(device_dir, i)"}],"source_content_type":"text/x-python","patch_set":5,"id":"49f811c6_8c599c7e","line":26,"updated":"2023-09-22 19:32:09.000000000","message":"OK -- `logger` is gone, but it wasn\u0027t ever used anyway.","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d7929dc5b942695ad7dfec31809ab0c7363ead03","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    return async_count"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def main():"},{"line_number":48,"context_line":"    try:"},{"line_number":49,"context_line":"        conf_path \u003d sys.argv[1]"},{"line_number":50,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"20f803f7_29ee18b6","line":47,"updated":"2023-09-22 19:32:09.000000000","message":"FWIW, if you wanted to try adding tests for this function, too, one idiom I\u0027ve seen is to do something like\n```\ndef main(arguments\u003dNone):\n    if arguments is not None:\n        argv \u003d arguments\n    else:\n        argv \u003d sys.argv\n    ...\n```\nto make it easier to pass some arbitrary args in tests.","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"},{"author":{"_account_id":36164,"name":"kim woo seok","display_name":"rladntjr4","email":"rladntjr4@gmail.com","username":"rladntjr4"},"change_message_id":"e51ce7a424d4c779b7b11dfecac182fa0726a904","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    return async_count"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def main():"},{"line_number":48,"context_line":"    try:"},{"line_number":49,"context_line":"        conf_path \u003d sys.argv[1]"},{"line_number":50,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"6b48afdf_2b5caa14","line":47,"in_reply_to":"20f803f7_29ee18b6","updated":"2023-09-23 14:59:55.000000000","message":"Ack","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d7929dc5b942695ad7dfec31809ab0c7363ead03","unresolved":true,"context_lines":[{"line_number":70,"context_line":"        msg \u003d \u0027Exception during recon-cron while accessing devices\u0027"},{"line_number":71,"context_line":"        logger.exception(msg)"},{"line_number":72,"context_line":"        print(\u0027%s: %s\u0027 % (msg, err))"},{"line_number":73,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":5,"id":"6899dd4b_8f17cb10","line":73,"updated":"2023-09-22 19:32:09.000000000","message":"Since we\u0027ve got `sys.exit(main())` in the slimmed-down `bin/swift-recon-cron`, I think this might be better as `return 1`\n\nWill also make it easier to test this section, too, when we get to that.","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"},{"author":{"_account_id":36164,"name":"kim woo seok","display_name":"rladntjr4","email":"rladntjr4@gmail.com","username":"rladntjr4"},"change_message_id":"e51ce7a424d4c779b7b11dfecac182fa0726a904","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        msg \u003d \u0027Exception during recon-cron while accessing devices\u0027"},{"line_number":71,"context_line":"        logger.exception(msg)"},{"line_number":72,"context_line":"        print(\u0027%s: %s\u0027 % (msg, err))"},{"line_number":73,"context_line":"        sys.exit(1)"}],"source_content_type":"text/x-python","patch_set":5,"id":"457080ce_1f36fa31","line":73,"in_reply_to":"6899dd4b_8f17cb10","updated":"2023-09-23 14:59:55.000000000","message":"Done","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"}],"test/unit/cli/test_recon_cron.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d7929dc5b942695ad7dfec31809ab0c7363ead03","unresolved":true,"context_lines":[{"line_number":36,"context_line":"        device_index \u003d os.path.join(device_dir, \u00271\u0027)"},{"line_number":37,"context_line":"        os.makedirs(device_index)"},{"line_number":38,"context_line":"        async_dir \u003d os.path.join(device_index, ASYNCDIR_BASE)"},{"line_number":39,"context_line":"        os.makedirs(async_dir)"},{"line_number":40,"context_line":"        entry1 \u003d os.path.join(async_dir, \u0027entry1\u0027)"},{"line_number":41,"context_line":"        entry2 \u003d os.path.join(async_dir, \u0027entry2\u0027)"},{"line_number":42,"context_line":"        os.makedirs(entry1)"}],"source_content_type":"text/x-python","patch_set":5,"id":"17d16f4c_72d38bbf","line":39,"updated":"2023-09-22 19:32:09.000000000","message":"nit: Since we\u0027re using `os.makedirs` (which will create any intermediate directories), I think we could probably skip it for `device_dir`, `device_index`, and `async_dir`","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"},{"author":{"_account_id":36164,"name":"kim woo seok","display_name":"rladntjr4","email":"rladntjr4@gmail.com","username":"rladntjr4"},"change_message_id":"e51ce7a424d4c779b7b11dfecac182fa0726a904","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        device_index \u003d os.path.join(device_dir, \u00271\u0027)"},{"line_number":37,"context_line":"        os.makedirs(device_index)"},{"line_number":38,"context_line":"        async_dir \u003d os.path.join(device_index, ASYNCDIR_BASE)"},{"line_number":39,"context_line":"        os.makedirs(async_dir)"},{"line_number":40,"context_line":"        entry1 \u003d os.path.join(async_dir, \u0027entry1\u0027)"},{"line_number":41,"context_line":"        entry2 \u003d os.path.join(async_dir, \u0027entry2\u0027)"},{"line_number":42,"context_line":"        os.makedirs(entry1)"}],"source_content_type":"text/x-python","patch_set":5,"id":"8f0c3e7f_547a0e1a","line":39,"in_reply_to":"17d16f4c_72d38bbf","updated":"2023-09-23 14:59:55.000000000","message":"Done","commit_id":"62a9dbca7613a760bc9ea179ab030f433b76f4a6"}]}