)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"2815e3c05b1b102375e727e3ff6bf4df52c62f29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"816901b9_bbbcf9e6","updated":"2023-05-31 06:38:33.000000000","message":"recheck","commit_id":"82f3b1cf1534b87b83850dd73ee3af1a3811f7e3"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"8718e7469f37af3a6aa5f6984631174241f027f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"32860576_8708aab9","updated":"2023-06-05 08:44:19.000000000","message":"recheck","commit_id":"09bb33c0df2351965b65d286685655a26fa1475e"}],"logscraper/logsender.py":[{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"4bcded7712f48e474a8c1ab1279744bdd69e8332","unresolved":true,"context_lines":[{"line_number":136,"context_line":"        return yaml.load(f)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"def remove_old_dir(root, build_uuid, files):"},{"line_number":140,"context_line":"    # Skip main download directory"},{"line_number":141,"context_line":"    if not files:"},{"line_number":142,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":5,"id":"a1f4ab5f_920d9e1f","line":139,"updated":"2023-05-31 11:48:19.000000000","message":"Would such simpler implementation also works?\n\n```python\ndef remove_old_dir(path):\n  if age(path) \u003e 12 * 3600:\n     remove_directory(path)\n```","commit_id":"82f3b1cf1534b87b83850dd73ee3af1a3811f7e3"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"ffc303daad8d7dd9ed7d7d5b0f067bcef27ae476","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        return yaml.load(f)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"def remove_old_dir(root, build_uuid, files):"},{"line_number":140,"context_line":"    # Skip main download directory"},{"line_number":141,"context_line":"    if not files:"},{"line_number":142,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":5,"id":"4228f712_5deff08e","line":139,"in_reply_to":"a0864478_ab6d8f3d","updated":"2023-06-05 08:19:09.000000000","message":"Done","commit_id":"82f3b1cf1534b87b83850dd73ee3af1a3811f7e3"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"a5c12bd2efe64243d1ca895714e1fa3ac26f1cb1","unresolved":true,"context_lines":[{"line_number":136,"context_line":"        return yaml.load(f)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"def remove_old_dir(root, build_uuid, files):"},{"line_number":140,"context_line":"    # Skip main download directory"},{"line_number":141,"context_line":"    if not files:"},{"line_number":142,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":5,"id":"a0864478_ab6d8f3d","line":139,"in_reply_to":"a1f4ab5f_920d9e1f","updated":"2023-05-31 13:40:05.000000000","message":"you mean just check the dir age and remove it when it\u0027s old?\nVery simple and it is a good idea!","commit_id":"82f3b1cf1534b87b83850dd73ee3af1a3811f7e3"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"4bcded7712f48e474a8c1ab1279744bdd69e8332","unresolved":true,"context_lines":[{"line_number":155,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"},{"line_number":156,"context_line":"        remove_directory(build_dir_path)"},{"line_number":157,"context_line":"    else:"},{"line_number":158,"context_line":"        with open(\"%s/skip_counter\" % build_dir_path, \"w\") as f:"},{"line_number":159,"context_line":"            f.write(str(ts_now.timestamp()))"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"96d277cc_4e94b492","line":158,"range":{"start_line":158,"start_character":22,"end_line":158,"end_character":34},"updated":"2023-05-31 11:48:19.000000000","message":"Should this be `skip_date` ?","commit_id":"82f3b1cf1534b87b83850dd73ee3af1a3811f7e3"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"2c88e4ff4fb63df0fa5a0f4858c719f96381ca39","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"},{"line_number":156,"context_line":"        remove_directory(build_dir_path)"},{"line_number":157,"context_line":"    else:"},{"line_number":158,"context_line":"        with open(\"%s/skip_counter\" % build_dir_path, \"w\") as f:"},{"line_number":159,"context_line":"            f.write(str(ts_now.timestamp()))"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a2e3080_8a732018","line":158,"range":{"start_line":158,"start_character":22,"end_line":158,"end_character":34},"in_reply_to":"96d277cc_4e94b492","updated":"2023-05-31 13:41:10.000000000","message":"yup, should be skip_date","commit_id":"82f3b1cf1534b87b83850dd73ee3af1a3811f7e3"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"88c950c34df4cd8eb02cdfcd03f768e33853303f","unresolved":true,"context_lines":[{"line_number":141,"context_line":"    if not files:"},{"line_number":142,"context_line":"        return"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    ago \u003d datetime.datetime.utcnow() - datetime.timedelta(hours\u003d12)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    build_dir_path \u003d \"%s/%s\" % (root, build_uuid)"},{"line_number":147,"context_line":"    dir_timestamp \u003d os.stat(build_dir_path).st_mtime"}],"source_content_type":"text/x-python","patch_set":6,"id":"5495ae88_d153e56e","line":144,"updated":"2023-06-01 14:34:33.000000000","message":"nit: how about naming this variable `min_age`, then you can compare `if build_age \u003c min_age`","commit_id":"0aaddbe7711ef4ce3bac36098fa7072f45611463"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"ffc303daad8d7dd9ed7d7d5b0f067bcef27ae476","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    if not files:"},{"line_number":142,"context_line":"        return"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    ago \u003d datetime.datetime.utcnow() - datetime.timedelta(hours\u003d12)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    build_dir_path \u003d \"%s/%s\" % (root, build_uuid)"},{"line_number":147,"context_line":"    dir_timestamp \u003d os.stat(build_dir_path).st_mtime"}],"source_content_type":"text/x-python","patch_set":6,"id":"824970a3_56d53beb","line":144,"in_reply_to":"5495ae88_d153e56e","updated":"2023-06-05 08:19:09.000000000","message":"Done","commit_id":"0aaddbe7711ef4ce3bac36098fa7072f45611463"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"88c950c34df4cd8eb02cdfcd03f768e33853303f","unresolved":true,"context_lines":[{"line_number":144,"context_line":"    ago \u003d datetime.datetime.utcnow() - datetime.timedelta(hours\u003d12)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    build_dir_path \u003d \"%s/%s\" % (root, build_uuid)"},{"line_number":147,"context_line":"    dir_timestamp \u003d os.stat(build_dir_path).st_mtime"},{"line_number":148,"context_line":"    if ago.timestamp() \u003e dir_timestamp:"},{"line_number":149,"context_line":"        logging.warning(\"Some files are still missing for %s and nothing \""},{"line_number":150,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"6b0d54f3_305a1c2b","line":147,"updated":"2023-06-01 14:34:33.000000000","message":"nit, you can also use pathlib like that:\n\n  from pathlib import Path\n  \n  build_age \u003d (Path(root) / build_uuid).stat().st_mtime","commit_id":"0aaddbe7711ef4ce3bac36098fa7072f45611463"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"ffc303daad8d7dd9ed7d7d5b0f067bcef27ae476","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    ago \u003d datetime.datetime.utcnow() - datetime.timedelta(hours\u003d12)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    build_dir_path \u003d \"%s/%s\" % (root, build_uuid)"},{"line_number":147,"context_line":"    dir_timestamp \u003d os.stat(build_dir_path).st_mtime"},{"line_number":148,"context_line":"    if ago.timestamp() \u003e dir_timestamp:"},{"line_number":149,"context_line":"        logging.warning(\"Some files are still missing for %s and nothing \""},{"line_number":150,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a30db0d5_635d588d","line":147,"in_reply_to":"6b0d54f3_305a1c2b","updated":"2023-06-05 08:19:09.000000000","message":"Done","commit_id":"0aaddbe7711ef4ce3bac36098fa7072f45611463"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"88c950c34df4cd8eb02cdfcd03f768e33853303f","unresolved":true,"context_lines":[{"line_number":177,"context_line":"            files.remove(\"buildinfo\")"},{"line_number":178,"context_line":"            files.remove(\"inventory.yaml\")"},{"line_number":179,"context_line":"            log_files[build_uuid] \u003d files"},{"line_number":180,"context_line":"        else:"},{"line_number":181,"context_line":"            logging.info(\"Skipping build with uuid %s. Probably all files \""},{"line_number":182,"context_line":"                         \"are not downloaded yet.\" % build_uuid)"},{"line_number":183,"context_line":"            remove_old_dir(root, build_uuid, files)"}],"source_content_type":"text/x-python","patch_set":6,"id":"f2f33d6c_4816587e","line":180,"updated":"2023-06-01 14:34:33.000000000","message":"instead of checking for main download directory in the remove_old_dir function, i think it would be better to do the check here:\n\n```\nelif files:\n```","commit_id":"0aaddbe7711ef4ce3bac36098fa7072f45611463"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"e649f58c2d1b15ba885ee6b2892f673f090a2836","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            files.remove(\"buildinfo\")"},{"line_number":178,"context_line":"            files.remove(\"inventory.yaml\")"},{"line_number":179,"context_line":"            log_files[build_uuid] \u003d files"},{"line_number":180,"context_line":"        else:"},{"line_number":181,"context_line":"            logging.info(\"Skipping build with uuid %s. Probably all files \""},{"line_number":182,"context_line":"                         \"are not downloaded yet.\" % build_uuid)"},{"line_number":183,"context_line":"            remove_old_dir(root, build_uuid, files)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ac1b3616_46ff8c36","line":180,"in_reply_to":"f2f33d6c_4816587e","updated":"2023-06-05 12:02:27.000000000","message":"Done","commit_id":"0aaddbe7711ef4ce3bac36098fa7072f45611463"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"e649f58c2d1b15ba885ee6b2892f673f090a2836","unresolved":true,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    min_age \u003d datetime.datetime.utcnow() - datetime.timedelta(hours\u003d12)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    build_dir_path \u003d \"%s/%s\" % (root, build_uuid)"},{"line_number":148,"context_line":"    build_age \u003d (Path(root) / build_uuid).stat().st_mtime"},{"line_number":149,"context_line":"    if min_age.timestamp() \u003e build_age:"},{"line_number":150,"context_line":"        logging.warning(\"Some files are still missing for %s and nothing \""},{"line_number":151,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf6863d5_f0bad843","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":57},"updated":"2023-06-05 12:02:27.000000000","message":"pathlib takes care of directory/path construction, so we can:\n\n```python\n    build_dir_path \u003d Path(root) / build_uuid\n    build_age \u003d build_dir_path.stat().st_mtime\n```\n\nThen rmtree works with pathlib.Path","commit_id":"09bb33c0df2351965b65d286685655a26fa1475e"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"fefb1b05ea6272b54d0a2832c1697273af7effc8","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    min_age \u003d datetime.datetime.utcnow() - datetime.timedelta(hours\u003d12)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    build_dir_path \u003d \"%s/%s\" % (root, build_uuid)"},{"line_number":148,"context_line":"    build_age \u003d (Path(root) / build_uuid).stat().st_mtime"},{"line_number":149,"context_line":"    if min_age.timestamp() \u003e build_age:"},{"line_number":150,"context_line":"        logging.warning(\"Some files are still missing for %s and nothing \""},{"line_number":151,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"88f7a642_aa3d61d8","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":57},"in_reply_to":"bf6863d5_f0bad843","updated":"2023-06-05 14:26:38.000000000","message":"good to know.\nSo far, remove_directory function is using shutil.rmtree and I don\u0027t want to change it in that PS.","commit_id":"09bb33c0df2351965b65d286685655a26fa1475e"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"e649f58c2d1b15ba885ee6b2892f673f090a2836","unresolved":true,"context_lines":[{"line_number":148,"context_line":"    build_age \u003d (Path(root) / build_uuid).stat().st_mtime"},{"line_number":149,"context_line":"    if min_age.timestamp() \u003e build_age:"},{"line_number":150,"context_line":"        logging.warning(\"Some files are still missing for %s and nothing \""},{"line_number":151,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"},{"line_number":152,"context_line":"        remove_directory(build_dir_path)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"539ed6a9_51bd55bb","line":151,"updated":"2023-06-05 12:02:27.000000000","message":"With logging, the string format parameter can be passed as argument, to avoid un-necessary formatting work, e.g.:\n\n```python\n  logging.warning(\"Some files are still missing for %s ...\", build_uuid) \n```","commit_id":"09bb33c0df2351965b65d286685655a26fa1475e"},{"author":{"_account_id":20676,"name":"daniel.pawlik","display_name":"Daniel Pawlik","email":"dpawlik@redhat.com","username":"daniel.pawlik"},"change_message_id":"fefb1b05ea6272b54d0a2832c1697273af7effc8","unresolved":false,"context_lines":[{"line_number":148,"context_line":"    build_age \u003d (Path(root) / build_uuid).stat().st_mtime"},{"line_number":149,"context_line":"    if min_age.timestamp() \u003e build_age:"},{"line_number":150,"context_line":"        logging.warning(\"Some files are still missing for %s and nothing \""},{"line_number":151,"context_line":"                        \"changed for 12 hours.\" % build_uuid)"},{"line_number":152,"context_line":"        remove_directory(build_dir_path)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5ef1fdc7_c2729406","line":151,"in_reply_to":"539ed6a9_51bd55bb","updated":"2023-06-05 14:26:38.000000000","message":"ack","commit_id":"09bb33c0df2351965b65d286685655a26fa1475e"}]}