Skip to content

Cloud implementation of shutil.copy and shutil.copytree - #142

Merged
pjbull merged 11 commits into
masterfrom
108-copy-upload
May 29, 2021
Merged

Cloud implementation of shutil.copy and shutil.copytree#142
pjbull merged 11 commits into
masterfrom
108-copy-upload

Conversation

@pjbull

@pjbull pjbull commented Apr 21, 2021

Copy link
Copy Markdown
Member

Seems like @genziano was busy (thanks for the initial work!), so I moved over his WIP from #125 to this branch.

@BenAsaf feel free to pick it up from here.


Moved over PR from #125:

  • Draft of cloudpathlib.copy, that replicates shutil.copy

  • Import only PathLike from os

  • Reuse _local_to_cloud_copy for cached files

  • try to use the intended cached _local version

  • Update cloudshutil.py

  • Implemented the CloudPath methods: upload_from, copy, copytree

  • Add docstrings

  • Fix indentation

  • fix unnecessary imports

  • Fix typing issues

  • Allow CloudPath.copy to a directory as well

Closes #58
Closes #108

* Draft of cloudpathlib.copy, that replicates shutil.copy

* Import only PathLike from os

* Reuse _local_to_cloud_copy for cached files

* try to use the intended cached _local version

* Update cloudshutil.py

* Implemented the CloudPath methods: upload_from, copy, copytree

* Add docstrings

* Fix indentation

* fix unnecessary imports

* Fix typing issues

* Allow CloudPath.copy to a directory as well
@pjbull
pjbull marked this pull request as draft April 21, 2021 05:48
@github-actions

github-actions Bot commented Apr 21, 2021

Copy link
Copy Markdown
Contributor

@codecov

codecov Bot commented Apr 21, 2021

Copy link
Copy Markdown

Codecov Report

Merging #142 (8dca2ed) into master (de6b547) will increase coverage by 0.4%.
The diff coverage is 100.0%.

❗ Current head 8dca2ed differs from pull request most recent head dc1c3a4. Consider uploading reports for the commit dc1c3a4 to get more accurate results

@@           Coverage Diff            @@
##           master    #142     +/-   ##
========================================
+ Coverage    93.5%   93.9%   +0.4%     
========================================
  Files          21      21             
  Lines        1119    1177     +58     
========================================
+ Hits         1047    1106     +59     
+ Misses         72      71      -1     
Impacted Files Coverage Δ
cloudpathlib/anypath.py 100.0% <100.0%> (ø)
cloudpathlib/azure/azblobclient.py 94.6% <100.0%> (+<0.1%) ⬆️
cloudpathlib/client.py 87.5% <100.0%> (ø)
cloudpathlib/cloudpath.py 91.9% <100.0%> (+1.2%) ⬆️
cloudpathlib/gs/gsclient.py 92.0% <100.0%> (+0.1%) ⬆️
cloudpathlib/local/localclient.py 97.4% <100.0%> (+<0.1%) ⬆️
cloudpathlib/s3/s3client.py 93.0% <100.0%> (+0.1%) ⬆️

@genziano

Copy link
Copy Markdown
Contributor

Hey @pjbull, indeed, sorry for leaving it unfinished but it's a very busy period! Glad to have contributed!

@pjbull
pjbull marked this pull request as ready for review May 15, 2021 22:38
@pjbull
pjbull requested a review from jayqi May 15, 2021 22:38
Comment thread cloudpathlib/cloudpath.py Outdated
Comment thread cloudpathlib/cloudpath.py Outdated
Comment thread cloudpathlib/cloudpath.py
Comment thread cloudpathlib/cloudpath.py Outdated
Comment thread cloudpathlib/cloudpath.py Outdated
Comment thread cloudpathlib/cloudpath.py
Comment thread cloudpathlib/cloudpath.py Outdated
Comment thread cloudpathlib/cloudpath.py Outdated
Comment thread cloudpathlib/cloudpath.py
Comment thread cloudpathlib/client.py
Comment thread cloudpathlib/anypath.py
@pjbull

pjbull commented May 20, 2021

Copy link
Copy Markdown
Member Author

@jayqi This is ready for re-review:

  • I opened issue for a few things that were too much to take on and block merging.
  • I'd like to have the default for upload not overwrite a newer file on the cloud. Agreed that normal path functions don't typically care, but I do think overwriting newer versions on cloud storage warrants extra caution.
  • Changing some of the return types per the review was interwoven with a handful of other functions, and mypy was very fussy for a long time. I eventually got there, but had to add a little bit of a workaround to anypath. Would appreciate your eyes there.

@pjbull
pjbull merged commit 3af76b9 into master May 29, 2021
@pjbull
pjbull deleted the 108-copy-upload branch May 29, 2021 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add copy and copytree methods API for creating a cloud file by uploading it

3 participants