Skip to content

Commit

Permalink
fix Issue aio-libs#1861: Client request timeout parameters are incons…
Browse files Browse the repository at this point in the history
…istent

Use sentinel (instead of None) for checking the arguments passed.

aiohttp/client.py:
* ClientSession.__init__():
  * set the `read_timeout` argument default value to `sentinel`
  * if it is equal to `sentinel` during the call, set it to the value of `DEFAULT_TIMEOUT`
* ClientSession._request():
  * set the `timeout` argument default value to `sentinel`
  * if it is equal to `sentinel` during the call, set it to the value of `self.read_timeout`

Update documentation to match the consistent usage of timeouts.
Update tests (excluding the confusing `timeout=None` argument usage).
  • Loading branch information
ykshatroff committed Apr 30, 2017
1 parent 9522ba6 commit ba9201e
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 13 deletions.
13 changes: 8 additions & 5 deletions aiohttp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
from .connector import * # noqa
from .connector import TCPConnector
from .cookiejar import CookieJar
from .helpers import PY_35, CeilTimeout, TimeoutHandle, deprecated_noop
from .helpers import (PY_35, CeilTimeout, TimeoutHandle,
deprecated_noop, sentinel)
from .http import WS_KEY, WebSocketReader, WebSocketWriter
from .streams import FlowControlDataQueue

Expand Down Expand Up @@ -52,7 +53,7 @@ def __init__(self, *, connector=None, loop=None, cookies=None,
ws_response_class=ClientWebSocketResponse,
version=http.HttpVersion11,
cookie_jar=None, connector_owner=True, raise_for_status=False,
read_timeout=None, conn_timeout=None):
read_timeout=sentinel, conn_timeout=None):

implicit_loop = False
if loop is None:
Expand Down Expand Up @@ -96,7 +97,8 @@ def __init__(self, *, connector=None, loop=None, cookies=None,
self._default_auth = auth
self._version = version
self._json_serialize = json_serialize
self._read_timeout = read_timeout
self._read_timeout = (read_timeout if read_timeout is not sentinel
else DEFAULT_TIMEOUT)
self._conn_timeout = conn_timeout
self._raise_for_status = raise_for_status

Expand Down Expand Up @@ -149,7 +151,7 @@ def _request(self, method, url, *,
read_until_eof=True,
proxy=None,
proxy_auth=None,
timeout=DEFAULT_TIMEOUT):
timeout=sentinel):

# NOTE: timeout clamps existing connect and read timeouts. We cannot
# set the default to None because we need to detect if the user wants
Expand Down Expand Up @@ -201,7 +203,8 @@ def _request(self, method, url, *,
# timeout is cumulative for all request operations
# (request, redirects, responses, data consuming)
tm = TimeoutHandle(
self._loop, timeout if timeout is not None else self._read_timeout)
self._loop,
timeout if timeout is not sentinel else self._read_timeout)
handle = tm.start()

timer = tm.timer()
Expand Down
14 changes: 7 additions & 7 deletions docs/client_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ The client session supports the context manager protocol for self closing.

.. versionadded:: 2.0

:param float read_timeout: Request operations timeout. read_timeout is
:param float read_timeout: Request operations timeout. ``read_timeout`` is
cumulative for all request operations (request, redirects, responses,
data consuming)
data consuming). By default, the read timeout is 5*60 seconds.
Use ``None`` or ``0`` to disable timeout checks.

:param float conn_timeout: timeout for connection establishing
(optional). Values ``0`` or ``None`` mean no timeout.
Expand Down Expand Up @@ -240,8 +241,7 @@ The client session supports the context manager protocol for self closing.
:param aiohttp.BasicAuth proxy_auth: an object that represents proxy HTTP
Basic Authorization (optional)

:param int timeout: a timeout for IO operations, 5min by default.
Use ``None`` or ``0`` to disable timeout checks.
:param int timeout: override the session's timeout (``read_timeout``) for IO operations.

:return ClientResponse: a :class:`client response <ClientResponse>` object.

Expand Down Expand Up @@ -714,7 +714,7 @@ TCPConnector
server presents matches. Useful for `certificate pinning
<https://en.wikipedia.org/wiki/Transport_Layer_Security#Certificate_pinning>`_.

Note: use of MD5 or SHA1 digests is insecure and deprecated.
Note: use of MD5 or SHA1 digests is insecure and deprecated.

.. versionadded:: 0.16

Expand All @@ -735,7 +735,7 @@ TCPConnector
means cached forever. By default 10 seconds.

By default DNS entries are cached forever, in some environments the IP
addresses related to a specific HOST can change after a specific time. Use
addresses related to a specific HOST can change after a specific time. Use
this option to keep the DNS cache updated refreshing each entry after N
seconds.

Expand Down Expand Up @@ -1372,7 +1372,7 @@ Hierarchy of exceptions:
- `aiohttp.ClientOSError` - subset of connection errors that are initiated by an OSError exception

- `aiohttp.ClientConnectorError` - connector related exceptions

- `aiohttp.ClientProxyConnectionError` - proxy connection initialization error

- `aiohttp.ServerConnectionError` - server connection related errors
Expand Down
2 changes: 1 addition & 1 deletion tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def handler(request):
client = yield from test_client(app, connector=conn, read_timeout=0.01)

with pytest.raises(asyncio.TimeoutError):
yield from client.get('/', timeout=None)
yield from client.get('/')


@asyncio.coroutine
Expand Down

0 comments on commit ba9201e

Please sign in to comment.