fix: download_to function tried to download path files #204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jayqi
reviewed
Feb 5, 2022
jayqi
reviewed
Feb 5, 2022
cloudpathlib/cloudpath.py
Outdated
| @@ -594,14 +594,15 @@ def download_to(self, destination: Union[str, os.PathLike]) -> Path: | |||
| destination = destination / self.name | |||
| return self.client._download_file(self, destination) | |||
| else: | |||
| destination.mkdir(exist_ok=True) | |||
| destination.mkdir(parents=True, exist_ok=True) | |||
There was a problem hiding this comment.
I'm not totally bought in on automatically creating missing parents by default. I believe most things (unix programs and Python libraries) do not normally do this.
A related consideration here is to maybe add a parents parameter to download_to?
There was a problem hiding this comment.
I changed the commit to pass parents as a parameter to download_to
This was referenced Feb 6, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In download directory cases, the
download_tofunction was running in an infinite loop. For example:We have a s3 example bucket with 1 one file in it:
s3://example/test_file.txtThe function
iterdir()will return:In this case the
rel_destvariable will be an empty string for the first case andtest_file.txtfor the second one.The
f.download_to(destination / rel_dest)will call thedownload_tofunction withre_destas empty, causing an infinite loop.The solution was to just verify if
re_destis not empty.I also added the
parents=Truetodestination.mkdirso it will create the directory even if is missing intermediate parent directories.