Skip to content

Commit

Permalink
Improve CI stability for timeouts and branch coverage (#1554)
Browse files Browse the repository at this point in the history
  • Loading branch information
pquentin authored and sethmlarson committed Mar 23, 2019
1 parent cbe558b commit edfd345
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 21 deletions.
9 changes: 5 additions & 4 deletions src/urllib3/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ class BaseSSLError(BaseException):
pass


try: # Python 3:
# Not a no-op, we're adding this to the namespace so it can be imported.
try:
# Python 3: not a no-op, we're adding this to the namespace so it can be imported.
ConnectionError = ConnectionError
except NameError: # Python 2:
except NameError:
# Python 2
class ConnectionError(Exception):
pass

Expand Down Expand Up @@ -101,7 +102,7 @@ class HTTPConnection(_HTTPConnection, object):
is_verified = False

def __init__(self, *args, **kw):
if six.PY3: # Python 3
if six.PY3:
kw.pop('strict', None)

# Pre-set source_address.
Expand Down
6 changes: 4 additions & 2 deletions src/urllib3/connectionpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,11 @@ def _make_request(self, conn, method, url, timeout=_Default, chunked=False,

# Receive the response from the server
try:
try: # Python 2.7, use buffering of HTTP responses
try:
# Python 2.7, use buffering of HTTP responses
httplib_response = conn.getresponse(buffering=True)
except TypeError: # Python 3
except TypeError:
# Python 3
try:
httplib_response = conn.getresponse()
except Exception as e:
Expand Down
5 changes: 3 additions & 2 deletions src/urllib3/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,10 @@ def from_httplib(ResponseCls, r, **response_kw):
headers = r.msg

if not isinstance(headers, HTTPHeaderDict):
if PY3: # Python 3
if PY3:
headers = HTTPHeaderDict(headers.items())
else: # Python 2
else:
# Python 2.7
headers = HTTPHeaderDict.from_httplib(headers)

# HTTPResponse objects in Python 3 don't have a .strict attribute
Expand Down
3 changes: 2 additions & 1 deletion src/urllib3/util/timeout.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ def _validate_timeout(cls, value, name):
raise ValueError("Attempted to set %s timeout to %s, but the "
"timeout cannot be set to a value less "
"than or equal to 0." % (name, value))
except TypeError: # Python 3
except TypeError:
# Python 3
raise ValueError("Timeout value %s was %s, but it must be an "
"int, float or None." % (name, value))

Expand Down
2 changes: 2 additions & 0 deletions test/with_dummyserver/test_connectionpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ def test_total_timeout(self):
self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/')

def test_create_connection_timeout(self):
self.start_basic_handler(block_send=Event(), num=0) # needed for self.port

timeout = Timeout(connect=SHORT_TIMEOUT, total=LONG_TIMEOUT)
pool = HTTPConnectionPool(TARPIT_HOST, self.port, timeout=timeout, retries=False)
self.addCleanup(pool.close)
Expand Down
2 changes: 1 addition & 1 deletion test/with_dummyserver/test_https.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ def test_https_timeout(self):
self.addCleanup(https_pool.close)
self.assertRaises(ConnectTimeoutError, https_pool.request, 'GET', '/')

timeout = Timeout(read=0.001)
timeout = Timeout(read=0.01)
https_pool = HTTPSConnectionPool(self.host, self.port,
timeout=timeout, retries=False,
cert_reqs='CERT_REQUIRED')
Expand Down
22 changes: 11 additions & 11 deletions test/with_dummyserver/test_socketlevel.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ def socket_handler(listener):

self._start_server(socket_handler)
http = HTTPConnectionPool(self.host, self.port,
timeout=0.001,
timeout=0.01,
retries=False,
maxsize=3,
block=True)
Expand All @@ -394,7 +394,7 @@ def socket_handler(listener):
sock.close()

self._start_server(socket_handler)
pool = HTTPConnectionPool(self.host, self.port, timeout=0.001, retries=True)
pool = HTTPConnectionPool(self.host, self.port, timeout=0.01, retries=True)
self.addCleanup(pool.close)

try:
Expand All @@ -415,7 +415,7 @@ def socket_handler(listener):
sock.close()

self._start_server(socket_handler)
pool = HTTPSConnectionPool(self.host, self.port, timeout=0.001, retries=False)
pool = HTTPSConnectionPool(self.host, self.port, timeout=0.01, retries=False)
self.addCleanup(pool.close)
try:
self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/')
Expand Down Expand Up @@ -454,7 +454,7 @@ def socket_handler(listener):

try:
self._start_server(socket_handler)
t = Timeout(connect=0.001, read=0.001)
t = Timeout(connect=0.001, read=0.01)
pool = HTTPConnectionPool(self.host, self.port, timeout=t)
self.addCleanup(pool.close)

Expand Down Expand Up @@ -487,7 +487,7 @@ def socket_handler(listener):
self.addCleanup(pool.close)

response = pool.urlopen('GET', '/', retries=0, preload_content=False,
timeout=Timeout(connect=1, read=0.001))
timeout=Timeout(connect=1, read=0.01))
try:
self.assertRaises(ReadTimeoutError, response.read)
finally:
Expand Down Expand Up @@ -517,7 +517,7 @@ def socket_handler(listener):
try:
self.assertRaises(ReadTimeoutError, pool.urlopen,
'GET', '/', retries=False,
timeout=Timeout(connect=1, read=0.001))
timeout=Timeout(connect=1, read=0.01))
finally:
timed_out.set()

Expand Down Expand Up @@ -614,7 +614,7 @@ def socket_handler(listener):
with HTTPConnectionPool(self.host, self.port) as pool:
poolsize = pool.pool.qsize()
response = pool.urlopen('GET', '/', retries=0, preload_content=False,
timeout=Timeout(connect=1, read=0.001))
timeout=Timeout(connect=1, read=0.01))
try:
self.assertRaises(ReadTimeoutError, response.read)
self.assertEqual(poolsize, pool.pool.qsize())
Expand Down Expand Up @@ -712,7 +712,7 @@ def socket_handler(listener):
# Second should succeed.
response = pool.urlopen('GET', '/', retries=0,
preload_content=False,
timeout=Timeout(connect=1, read=0.1))
timeout=Timeout(connect=1, read=1))
self.assertEqual(len(response.read()), 8)

def test_closing_response_actually_closes_connection(self):
Expand Down Expand Up @@ -803,7 +803,7 @@ def socket_handler(listener):
# save it.
response = pool.urlopen('GET', '/', retries=1,
release_conn=False, preload_content=False,
timeout=Timeout(connect=1, read=0.001))
timeout=Timeout(connect=1, read=0.01))

# The connection should still be on the response object, and none
# should be in the pool. We opened two though.
Expand Down Expand Up @@ -1093,7 +1093,7 @@ def socket_handler(listener):
self.addCleanup(pool.close)

response = pool.urlopen('GET', '/', retries=0, preload_content=False,
timeout=Timeout(connect=1, read=0.001))
timeout=Timeout(connect=1, read=0.01))
try:
self.assertRaises(ReadTimeoutError, response.read)
finally:
Expand Down Expand Up @@ -1127,7 +1127,7 @@ def request():
assert_fingerprint=fingerprint)
try:
response = pool.urlopen('GET', '/', preload_content=False,
timeout=Timeout(connect=1, read=0.001),
timeout=Timeout(connect=1, read=0.01),
retries=0)
response.read()
finally:
Expand Down

0 comments on commit edfd345

Please sign in to comment.