Add various shutil methods #16

martinhoefling · 2019-11-08T09:45:23Z

Added various methods like copy2, copytree etc. Also added a kwarg that allows to perform copying server side. Many methods are copied and adapted from their python2 shutil counterpart to mimic their behavior. Further, a set of integration tests that at least cover the happy paths was added. @jborean93 let me know what you think.

coveralls · 2019-11-08T09:59:39Z

Coverage decreased (-0.2%) to 94.649% when pulling 7570860 on martinhoefling:easy-api-additions into 0978ba4 on jborean93:easy-api.

jborean93 · 2019-11-08T10:05:04Z

Awesome work thanks for doing this. I'm still working through some final stuff on the easy-api branch and once that is in I'll start the review on this so we can include it in the same release.

martinhoefling · 2019-11-08T12:24:06Z

@jborean93 all right, I will try to fix the build in the meantime.

martinhoefling · 2019-11-09T12:35:44Z

Some parts still require tests and I am not sure how to handle Py2/3 compatibility for some of the error formattings. I'll add some more tests for the missing branches.

martinhoefling · 2019-11-09T15:35:27Z

Ah, this is actually a pytest bug when pytest.raises tries to match a unicode string. pytest-dev/pytest#5478. I'll do the strict checking for the message only on python3.

jborean93

Thanks again for working on this, things look pretty good and have added some comments inline.

smbclient/shutil.py

jborean93 · 2019-11-11T04:57:52Z

smbclient/shutil.py

+    else:
+        raise ValueError("Target %s already exists" % norm_dst)
+
+    with SMBRawIO(norm_src, mode='r', desired_access=FilePipePrinterAccessMask.GENERIC_READ,


it's simpler to just do with open_file(norm_src, mode='r', share_access='r', **kwargs) and rely on that doing the conversion with the access mask and create options.

After rebase, the test are failing, I'll have to find out why first.

Some of the failures were my doing sorry, I renamed some kwargs to avoid the builtin names. One I know off the top of my head is that ioctl_request's buffer kwarg is now input_buffer.

Right, that was the issue!

When changing to the suggested open_file I get

> sid = self.raw.fd.tree_connect.session.session_id E AttributeError: '_io.TextIOWrapper' object has no attribute 'fd'

It sounds like you need to open it in binary mode. Typically you will want to set mode='<x>b (the b is important) to make sure you are dealing with bytes. Internal functions should avoid using the text side unless they really need it.

smbclient/shutil.py

jborean93 · 2019-11-11T05:15:51Z

Also, if you rebase the commit against easy-api I've removed Python 3.4 from the matrix as it is EOL and I've removed support for it. That should get CI green on Appveyor.

…moved

martinhoefling · 2019-11-12T15:00:37Z

I have cleaned up the code a bit, rmtree, copy and copytree_copy are in shutil, the server side implementation has been moved to _os. I still wasn't able to make the SMBRawIO replacement working. I will check out next week.

jborean93

Nearly there, ~~I'm still dubious about the source of the max chunk size part so I'm curious if you've got further evidence that state that's the max size for SMB2.~~ (Edit: see #16 (comment))

smbclient/_os.py

smbclient/shutil.py

tests/test_smbclient_shutil.py

smbclient/_os.py

martinhoefling · 2019-11-18T11:14:07Z

@jborean93 , I hope I have addressed most issues. Two are still open I think:

SMBRawIO usage: Maybe you can take a look, why the suggestions didn't work out.
pytest bug: Same thing. Edit: we just leave it like that and omit checking the message on python 2.7.

jborean93

Looks great, thanks for sticking with this and making my job a whole lot easier :)

jborean93 · 2019-11-18T23:10:24Z

I'll have a look at both points once easy-api has been merged into the master branch before finalising the new release as well.

martinhoefling · 2019-11-19T06:35:07Z

@jborean93 thx, for all your comments and help. Lets get that one released and I would then work on copy2.

jborean93 · 2019-11-20T02:49:30Z

@martinhoefling about those 2 remaining issues, here is the fix for the unicode chars in the exception dd1b5de.

It wasn't an issue in pytest but rather how Python 2 handles native string compared to 3. In 2, a native string is actually a byte string and the default behaviour of converting a unicode string to byte is to encode it using the ascii encoding. This causes issues in cases where the unicode string contains non-ascii chars and then re-raises another ValueError for that. The way to get past that is to ensure you already pass in a native string (byte in 2, unicode in 3) so it doesn't try and encode it using ascii. The to_native method in smbprotocol handles all this for you and defaults to utf-8 which is lot more sane in today's environment.

As for the SMBRawIO to open_file this was fixed in 39d82b7. Basically the SMBFileTransaction expects a SMBRawIO object and when you use open_file that can be encapsulated in a few different layers. In this case I am making sure I open the file in binary mode (mode='*b') and I cna then access the raw IO object with the raw property. I can even take this a step futher and set buffering=0 to open_file and that returns the relevant SMBRawIO object for the file type specified.

I'm hoping to expand on the tests a bit more before the end of the week and get a PR ready this weekend, early next week. From there I'll be using it for some further testing with downstream libraries and hopefully a full release soon after that.

martinhoefling · 2019-11-21T07:59:23Z

Thx!

* Added higher level client API * Fixes after recent pull rebase * remove utils mention now we use conftest * Add default timeout to connection * try and reduce deadlocks in the msg queue processor * Fix deadlock issue and ensure tests are passing locally * Try and smooth out transport error handling * Fix up transport test after change * Dropped support for Python 3.4 * Added the ability to register and close a session in the pool * Fix up test and typos * More typos and minor nit changes * Set Impersonation as the default Impersonation level * Fix up listdir and read for pipes * Add various shutil methods (#16) * Add various shutil methods * reworked rmtree, review comments addressed * renamed copy2 -> copy and copytree -> copytree_copy, copying stats removed * service side copy moved to _os * fix pep8 indentation * review comments in shutil addressed * Fix up exceptions that can have unicode chars * Use open_file for copyfile * Move copyfile to top of file and add more tests * Use set size for SRV_REQUEST_RESUMT_KEY response output * Expand shutil and added more tests * Attempt to get Appveyor working again * Hopefully the last Appveyor fixes to go in * Should be last fix for appveyor * Last Appveyor fixes * Simplify stat and add ads tests * Remove ordereddict requirement

martinhoefling force-pushed the easy-api-additions branch 2 times, most recently from f1cb8f3 to ca2ec50 Compare November 8, 2019 09:55

martinhoefling force-pushed the easy-api-additions branch 5 times, most recently from 92c7d85 to ba35490 Compare November 8, 2019 14:39

martinhoefling force-pushed the easy-api-additions branch 2 times, most recently from b527e7d to 46a7a85 Compare November 9, 2019 15:41

jborean93 requested changes Nov 11, 2019

View reviewed changes

martinhoefling added 2 commits November 11, 2019 08:41

Add various shutil methods

46f17e2

reworked rmtree, review comments addressed

ad094fa

martinhoefling force-pushed the easy-api-additions branch 2 times, most recently from 1b5110f to 84eb2cb Compare November 11, 2019 15:11

renamed copy2 -> copy and copytree -> copytree_copy, copying stats re…

109326f

…moved

martinhoefling force-pushed the easy-api-additions branch from 84eb2cb to 109326f Compare November 11, 2019 20:03

service side copy moved to _os

19eb658

fix pep8 indentation

8f4bfa8

jborean93 requested changes Nov 13, 2019

View reviewed changes

jborean93 reviewed Nov 13, 2019

View reviewed changes

smbclient/_os.py Outdated Show resolved Hide resolved

review comments in shutil addressed

7570860

jborean93 approved these changes Nov 18, 2019

View reviewed changes

jborean93 merged commit 74411dc into jborean93:easy-api Nov 18, 2019

martinhoefling deleted the easy-api-additions branch November 19, 2019 06:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add various shutil methods #16

Add various shutil methods #16

martinhoefling commented Nov 8, 2019

coveralls commented Nov 8, 2019 •

edited

Loading

jborean93 commented Nov 8, 2019

martinhoefling commented Nov 8, 2019

martinhoefling commented Nov 9, 2019

martinhoefling commented Nov 9, 2019

jborean93 left a comment

jborean93 Nov 11, 2019

martinhoefling Nov 11, 2019

jborean93 Nov 11, 2019

martinhoefling Nov 11, 2019

martinhoefling Nov 11, 2019

jborean93 Nov 11, 2019

jborean93 commented Nov 11, 2019 •

edited

Loading

martinhoefling commented Nov 12, 2019

jborean93 left a comment •

edited

Loading

martinhoefling commented Nov 18, 2019 •

edited

Loading

jborean93 left a comment

jborean93 commented Nov 18, 2019

martinhoefling commented Nov 19, 2019

jborean93 commented Nov 20, 2019 •

edited

Loading

martinhoefling commented Nov 21, 2019

Add various shutil methods #16

Add various shutil methods #16

Conversation

martinhoefling commented Nov 8, 2019

coveralls commented Nov 8, 2019 • edited Loading

jborean93 commented Nov 8, 2019

martinhoefling commented Nov 8, 2019

martinhoefling commented Nov 9, 2019

martinhoefling commented Nov 9, 2019

jborean93 left a comment

Choose a reason for hiding this comment

jborean93 Nov 11, 2019

Choose a reason for hiding this comment

martinhoefling Nov 11, 2019

Choose a reason for hiding this comment

jborean93 Nov 11, 2019

Choose a reason for hiding this comment

martinhoefling Nov 11, 2019

Choose a reason for hiding this comment

martinhoefling Nov 11, 2019

Choose a reason for hiding this comment

jborean93 Nov 11, 2019

Choose a reason for hiding this comment

jborean93 commented Nov 11, 2019 • edited Loading

martinhoefling commented Nov 12, 2019

jborean93 left a comment • edited Loading

Choose a reason for hiding this comment

martinhoefling commented Nov 18, 2019 • edited Loading

jborean93 left a comment

Choose a reason for hiding this comment

jborean93 commented Nov 18, 2019

martinhoefling commented Nov 19, 2019

jborean93 commented Nov 20, 2019 • edited Loading

martinhoefling commented Nov 21, 2019

coveralls commented Nov 8, 2019 •

edited

Loading

jborean93 commented Nov 11, 2019 •

edited

Loading

jborean93 left a comment •

edited

Loading

martinhoefling commented Nov 18, 2019 •

edited

Loading

jborean93 commented Nov 20, 2019 •

edited

Loading