msabramo / requests-unixsocket

Use requests to talk HTTP via a UNIX domain socket
Apache License 2.0
207 stars 29 forks source link

Improve the monkey-patching library to replicate requests more closely #12

Closed wrouesnel closed 8 years ago

wrouesnel commented 9 years ago

This patch modifies the monkey-patch library to properly patch all the requests built-in methods, providing a complete drop-in replacement.

This also means the library exports the requests methods of requests_unixsocket directly, following the usual requests exports.

The redundant all line of the main module is removed for code clean-up.

msabramo commented 9 years ago

Thanks for your contribution! This looks interesting but it's not PEP 8 compliant. Should be pretty easy for you to fix up with either autopep8 or manually.

wrouesnel commented 9 years ago

Should be PEP8 compliant now I believe.

msabramo commented 9 years ago

Thanks for working on this. I'll try to take a look soon.

msabramo commented 9 years ago

Hmm, flake8 still flagged things -- see https://travis-ci.org/msabramo/requests-unixsocket/jobs/57738245

What tool did you use to check pep8? flake8 is what I generally use.

msabramo commented 9 years ago

Also, are you willing to add tests for your additions?

msabramo commented 9 years ago

Ideally the changes you make should pass tox tests for all Python versions and the flake8 check -- e.g.:

$ pip install tox
...
$ tox
...
  py26: commands succeeded
  py27: commands succeeded
  py32: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
  pypy: commands succeeded
  flake8: commands succeeded
  congratulations :)

For your change, I get:

$ tox
...
flake8 inst-nodeps: /Users/marca/dev/git-repos/requests-unixsocket/.tox/dist/requests-unixsocket-0.1.4.post2.zip
flake8 runtests: PYTHONHASHSEED='3651994363'
flake8 runtests: commands[0] | flake8
./requests_unixsocket/__init__.py:8:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:13:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:17:1: W293 blank line contains whitespace
./requests_unixsocket/__init__.py:19:58: W291 trailing whitespace
./requests_unixsocket/__init__.py:22:34: E201 whitespace after '('
./requests_unixsocket/__init__.py:22:37: E231 missing whitespace after ','
./requests_unixsocket/__init__.py:22:80: E501 line too long (82 > 79 characters)
./requests_unixsocket/__init__.py:22:81: E202 whitespace before ')'
./requests_unixsocket/__init__.py:40:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:44:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:48:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:52:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:53:38: F821 undefined name 'data'
./requests_unixsocket/__init__.py:55:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:56:40: F821 undefined name 'data'
./requests_unixsocket/__init__.py:58:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:59:37: F821 undefined name 'data'
./requests_unixsocket/__init__.py:61:1: E302 expected 2 blank lines, found 1
./requests_unixsocket/__init__.py:64:1: E302 expected 2 blank lines, found 1
ERROR: InvocationError: '/Users/marca/dev/git-repos/requests-unixsocket/.tox/flake8/bin/flake8'
___________________________________________________________________________________ summary ____________________________________________________________________________________
  py26: commands succeeded
  py27: commands succeeded
  py32: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
  pypy: commands succeeded
ERROR:   flake8: commands failed
msabramo commented 9 years ago

And once again, thanks for working on this; I appreciate you taking the time to work on this and to send PRs!

msabramo commented 9 years ago

See https://github.com/wrouesnel/requests-unixsocket/pull/1 -- that does some of the minor stuff. It would still be great if you could add tests for your change.

esben commented 9 years ago

:+1: for getting this merged

wrouesnel commented 9 years ago

I'm coding up some tests for the methods now too.

msabramo commented 9 years ago

Awesome! Much appreciated!

szaher commented 9 years ago

is it working with HTTPS requests too ?