From 71579a1454e9ff2ba192273ce07017a5385cdb20 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Estival Date: Fri, 19 Nov 2021 18:36:27 +0100 Subject: [PATCH] Fixes and unit tests. --- CHANGES/{6008.misc => 6316.misc} | 0 CONTRIBUTORS.txt | 1 + aiohttp/web_protocol.py | 10 ++++---- aiohttp/web_server.py | 11 ++++++++- docs/client_quickstart.rst | 9 ++++++-- docs/web_advanced.rst | 19 ++++++++++++++++ tests/test_helpers.py | 39 +++++++++++++++++++++++++++++++- tests/test_web_runner.py | 30 ++++++++++++++++++++++++ 8 files changed, 110 insertions(+), 9 deletions(-) rename CHANGES/{6008.misc => 6316.misc} (100%) diff --git a/CHANGES/6008.misc b/CHANGES/6316.misc similarity index 100% rename from CHANGES/6008.misc rename to CHANGES/6316.misc diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index f1a3be11fc9..7c72a8f346c 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -158,6 +158,7 @@ Jakob Ackermann Jakub Wilk Jan Buchar Jashandeep Sohi +Jean-Baptiste Estival Jens Steinhauser Jeonghun Lee Jeongkyu Shin diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index c059a85084c..bcb2e7acf96 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -144,9 +144,9 @@ class RequestHandler(BaseProtocol): max_headers -- Optional maximum header size - timeout_ceil_threshold -- Optional threshold value - for to trigger ceiling of - timeout event values + timeout_ceil_threshold -- Optional value to specify + threshold to ceil() timeout + values """ @@ -236,8 +236,8 @@ def __init__( self._timeout_ceil_threshold = 5 # type: float try: - self._timeout_ceil_threshold = timeout_ceil_threshold - except ValueError: + self._timeout_ceil_threshold = float(timeout_ceil_threshold) + except (TypeError, ValueError): pass self.logger = logger diff --git a/aiohttp/web_server.py b/aiohttp/web_server.py index 1c9fbf34ca8..6998fff34ae 100644 --- a/aiohttp/web_server.py +++ b/aiohttp/web_server.py @@ -65,4 +65,13 @@ async def shutdown(self, timeout: Optional[float] = None) -> None: self._connections.clear() def __call__(self) -> RequestHandler: - return RequestHandler(self, loop=self._loop, **self._kwargs) + try: + return RequestHandler(self, loop=self._loop, **self._kwargs) + except TypeError: + # Failsafe creation: remove all custom handler_args + kwargs = { + k: v + for k, v in self._kwargs.items() + if k in ["debug", "access_log_class"] + } + return RequestHandler(self, loop=self._loop, **kwargs) diff --git a/docs/client_quickstart.rst b/docs/client_quickstart.rst index 381b9119624..29981d6d3ea 100644 --- a/docs/client_quickstart.rst +++ b/docs/client_quickstart.rst @@ -444,13 +444,17 @@ Supported :class:`ClientTimeout` fields are: The maximal number of seconds allowed for period between reading a new data portion from a peer. + ``ceil_threshold`` + + The threshold value to trigger ceiling of absolute timeout values. + All fields are floats, ``None`` or ``0`` disables a particular timeout check, see the :class:`ClientTimeout` reference for defaults and additional details. Thus the default timeout is:: aiohttp.ClientTimeout(total=5*60, connect=None, - sock_connect=None, sock_read=None) + sock_connect=None, sock_read=None, ceil_threshold=5) .. note:: @@ -467,4 +471,5 @@ Thus the default timeout is:: timeout expiration. Smaller timeouts are not rounded to help testing; in the real life network - timeouts usually greater than tens of seconds. + timeouts usually greater than tens of seconds. However, the default threshold + value of 5 seconds can be configured using the ``ceil_threshold`` parameter. diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst index 0476b5f1faf..4ed09af58f6 100644 --- a/docs/web_advanced.rst +++ b/docs/web_advanced.rst @@ -834,6 +834,25 @@ Signal handler may look like:: Both :func:`run_app` and :meth:`AppRunner.cleanup` call shutdown signal handlers. +.. _aiohttp-web-ceil-absolute-timeout: + +Ceil of absolute timeout value +------------------------------ + +*aiohttp* **ceils** internal timeout values if the value is equal or +greater than 5 seconds. The timeout expires at the next integer second +greater than ``current_time + timeout``. + +More details about ceiling absolute timeout values is available here +:ref:`aiohttp-client-timeouts`. + +The default threshold can be configured at :class:`aiohttp.web.Application` +level using the ``handler_args`` parameter. + +.. code-block:: python3 + + app = web.Application(handler_args={"timeout_ceil_threshold": 1}) + .. _aiohttp-web-background-tasks: Background tasks diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 6c56e5d3aae..9cfebc7b3a6 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -4,7 +4,8 @@ import datetime import gc import platform -from math import isclose, modf +import weakref +from math import ceil, isclose, modf from unittest import mock from urllib.request import getproxies_environment @@ -352,6 +353,18 @@ def test_when_timeout_smaller_second(loop) -> None: assert isclose(when - timer, 0, abs_tol=0.001) +def test_when_timeout_smaller_second_with_low_threshold(loop) -> None: + timeout = 0.1 + timer = loop.time() + timeout + + handle = helpers.TimeoutHandle(loop, timeout, 0.01) + when = handle.start()._when + handle.close() + + assert isinstance(when, int) + assert when == ceil(timer) + + def test_timeout_handle_cb_exc(loop) -> None: handle = helpers.TimeoutHandle(loop, 10.2) cb = mock.Mock() @@ -389,6 +402,16 @@ async def test_weakref_handle(loop) -> None: assert cb.test.called +async def test_weakref_handle_with_small_threshold(loop) -> None: + cb = mock.Mock() + loop = mock.Mock() + loop.time.return_value = 10 + helpers.weakref_handle(cb, "test", 0.1, loop, 0.01) + loop.call_at.assert_called_with( + 11, helpers._weakref_handle, (weakref.ref(cb), "test") + ) + + async def test_weakref_handle_weak(loop) -> None: cb = mock.Mock() helpers.weakref_handle(cb, "test", 0.01, loop) @@ -408,6 +431,14 @@ def test_ceil_call_later() -> None: loop.call_at.assert_called_with(21.0, cb) +def test_ceil_call_later_with_small_threshold() -> None: + cb = mock.Mock() + loop = mock.Mock() + loop.time.return_value = 10.1 + helpers.call_later(cb, 4.5, loop, 1) + loop.call_at.assert_called_with(15, cb) + + def test_ceil_call_later_no_timeout() -> None: cb = mock.Mock() loop = mock.Mock() @@ -433,6 +464,12 @@ async def test_ceil_timeout_small(loop) -> None: assert frac != 0 +async def test_ceil_timeout_small_with_overriden_threshold(loop) -> None: + async with helpers.ceil_timeout(1.5, ceil_threshold=1) as cm: + frac, integer = modf(cm.deadline) + assert frac == 0 + + # -------------------------------- ContentDisposition ------------------- diff --git a/tests/test_web_runner.py b/tests/test_web_runner.py index 48ec3944337..82965432890 100644 --- a/tests/test_web_runner.py +++ b/tests/test_web_runner.py @@ -113,6 +113,36 @@ def test_app_handler_args() -> None: assert runner._kwargs == {"access_log_class": web.AccessLogger, "test": True} +async def test_app_handler_args_failure() -> None: + app = web.Application(handler_args={"unknown_parameter": 5}) + runner = web.AppRunner(app) + await runner.setup() + assert runner._server + rh = runner._server() + assert rh._timeout_ceil_threshold == 5 + await runner.cleanup() + assert app + + +@pytest.mark.parametrize( + ("value", "expected"), + ( + (2, 2), + (None, 5), + ("2", 2), + ), +) +async def test_app_handler_args_ceil_threshold(value: Any, expected: Any) -> None: + app = web.Application(handler_args={"timeout_ceil_threshold": value}) + runner = web.AppRunner(app) + await runner.setup() + assert runner._server + rh = runner._server() + assert rh._timeout_ceil_threshold == expected + await runner.cleanup() + assert app + + async def test_app_make_handler_access_log_class_bad_type1() -> None: class Logger: pass