Fixes in download-file script for windows #318
Fixes in download-file script for windows #318gfursin merged 5 commits intomlcommons:mainfrom anandhu-eng:downloadfilefix
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
| env['CM_PRE_DOWNLOAD_CMD'] = '' | ||
|
|
||
| if os_info['platform'] == 'windows' and env.get('CM_DOWNLOAD_CMD', '') != '': | ||
| env['CM_DOWNLOAD_CMD'] = env['CM_DOWNLOAD_CMD'].replace('&', '^&').replace('|', '^|').replace('(', '^(').replace(')', '^)') |
There was a problem hiding this comment.
don't we need this replace?
There was a problem hiding this comment.
I tested it on both powershell and command prompt. Both worked fine only when the escaping was removed.
There was a problem hiding this comment.
@anandhu-eng - may I suggest you to add # to this line instead of removing. Probably there was a reason why it was added. If we encounter an issue in the future, we can then trace it back ... Thanks a lot!!!
There was a problem hiding this comment.
I will then merge it and test on my machine before running another PR ...
There was a problem hiding this comment.
@anandhu-eng It broke the Windows test of ABTF POC - it was working fine earlier as it uses wget for the download I believe.
https://github.com/mlcommons/cm4mlops/actions/runs/11126891963/job/30917906688?pr=318#step:5:3440
script/download-file/customize.py
Outdated
| if tool != "rclone": | ||
| text = text.replace('%', "%%") | ||
|
|
||
| print(text) |
There was a problem hiding this comment.
we don't need the print right?
3 participants
No description provided.