requests-unixsocket icon indicating copy to clipboard operation
requests-unixsocket copied to clipboard

requests-unixsocket should use the same timeout interface as requests.

Open tomprince opened this issue 5 years ago • 3 comments

It seems like requests-unixsocket currently has its own way of setting timeouts, which differs from requests itself.

  • The default for requests-unixsocket is 60s unlike request's default of no timeout.
  • The timeout is set as a constructor parameter of UnixAdapter, which is inaccessible, if one uses requests_unixsocket.Session.

tomprince avatar Dec 23 '19 18:12 tomprince

I think your second issue may be satisfied with https://github.com/msabramo/requests-unixsocket/pull/42/files (not released yet) and something like:

   session = requests_unixsocket.Session()
   adapter = requests_unixsocket.UnixAdapter(**your_kwargs_here)
   session.mount('http+unix://', adapter)

TomGoBravo avatar Apr 07 '20 17:04 TomGoBravo

https://pypi.org/project/requests-unixsocket/0.3.0/ now has #42 in case that helps.

msabramo avatar Dec 24 '21 01:12 msabramo

It looks to me like the timeout code I have isn't actually taking effect, maybe because requests is overriding it.

I made this small change to add some debugging:

diff --git a/requests_unixsocket/adapters.py b/requests_unixsocket/adapters.py
index 83e1400..42ef5d6 100644
--- a/requests_unixsocket/adapters.py
+++ b/requests_unixsocket/adapters.py
@@ -28,6 +28,7 @@ class UnixHTTPConnection(httplib.HTTPConnection, object):
         super(UnixHTTPConnection, self).__init__('localhost', timeout=timeout)
         self.unix_socket_url = unix_socket_url
         self.timeout = timeout
+        print(f'UnixHTTPConnection.__init__: self.timeout = {self.timeout}')
         self.sock = None

     def __del__(self):  # base class does not have d'tor
@@ -36,6 +37,7 @@ class UnixHTTPConnection(httplib.HTTPConnection, object):

     def connect(self):
         sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+        print(f'UnixHTTPConnection.connect: self.timeout = {self.timeout}')
         sock.settimeout(self.timeout)
         socket_path = unquote(urlparse(self.unix_socket_url).netloc)
         sock.connect(socket_path)

and then I do this:

$ .tox/py39/bin/ipython
Python 3.9.9 (main, Dec 23 2021, 15:32:01)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.30.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import json
   ...: import time
   ...:
   ...: import requests_unixsocket
   ...:
   ...: session = requests_unixsocket.Session()
   ...:
   ...: res = session.get('http+unix://%2FUsers%2Fabramowi%2Fdev%2Fgit-repos%2Frequests-unixsocket%2Fuds_socket/info')
UnixHTTPConnection.__init__: self.timeout = 60
UnixHTTPConnection.connect: self.timeout = None

Notice how my code sets self.timeout to 60 in the UnixHTTPConnection constructor, but by the time it gets to the connect method, where it calls sock.settimeout, self.timeout has been set to None.

The other thing that I'm noticing is that if I specify a timeout to session.get, then it seems to get applied correctly:

$ .tox/py39/bin/ipython
Python 3.9.9 (main, Dec 23 2021, 15:32:01)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.30.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import json
   ...: import time
   ...:
   ...: import requests_unixsocket
   ...:
   ...: session = requests_unixsocket.Session()
   ...:
   ...: res = session.get('http+unix://%2FUsers%2Fabramowi%2Fdev%2Fgit-repos%2Frequests-unixsocket%2Fuds_socket/info', timeout=3)
UnixHTTPConnection.__init__: self.timeout = 60
UnixHTTPConnection.connect: self.timeout = 3
---------------------------------------------------------------------------
...
ReadTimeout: UnixHTTPConnectionPool(host='localhost', port=None): Read timed out. (read timeout=3)

So it seems to me that despite the incorrect-looking code that I have, the right thing seems to be happening (purely by accident :smile:) and therefore I am inclined to simply remove my wrong-looking but ineffective and confusing timeout code and do a new release where I mark this as a breaking change, since I'd be removing the timeout parameter from the constructors for UnixHTTPConnection, UnixHTTPConnectionPool, and UnixAdapter.

See https://github.com/msabramo/requests-unixsocket/pull/62

msabramo avatar Dec 27 '21 22:12 msabramo