pywinrm transport using requests about pywinrm HOT 11 CLOSED

@cchurch I did a code review and in general don't see any major issues with style, naming and general strucutre.

Maybe only one thing I would really would like to change is old stype string formatting into str.format

from pywinrm.

I am for switching to requests. I agree with the principal that the less third party modules the better. However requests makes sense.

In our company all new code we do uses requests and old code gets cut over as needed.

On Nov 22, 2014, at 3:09 PM, Alexey Diyan [email protected] wrote:

Below is email that I've got from @cchurch.

I'm not entirely sure what approach we should follow - try to stick with Python stdlib or use requests.

On the one hand we may keep dependencies as less as possible and may even consider to concatenate all sources into single super-module like I saw in defnull/bottle and amoffat/sh.

It could be very convenient for Linux system administrators just do a wget/curl and start using this library at almost any Linux box right away.

On the other hand we may start to use super popular library that everyone knows which will make some things for us much simpler. Code will be simpler than now and we will have less warries on Python 2/3 compatibility at transport level.

I would happy to hear any opinions, especially from people thad did contributions () .

@cchurch:
I've taken a shot at rewriting the transport class to use requests (and requests-kerberos and a modified copy of requests-ntlm) for the following reasons:

Keeping the connection alive to avoid reconnecting or re-authenticating for each message
Adding support for NTLM authentication
Trying multiple authentication schemes based on what the server supports (e.g. trying kerberos but falling back to NTLM or basic)
See https://github.com/cchurch/pywinrm/blob/transport-refactor/winrm/requests_transport.py

It works for me but hasn't been extensively tested yet, doesn't fully implement certificate auth, etc.

I would be interested in your feedback and comments - do you think you'd be interested in including these changes in the project (with additional cleanup and tests)?

—
Reply to this email directly or view it on GitHub.

from pywinrm.

I'd really love to see this switched to requests. Requests is clean and easy to work with. NTLM authentication for free would be a big plus for free.

from pywinrm.

Status update.

I took one day off at my job and spent some time on this.

If anyone interested, all changes are here - https://github.com/diyan/pywinrm/commits/requests

Note that code is mostly broken, so please don't waste your time on sending bug reports.

Next steps would be:

• prepare a set of Dockerfiles with all versions of Python and at least Ubuntu/CentOS/Arch
• fix failed build on Travis CI
• reformat all code into 80 column layout
• try fix majority of issues reported by pytest_pep8
• integration testing

from pywinrm.

Update.

• Managed to get pywinrm/requests working with Basic auth
• Introduce BasicAuthDisabledError and InvalidCredentialsError instead of just HTTP 401 error
• Added more detailed tutorial on how to setup WinRM client and server

from pywinrm.

@diyan Thanks for keeping this one moving forward!

from pywinrm.

@diyan I'd also like to see this happen. We are going to be using pywinrm at Rackspace and I generally trust urllib3 (and Requests) more than urllib2. Taking the dependencies is no problem for us, at least.

Is there anything I can help with?

from pywinrm.

Using the requests branch solved an issue I was experiencing with 100s of connections sat in TIME_WAIT and frequent "Connection Refused" errors from pywinrm/ansible. Was that a coincdence, or actively fixed in the changes?

(I got it working by copying the winrm.py connection class from Ansible in to a directory specified by the ansible connection_plugins config option, and just fixed the name of the winrm exceptions)

That's a blocker to me using ansible in production with Windows, so I'm interested in seeing requests making it to master.

from pywinrm.

@diyan Do you have an estimate for when this will land?

from pywinrm.

Yep,

Short story:also waiting for this to land.

Long story: I am facing a problem where our CI runs fail sporadically with "Kerberos-based authentication was failed"-messages . We tried all sorts of "kerberos fixes" found via ggling -but now that I read the source of pywinrm...it seems that all HTTPError&URLError exceptions are pointing the finger at kerberos. So the wrong error messages hinted us in the wrong direction, and actually seems our win-VM replies with code 500(maybe due to overload of some sort). Then found this branch and the issues that are waiting to get solved by it.

from pywinrm.

pywinrm has switched to requests a few releases ago, closing this issue.

from pywinrm.