)]}'
{"kolla_cli/api/config.py":[{"author":{"_account_id":16520,"name":"Borne Mace","email":"borne.mace@oracle.com","username":"bmace"},"change_message_id":"f729aaf8792633497a9cc48f2bc6cc98db23a2b5","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        :param file_path: path to inventory file to import"},{"line_number":50,"context_line":"        :type file_path: string"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        check_arg(file_path, u._(\u0027File path\u0027), str)"},{"line_number":53,"context_line":"        if not os.path.exists(file_path):"},{"line_number":54,"context_line":"            raise Exception("},{"line_number":55,"context_line":"                u._(\u0027inventory file {path} is not exists.\u0027).format("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_e0919047","line":52,"updated":"2018-06-26 15:06:37.000000000","message":"I\u0027m sort of torn on this change.  Doesn\u0027t the isfile check fail if the file doesn\u0027t exist and the error returned that the path is not valid?  I am just trying to understand the reason for the additional check.","commit_id":"81017289b2465350ef86022f34769e1017f0ea1b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"2c6df450772eb97ee05b3b1808edf1620cecde68","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        :param file_path: path to inventory file to import"},{"line_number":50,"context_line":"        :type file_path: string"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        check_arg(file_path, u._(\u0027File path\u0027), str)"},{"line_number":53,"context_line":"        if not os.path.exists(file_path):"},{"line_number":54,"context_line":"            raise Exception("},{"line_number":55,"context_line":"                u._(\u0027inventory file {path} is not exists.\u0027).format("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_bdc97e25","line":52,"in_reply_to":"5f7c97a3_06038411","updated":"2018-06-26 18:15:39.000000000","message":"This new check is not needed. If the path is to a non-existent file, it will return false, which will throw the exception.\n\n\u003e\u003e\u003e os.path.isfile(\u0027/eiwquie\u0027)\nFalse\n\n(Also, we should not introduce generic Exceptions into the code.)","commit_id":"81017289b2465350ef86022f34769e1017f0ea1b"},{"author":{"_account_id":22165,"name":"caoyuan","email":"cao.yingjunz@gmail.com","username":"caoyuan"},"change_message_id":"609d5a76e3e953b86f6934cd87a33b19d7a09c48","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        :param file_path: path to inventory file to import"},{"line_number":50,"context_line":"        :type file_path: string"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        check_arg(file_path, u._(\u0027File path\u0027), str)"},{"line_number":53,"context_line":"        if not os.path.exists(file_path):"},{"line_number":54,"context_line":"            raise Exception("},{"line_number":55,"context_line":"                u._(\u0027inventory file {path} is not exists.\u0027).format("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_28a9a1db","line":52,"in_reply_to":"5f7c97a3_06038411","updated":"2018-06-27 03:33:14.000000000","message":"for me, add the check is better, \nyour suggestion is LGTM, I will abondon this ps\nthanks for your review","commit_id":"81017289b2465350ef86022f34769e1017f0ea1b"},{"author":{"_account_id":22165,"name":"caoyuan","email":"cao.yingjunz@gmail.com","username":"caoyuan"},"change_message_id":"609d5a76e3e953b86f6934cd87a33b19d7a09c48","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        :param file_path: path to inventory file to import"},{"line_number":50,"context_line":"        :type file_path: string"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        check_arg(file_path, u._(\u0027File path\u0027), str)"},{"line_number":53,"context_line":"        if not os.path.exists(file_path):"},{"line_number":54,"context_line":"            raise Exception("},{"line_number":55,"context_line":"                u._(\u0027inventory file {path} is not exists.\u0027).format("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_e8ae49e2","line":52,"in_reply_to":"5f7c97a3_bdc97e25","updated":"2018-06-27 03:33:14.000000000","message":"ok, I will abondon this PS, thanks for your review","commit_id":"81017289b2465350ef86022f34769e1017f0ea1b"},{"author":{"_account_id":21691,"name":"weizj","email":"wzj334965317@outlook.com","username":"ZhijunWei"},"change_message_id":"9f1acca5407e19d42733c55ec1c8efa0bf180f99","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        :param file_path: path to inventory file to import"},{"line_number":50,"context_line":"        :type file_path: string"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        check_arg(file_path, u._(\u0027File path\u0027), str)"},{"line_number":53,"context_line":"        if not os.path.exists(file_path):"},{"line_number":54,"context_line":"            raise Exception("},{"line_number":55,"context_line":"                u._(\u0027inventory file {path} is not exists.\u0027).format("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_ebd41149","line":52,"in_reply_to":"5f7c97a3_e0919047","updated":"2018-06-26 15:34:17.000000000","message":"so do you have some suggestion?","commit_id":"81017289b2465350ef86022f34769e1017f0ea1b"},{"author":{"_account_id":16520,"name":"Borne Mace","email":"borne.mace@oracle.com","username":"bmace"},"change_message_id":"f12fe72c751f81bc64d78750096a2e4d39647e06","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        :param file_path: path to inventory file to import"},{"line_number":50,"context_line":"        :type file_path: string"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        check_arg(file_path, u._(\u0027File path\u0027), str)"},{"line_number":53,"context_line":"        if not os.path.exists(file_path):"},{"line_number":54,"context_line":"            raise Exception("},{"line_number":55,"context_line":"                u._(\u0027inventory file {path} is not exists.\u0027).format("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_06038411","line":52,"in_reply_to":"5f7c97a3_ebd41149","updated":"2018-06-26 16:39:22.000000000","message":"well, my suggestion is, if the code already fails because isfile is false on a non-existing file, and we get a \"path is not valid\" message, why would we need a separate specific message about it not existing?   as a user, if i see the message about \"path X invalid\" and go look in the file system, and it isn\u0027t there, that feels like pretty obviously the problem, so is this addition really necessary?","commit_id":"81017289b2465350ef86022f34769e1017f0ea1b"}]}