search

jborean93 / smbprotocol

Python SMBv2 and v3 Client
MIT License
309 stars 72 forks source link

standard `kwargs` not supported in `copytree` #249

Open brno32 opened 8 months ago

brno32 commented 8 months ago

Hello,

I tried to specify buffering and share_access in smbclient.shutil.copytree like so:

smbclient.shutil.copytree(src, dist, buffering=SIZE, share_access="r")

and it yields the following error

TypeError: get_smb_tree() got an unexpected argument 'buffering'

Looking at the source code, I can see why. Inside of scandir, which copytree calls internally, these kwargs get forwarded to get_smb_tree().

Can we add some additional logic to pop these kwargs when specified inside of copytree, but use their values where appropriate? (such as in copyfile)

jborean93 commented 7 months ago

Hmm I can see buffering being something we may want on there as at least it's own option to use when copying files but I'm unsure whether share_access should be exposed and whether we should just always be opening with a read share access. I can't think of a reason why that shouldn't always be there for the source file.

brno32 commented 7 months ago

If the underlying function that copies the files exposes share_access I think it'd be nice to have control over it from the copytree function that wraps it

jborean93 commented 7 months ago

I’m just saying what would the purpose be for it. I can understand doing read share access by default but allowing write or delete means the file could be changed as it is being read from. This would also only apply to the source file, the dest file would most likely not have have share flags because the contents are being changed.

I’m just not sure there are any benefits to allow a user defined share_access here aside from changing the default on the source file.

brno32 commented 7 months ago

fair enough. I could make a PR that:

jborean93 commented 7 months ago

Sounds fair, keep in mind the copytree function is quite complicated. There are two different types of copy modes used one for when the files are on the same share which does a server side copy and another when the files are on different locations which falls back to shutil.copyfileobj. I would probably investigate the reasons why you want to provide an explicit buffering value here rather than expose a new kwarg that may or may not be used depending on the src and dst paths.

Setting the share_access would be done on the src_open but only when src_open is our SMB open_file implementation. Setting it on the dest open wouldn't really make much sense there.