jborean93 / smbprotocol

Python SMBv2 and v3 Client
MIT License
320 stars 74 forks source link

Check whether remote path before calling makedirs #97

Closed dHannasch closed 3 years ago

dHannasch commented 3 years ago

This is built atop https://github.com/jborean93/smbprotocol/pull/96; look there first.

This allows copytree to work with a local destination.

codecov[bot] commented 3 years ago

Codecov Report

Merging #97 (6a14f0f) into master (3c58580) will increase coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   98.61%   98.63%   +0.02%     
==========================================
  Files          23       23              
  Lines        4979     4991      +12     
==========================================
+ Hits         4910     4923      +13     
+ Misses         69       68       -1     
Impacted Files Coverage Δ
smbclient/_os.py 98.49% <100.00%> (+0.01%) :arrow_up:
smbclient/path.py 92.45% <100.00%> (+1.75%) :arrow_up:
smbclient/shutil.py 92.09% <100.00%> (+0.38%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3c58580...6a14f0f. Read the comment docs.

jborean93 commented 3 years ago

For the same reasons listed here https://github.com/jborean93/smbprotocol/pull/100#issuecomment-834071301 this is not something I think _os should be handling. Any of the remote call smbclient._os or local call stdlib.os logic should be handled in smbclient.shutil, either in the function that requires it or in some other helper function.

jborean93 commented 3 years ago

My comment from https://github.com/jborean93/smbprotocol/pull/97#issuecomment-834073455 is still the ideal case. The _os.py functions are meant to be stubs for pure SMB calls. It's not meant to deal with cross SMB and local paths. The way forward for implementing local path support in copytree would be to handle the is_remote_path checks in smbclient.shutil.py that decide whether to call the smbclient._os.py function for SMB paths or the builtin os/shutil one.

There are quite a few merge conflicts that I encountered when trying to rebase https://github.com/jborean93/smbprotocol/pull/96 so I'm going to close this PR and if you still wish to implement this feature please open a new one with the changes you have in mind and we can move forward from there.