From ea6a02bf97616e5eaec8bb45eba42492d28f3418 Mon Sep 17 00:00:00 2001 From: mpa Date: Sun, 3 May 2020 00:53:25 +0400 Subject: [PATCH 1/5] refactor(sessions): remove BaseSession's initializer, add timeout ommitable field to base method model --- aiogram/api/client/session/aiohttp.py | 46 +++++++++------- aiogram/api/client/session/base.py | 52 ++++++++++++------- aiogram/api/methods/base.py | 11 ++++ .../test_session/test_aiohttp_session.py | 16 ++++++ .../test_session/test_base_session.py | 4 +- 5 files changed, 90 insertions(+), 39 deletions(-) diff --git a/aiogram/api/client/session/aiohttp.py b/aiogram/api/client/session/aiohttp.py index c3c1bba5..be5861ce 100644 --- a/aiogram/api/client/session/aiohttp.py +++ b/aiogram/api/client/session/aiohttp.py @@ -3,7 +3,6 @@ from __future__ import annotations from typing import ( Any, AsyncGenerator, - Callable, Dict, Iterable, List, @@ -15,11 +14,11 @@ from typing import ( cast, ) -from aiohttp import BasicAuth, ClientSession, ClientTimeout, FormData, TCPConnector +from aiohttp import BasicAuth, ClientSession, FormData, TCPConnector from aiogram.api.methods import Request, TelegramMethod -from .base import PRODUCTION, BaseSession, TelegramAPIServer +from .base import BaseSession T = TypeVar("T") _ProxyBasic = Union[str, Tuple[str, BasicAuth]] @@ -72,34 +71,42 @@ def _prepare_connector(chain_or_plain: _ProxyType) -> Tuple[Type["TCPConnector"] class AiohttpSession(BaseSession): - def __init__( - self, - api: TelegramAPIServer = PRODUCTION, - json_loads: Optional[Callable[..., Any]] = None, - json_dumps: Optional[Callable[..., str]] = None, - proxy: Optional[_ProxyType] = None, - ): - super(AiohttpSession, self).__init__( - api=api, json_loads=json_loads, json_dumps=json_dumps, proxy=proxy - ) + def __init__(self, proxy: Optional[_ProxyType] = None): self._session: Optional[ClientSession] = None self._connector_type: Type[TCPConnector] = TCPConnector self._connector_init: Dict[str, Any] = {} + self._should_reset_connector = True # flag determines connector state + self._proxy: Optional[_ProxyType] = None - if self.proxy: + if proxy is not None: try: - self._connector_type, self._connector_init = _prepare_connector( - cast(_ProxyType, self.proxy) - ) + self._setup_proxy_connector(proxy) except ImportError as exc: # pragma: no cover raise UserWarning( "In order to use aiohttp client for proxy requests, install " "https://pypi.org/project/aiohttp-socks/" ) from exc + def _setup_proxy_connector(self, proxy: _ProxyType) -> None: + self._connector_type, self._connector_init = _prepare_connector(proxy) + self._proxy = proxy + + @property + def proxy(self) -> Optional[_ProxyType]: + return self._proxy + + @proxy.setter + def proxy(self, proxy: _ProxyType) -> None: + self._setup_proxy_connector(proxy) + self._should_reset_connector = True + async def create_session(self) -> ClientSession: + if self._should_reset_connector: + await self.close() + if self._session is None or self._session.closed: self._session = ClientSession(connector=self._connector_type(**self._connector_init)) + self._should_reset_connector = False return self._session @@ -125,7 +132,7 @@ class AiohttpSession(BaseSession): url = self.api.api_url(token=token, method=request.method) form = self.build_form_data(request) - async with session.post(url, data=form) as resp: + async with session.post(url, data=form, timeout=call.request_timeout) as resp: raw_result = await resp.json(loads=self.json_loads) response = call.build_response(raw_result) @@ -136,9 +143,8 @@ class AiohttpSession(BaseSession): self, url: str, timeout: int, chunk_size: int ) -> AsyncGenerator[bytes, None]: session = await self.create_session() - client_timeout = ClientTimeout(total=timeout) - async with session.get(url, timeout=client_timeout) as resp: + async with session.get(url, timeout=timeout) as resp: async for chunk in resp.content.iter_chunked(chunk_size): yield chunk diff --git a/aiogram/api/client/session/base.py b/aiogram/api/client/session/base.py index 83e7f3ff..450b958d 100644 --- a/aiogram/api/client/session/base.py +++ b/aiogram/api/client/session/base.py @@ -16,26 +16,42 @@ PT = TypeVar("PT") class BaseSession(abc.ABC): - def __init__( - self, - api: Optional[TelegramAPIServer] = None, - json_loads: Optional[Callable[..., Any]] = None, - json_dumps: Optional[Callable[..., str]] = None, - proxy: Optional[PT] = None, - ) -> None: - if api is None: - api = PRODUCTION - if json_loads is None: - json_loads = json.loads - if json_dumps is None: - json_dumps = json.dumps + _api: TelegramAPIServer + _json_loads: Callable[..., Any] + _json_dumps: Callable[..., str] - self.api = api - self.json_loads = json_loads - self.json_dumps = json_dumps - self.proxy = proxy + @property + def api(self) -> TelegramAPIServer: # pragma: no cover + if not hasattr(self, "_api"): + return PRODUCTION + return self._api - def raise_for_status(self, response: Response[T]) -> None: + @api.setter + def api(self, value: TelegramAPIServer) -> None: # pragma: no cover + self._api = value + + @property + def json_loads(self) -> Callable[..., Any]: # pragma: no cover + if not hasattr(self, "_json_loads"): + return json.loads + return self._json_loads + + @json_loads.setter + def json_loads(self, value: Callable[..., Any]) -> None: # pragma: no cover + self._json_loads = value # type: ignore + + @property + def json_dumps(self) -> Callable[..., str]: # pragma: no cover + if not hasattr(self, "_json_dumps"): + return json.dumps + return self._json_dumps + + @json_dumps.setter + def json_dumps(self, value: Callable[..., str]) -> None: # pragma: no cover + self._json_dumps = value # type: ignore + + @classmethod + def raise_for_status(cls, response: Response[T]) -> None: if response.ok: return raise TelegramAPIError(response.description) diff --git a/aiogram/api/methods/base.py b/aiogram/api/methods/base.py index 72eafa05..fc57a2ff 100644 --- a/aiogram/api/methods/base.py +++ b/aiogram/api/methods/base.py @@ -13,6 +13,7 @@ if TYPE_CHECKING: # pragma: no cover from ..client.bot import Bot T = TypeVar("T") +DEFAULT_REQUEST_TIMEOUT_SECONDS = 60.0 class Request(BaseModel): @@ -55,6 +56,16 @@ class TelegramMethod(abc.ABC, BaseModel, Generic[T]): def build_request(self) -> Request: # pragma: no cover pass + request_timeout: float = DEFAULT_REQUEST_TIMEOUT_SECONDS + + def dict(self, **kwargs: Any) -> Any: + # override dict of pydantic.BaseModel to overcome exporting request_timeout field + exclude = kwargs.pop("exclude", set()) + if isinstance(exclude, set): + exclude.add("request_timeout") + + return super().dict(exclude=exclude, **kwargs) + def build_response(self, data: Dict[str, Any]) -> Response[T]: # noinspection PyTypeChecker return Response[self.__returning__](**data) # type: ignore diff --git a/tests/test_api/test_client/test_session/test_aiohttp_session.py b/tests/test_api/test_client/test_session/test_aiohttp_session.py index 2587a686..e7716f9a 100644 --- a/tests/test_api/test_client/test_session/test_aiohttp_session.py +++ b/tests/test_api/test_client/test_session/test_aiohttp_session.py @@ -83,6 +83,22 @@ class TestAiohttpSession: aiohttp_session = await session.create_session() assert isinstance(aiohttp_session.connector, aiohttp_socks.ChainProxyConnector) + @pytest.mark.asyncio + async def test_reset_connector(self): + session = AiohttpSession() + assert session._should_reset_connector + await session.create_session() + assert session._should_reset_connector is False + await session.close() + assert session._should_reset_connector is False + + assert session.proxy is None + session.proxy = "socks5://auth:auth@proxy.url/" + assert session._should_reset_connector + await session.create_session() + assert session._should_reset_connector is False + await session.close() + @pytest.mark.asyncio async def test_close_session(self): session = AiohttpSession() diff --git a/tests/test_api/test_client/test_session/test_base_session.py b/tests/test_api/test_client/test_session/test_base_session.py index 4c86f9da..9d775123 100644 --- a/tests/test_api/test_client/test_session/test_base_session.py +++ b/tests/test_api/test_client/test_session/test_base_session.py @@ -40,8 +40,10 @@ class TestBaseSession: base="http://example.com/{token}/{method}", file="http://example.com/{token}/file/{path{", ) - session = CustomSession(api=api) + session = CustomSession() + session.api = api assert session.api == api + assert "example.com" in session.api.base def test_prepare_value(self): session = CustomSession() From 2c1de9cdbc991d0988b0b0d84669676af8d39e9b Mon Sep 17 00:00:00 2001 From: mpa Date: Sun, 3 May 2020 01:17:40 +0400 Subject: [PATCH 2/5] dummy commit | run actions From 2adc19724d1e2605dc6ba797da6c5e399031c199 Mon Sep 17 00:00:00 2001 From: mpa Date: Wed, 6 May 2020 00:04:50 +0400 Subject: [PATCH 3/5] refactor(props): simplify some descriptors with tests for them, remove silly "nocovers" --- aiogram/api/client/session/base.py | 31 ++++++++----------- .../test_session/test_base_session.py | 21 +++++++++++++ 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/aiogram/api/client/session/base.py b/aiogram/api/client/session/base.py index 450b958d..b99e9e4b 100644 --- a/aiogram/api/client/session/base.py +++ b/aiogram/api/client/session/base.py @@ -12,42 +12,37 @@ from ...methods import Response, TelegramMethod from ..telegram import PRODUCTION, TelegramAPIServer T = TypeVar("T") -PT = TypeVar("PT") +_JSON_LOADS = Callable[..., Any] +_JSON_DUMPS = Callable[..., str] class BaseSession(abc.ABC): _api: TelegramAPIServer - _json_loads: Callable[..., Any] - _json_dumps: Callable[..., str] + _json_loads: _JSON_LOADS + _json_dumps: _JSON_DUMPS @property - def api(self) -> TelegramAPIServer: # pragma: no cover - if not hasattr(self, "_api"): - return PRODUCTION - return self._api + def api(self) -> TelegramAPIServer: + return getattr(self, "_api", PRODUCTION) # type: ignore @api.setter - def api(self, value: TelegramAPIServer) -> None: # pragma: no cover + def api(self, value: TelegramAPIServer) -> None: self._api = value @property - def json_loads(self) -> Callable[..., Any]: # pragma: no cover - if not hasattr(self, "_json_loads"): - return json.loads - return self._json_loads + def json_loads(self) -> _JSON_LOADS: + return getattr(self, "_json_loads", json.loads) # type: ignore @json_loads.setter - def json_loads(self, value: Callable[..., Any]) -> None: # pragma: no cover + def json_loads(self, value: _JSON_LOADS) -> None: self._json_loads = value # type: ignore @property - def json_dumps(self) -> Callable[..., str]: # pragma: no cover - if not hasattr(self, "_json_dumps"): - return json.dumps - return self._json_dumps + def json_dumps(self) -> _JSON_DUMPS: + return getattr(self, "_json_dumps", json.dumps) # type: ignore @json_dumps.setter - def json_dumps(self, value: Callable[..., str]) -> None: # pragma: no cover + def json_dumps(self, value: _JSON_DUMPS) -> None: self._json_dumps = value # type: ignore @classmethod diff --git a/tests/test_api/test_client/test_session/test_base_session.py b/tests/test_api/test_client/test_session/test_base_session.py index 9d775123..43f1d7d5 100644 --- a/tests/test_api/test_client/test_session/test_base_session.py +++ b/tests/test_api/test_client/test_session/test_base_session.py @@ -1,4 +1,5 @@ import datetime +import json from typing import AsyncContextManager, AsyncGenerator import pytest @@ -35,6 +36,26 @@ class TestBaseSession: session = CustomSession() assert session.api == PRODUCTION + def test_default_props(self): + session = CustomSession() + assert session.api == PRODUCTION + assert session.json_loads == json.loads + assert session.json_dumps == json.dumps + + def custom_loads(*_): + return json.loads + + def custom_dumps(*_): + return json.dumps + + session.json_dumps = custom_dumps + assert session.json_dumps == custom_dumps == session._json_dumps + session.json_loads = custom_loads + assert session.json_loads == custom_loads == session._json_loads + + different_session = CustomSession() + assert all(not hasattr(different_session, attr) for attr in ("_json_loads", "_json_dumps", "_api")) + def test_init_custom_api(self): api = TelegramAPIServer( base="http://example.com/{token}/{method}", From df4ba87dfcce421663d578f68efd7bd2782e8b9c Mon Sep 17 00:00:00 2001 From: mpa Date: Wed, 6 May 2020 02:42:54 +0400 Subject: [PATCH 4/5] feat(timeout): implement (class-bound, instance-bound, request-bound) session timeout for requests. fix docs config, fix aiohttp session docs links. --- aiogram/api/client/session/aiohttp.py | 4 ++- aiogram/api/client/session/base.py | 34 ++++++++++++++----- aiogram/api/methods/base.py | 3 +- docs/api/client/session/aiohttp.md | 4 +-- mkdocs.yml | 1 + .../test_session/test_base_session.py | 25 ++++++++++++-- 6 files changed, 55 insertions(+), 16 deletions(-) diff --git a/aiogram/api/client/session/aiohttp.py b/aiogram/api/client/session/aiohttp.py index be5861ce..774e7186 100644 --- a/aiogram/api/client/session/aiohttp.py +++ b/aiogram/api/client/session/aiohttp.py @@ -132,7 +132,9 @@ class AiohttpSession(BaseSession): url = self.api.api_url(token=token, method=request.method) form = self.build_form_data(request) - async with session.post(url, data=form, timeout=call.request_timeout) as resp: + async with session.post( + url, data=form, timeout=call.request_timeout or self.timeout + ) as resp: raw_result = await resp.json(loads=self.json_loads) response = call.build_response(raw_result) diff --git a/aiogram/api/client/session/base.py b/aiogram/api/client/session/base.py index b99e9e4b..97137e7c 100644 --- a/aiogram/api/client/session/base.py +++ b/aiogram/api/client/session/base.py @@ -4,7 +4,7 @@ import abc import datetime import json from types import TracebackType -from typing import Any, AsyncGenerator, Callable, Optional, Type, TypeVar, Union +from typing import Any, AsyncGenerator, Callable, ClassVar, Optional, Type, TypeVar, Union from aiogram.utils.exceptions import TelegramAPIError @@ -12,14 +12,18 @@ from ...methods import Response, TelegramMethod from ..telegram import PRODUCTION, TelegramAPIServer T = TypeVar("T") -_JSON_LOADS = Callable[..., Any] -_JSON_DUMPS = Callable[..., str] +_JsonLoads = Callable[..., Any] +_JsonDumps = Callable[..., str] class BaseSession(abc.ABC): + # global session timeout + default_timeout: ClassVar[float] = 60.0 + _api: TelegramAPIServer - _json_loads: _JSON_LOADS - _json_dumps: _JSON_DUMPS + _json_loads: _JsonLoads + _json_dumps: _JsonDumps + _timeout: float @property def api(self) -> TelegramAPIServer: @@ -30,21 +34,33 @@ class BaseSession(abc.ABC): self._api = value @property - def json_loads(self) -> _JSON_LOADS: + def json_loads(self) -> _JsonLoads: return getattr(self, "_json_loads", json.loads) # type: ignore @json_loads.setter - def json_loads(self, value: _JSON_LOADS) -> None: + def json_loads(self, value: _JsonLoads) -> None: self._json_loads = value # type: ignore @property - def json_dumps(self) -> _JSON_DUMPS: + def json_dumps(self) -> _JsonDumps: return getattr(self, "_json_dumps", json.dumps) # type: ignore @json_dumps.setter - def json_dumps(self, value: _JSON_DUMPS) -> None: + def json_dumps(self, value: _JsonDumps) -> None: self._json_dumps = value # type: ignore + @property + def timeout(self) -> float: + return getattr(self, "_timeout", self.__class__.default_timeout) # type: ignore + + @timeout.setter + def timeout(self, value: float) -> None: + self._timeout = value + + @timeout.deleter + def timeout(self) -> None: + del self._timeout + @classmethod def raise_for_status(cls, response: Response[T]) -> None: if response.ok: diff --git a/aiogram/api/methods/base.py b/aiogram/api/methods/base.py index fc57a2ff..78158192 100644 --- a/aiogram/api/methods/base.py +++ b/aiogram/api/methods/base.py @@ -13,7 +13,6 @@ if TYPE_CHECKING: # pragma: no cover from ..client.bot import Bot T = TypeVar("T") -DEFAULT_REQUEST_TIMEOUT_SECONDS = 60.0 class Request(BaseModel): @@ -56,7 +55,7 @@ class TelegramMethod(abc.ABC, BaseModel, Generic[T]): def build_request(self) -> Request: # pragma: no cover pass - request_timeout: float = DEFAULT_REQUEST_TIMEOUT_SECONDS + request_timeout: Optional[float] = None def dict(self, **kwargs: Any) -> Any: # override dict of pydantic.BaseModel to overcome exporting request_timeout field diff --git a/docs/api/client/session/aiohttp.md b/docs/api/client/session/aiohttp.md index 9eab5ede..223ad468 100644 --- a/docs/api/client/session/aiohttp.md +++ b/docs/api/client/session/aiohttp.md @@ -1,6 +1,6 @@ # Aiohttp session -AiohttpSession represents a wrapper-class around `ClientSession` from [aiohttp]('https://pypi.org/project/aiohttp/') +AiohttpSession represents a wrapper-class around `ClientSession` from [aiohttp](https://pypi.org/project/aiohttp/ "PyPi repository"){target=_blank} Currently `AiohttpSession` is a default session used in `aiogram.Bot` @@ -17,7 +17,7 @@ Bot('token', session=session) ## Proxy requests in AiohttpSession -In order to use AiohttpSession with proxy connector you have to install [aiohttp-socks]('https://pypi.org/project/aiohttp-socks/') +In order to use AiohttpSession with proxy connector you have to install [aiohttp-socks](https://pypi.org/project/aiohttp-socks/ "PyPi repository"){target=_blank} Binding session to bot: ```python diff --git a/mkdocs.yml b/mkdocs.yml index 0d77e640..527d0561 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -35,6 +35,7 @@ markdown_extensions: - pymdownx.inlinehilite - markdown_include.include: base_path: docs + - attr_list nav: - index.md diff --git a/tests/test_api/test_client/test_session/test_base_session.py b/tests/test_api/test_client/test_session/test_base_session.py index 43f1d7d5..35dcfa8e 100644 --- a/tests/test_api/test_client/test_session/test_base_session.py +++ b/tests/test_api/test_client/test_session/test_base_session.py @@ -54,12 +54,33 @@ class TestBaseSession: assert session.json_loads == custom_loads == session._json_loads different_session = CustomSession() - assert all(not hasattr(different_session, attr) for attr in ("_json_loads", "_json_dumps", "_api")) + assert all( + not hasattr(different_session, attr) for attr in ("_json_loads", "_json_dumps", "_api") + ) + + def test_timeout(self): + session = CustomSession() + assert session.timeout == session.default_timeout == CustomSession.default_timeout + + session.default_timeout = float(65.0_0) # mypy will complain + assert session.timeout != session.default_timeout + + CustomSession.default_timeout = float(68.0_0) + assert session.timeout == CustomSession.default_timeout + + session.timeout = float(71.0_0) + assert session.timeout != session.default_timeout + del session.timeout + CustomSession.default_timeout = session.default_timeout + 100 + assert ( + session.timeout != BaseSession.default_timeout + and session.timeout == CustomSession.default_timeout + ) def test_init_custom_api(self): api = TelegramAPIServer( base="http://example.com/{token}/{method}", - file="http://example.com/{token}/file/{path{", + file="http://example.com/{token}/file/{path}", ) session = CustomSession() session.api = api From 0aa8069dc478c32a4adeabfebc170104cf211fae Mon Sep 17 00:00:00 2001 From: mpa Date: Wed, 6 May 2020 03:05:43 +0400 Subject: [PATCH 5/5] fix(sessions): make timeout deleter more clever in particular cases --- aiogram/api/client/session/base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aiogram/api/client/session/base.py b/aiogram/api/client/session/base.py index 97137e7c..8213e4c3 100644 --- a/aiogram/api/client/session/base.py +++ b/aiogram/api/client/session/base.py @@ -59,7 +59,8 @@ class BaseSession(abc.ABC): @timeout.deleter def timeout(self) -> None: - del self._timeout + if hasattr(self, "_timeout"): + del self._timeout @classmethod def raise_for_status(cls, response: Response[T]) -> None: