-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added --output-dir flag in download command #98
Conversation
cf_remote/commands.py
Outdated
@@ -296,7 +296,7 @@ def _iterate_over_packages(tags=None, version=None, edition=None, download=False | |||
else: | |||
for artifact in artifacts: | |||
if download: | |||
download_package(artifact.url) | |||
download_package(artifact.url, directory=output_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at the ticket again. We want to keep downloading to the same place, and then copy once it's done downloading and also checksum is verified.
cf_remote/commands.py
Outdated
tmp_filename = '.{}.tmp'.format(filename) | ||
tmp_output_path = os.path.join(output_dir, tmp_filename) | ||
|
||
shutil.copyfile(package_path, tmp_output_path) | ||
os.rename(tmp_output_path, output_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@victormlg This logic (generate tmp filename, copy to temp file, rename) is generally useful. Maybe you could put these 4 lines into a new function, called copy_file
in utils.py
?
cf_remote/utils.py
Outdated
tmp_output_path = os.path.join(output_dir, tmp_filename) | ||
|
||
shutil.copyfile(input_path, tmp_output_path) | ||
os.rename(tmp_output_path, output_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a newline (empty line) at the end of the file. That's what the red circle with minus means here in GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@victormlg Looks good, can you squash the commits?
Ticket: CFE-3684 Signed-off-by: Victor Moene <[email protected]>
No description provided.