Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve default total client timeout value #7275

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/7274.feature
@@ -0,0 +1 @@
Changed default value for ``ClientTimeout.total`` so that the default is always present, even if not explicitly specified.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Expand Up @@ -351,6 +351,7 @@ Ye Cao
Yegor Roganov
Yifei Kong
Young-Ho Cha
Yuri Sukhov
Yuriy Shatrov
Yury Pliner
Yury Selivanov
Expand Down
9 changes: 3 additions & 6 deletions aiohttp/client.py
Expand Up @@ -31,7 +31,7 @@
)

from multidict import CIMultiDict, MultiDict, MultiDictProxy, istr
from typing_extensions import Final, final
from typing_extensions import final
from yarl import URL

from . import hdrs, http, payload
Expand Down Expand Up @@ -134,7 +134,7 @@

@dataclasses.dataclass(frozen=True)
class ClientTimeout:
total: Optional[float] = None
total: Optional[float] = 5 * 60 # 5 minute default timeout
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, is this the expected behaviour if a user were to do something like
ClientTimeout(sock_read=600)?

i.e. They set sock_read to 10 mins, but total is still set to 5 mins, so it achieves nothing.

I'm not sure now if having this default value makes things simpler or more complex...
@webknjaz Any thoughts?

connect: Optional[float] = None
sock_read: Optional[float] = None
sock_connect: Optional[float] = None
Expand All @@ -154,9 +154,6 @@ class ClientTimeout:
# to overwrite the defaults


# 5 Minute default read timeout
DEFAULT_TIMEOUT: Final[ClientTimeout] = ClientTimeout(total=5 * 60)

_RetType = TypeVar("_RetType")


Expand Down Expand Up @@ -257,7 +254,7 @@ def __init__(
self._version = version
self._json_serialize = json_serialize
if timeout is sentinel or timeout is None:
self._timeout = DEFAULT_TIMEOUT
self._timeout = ClientTimeout()
else:
self._timeout = timeout
self._raise_for_status = raise_for_status
Expand Down
4 changes: 2 additions & 2 deletions docs/client_reference.rst
Expand Up @@ -1684,7 +1684,7 @@ Utilities
ClientTimeout
^^^^^^^^^^^^^

.. class:: ClientTimeout(*, total=None, connect=None, \
.. class:: ClientTimeout(*, total=5*60, connect=None, \
sock_connect=None, sock_read=None)

A data class for client timeout settings.
Expand All @@ -1695,7 +1695,7 @@ ClientTimeout

Total number of seconds for the whole request.

:class:`float`, ``None`` by default.
:class:`float`, 300 seconds (5 min) by default.

.. attribute:: connect

Expand Down
2 changes: 1 addition & 1 deletion tests/test_client_session.py
Expand Up @@ -790,7 +790,7 @@ async def test_client_session_custom_attr() -> None:

async def test_client_session_timeout_default_args(loop: Any) -> None:
session1 = ClientSession()
assert session1.timeout == client.DEFAULT_TIMEOUT
assert session1.timeout == client.ClientTimeout(total=5 * 60)
await session1.close()


Expand Down