From deb6cb19b3b45e5c1c3f58f0b91a7770d95e9a53 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sat, 21 Jan 2023 01:45:02 +0530 Subject: [PATCH] Allow to Adjust HTTP Version and Use HTTP/2 by Default (#3506) --- README.rst | 4 ++- README_RAW.rst | 4 ++- requirements.txt | 3 +- telegram/ext/_applicationbuilder.py | 56 +++++++++++++++++++++++++++++ telegram/request/_httpxrequest.py | 14 ++++++++ tests/test_applicationbuilder.py | 22 +++++++++++- tests/test_request.py | 27 ++++++++++++++ 7 files changed, 126 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index b5d8cce26..8a8ff92f3 100644 --- a/README.rst +++ b/README.rst @@ -135,7 +135,9 @@ As these features are *optional*, the corresponding 3rd party dependencies are n Instead, they are listed as optional dependencies. This allows to avoid unnecessary dependency conflicts for users who don't need the optional features. -The only required dependency is `httpx ~= 0.23.3 `_ for ``telegram.request.HTTPXRequest``, the default networking backend. +The only required dependency is `httpx[http2] ~= 0.23.3 `_ for +``telegram.request.HTTPXRequest``, the default networking backend. By default, HTTP/2 is used, as it +provides greater performance and stability, specially for concurrent requests. ``python-telegram-bot`` is most useful when used along with additional libraries. To minimize dependency conflicts, we try to be liberal in terms of version requirements on the (optional) dependencies. diff --git a/README_RAW.rst b/README_RAW.rst index 52822c962..7e7e19dc6 100644 --- a/README_RAW.rst +++ b/README_RAW.rst @@ -136,7 +136,9 @@ As these features are *optional*, the corresponding 3rd party dependencies are n Instead, they are listed as optional dependencies. This allows to avoid unnecessary dependency conflicts for users who don't need the optional features. -The only required dependency is `httpx ~= 0.23.3 `_ for ``telegram.request.HTTPXRequest``, the default networking backend. +The only required dependency is `httpx[http2] ~= 0.23.3 `_ for +``telegram.request.HTTPXRequest``, the default networking backend. By default, HTTP/2 is used, as +it provides greater performance and stability, specially for concurrent requests. ``python-telegram-bot`` is most useful when used along with additional libraries. To minimize dependency conflicts, we try to be liberal in terms of version requirements on the (optional) dependencies. diff --git a/requirements.txt b/requirements.txt index 70e15ca22..5e9ac2913 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,4 +6,5 @@ # versions and only increase the lower bound if necessary # httpx has no stable release yet, so let's be cautious for now -httpx ~= 0.23.3 +# HTTP/2 is more performant and stable than HTTP/1.1, specially for concurrent requests +httpx[http2] ~= 0.23.3 diff --git a/telegram/ext/_applicationbuilder.py b/telegram/ext/_applicationbuilder.py index c2afacb54..998e9428d 100644 --- a/telegram/ext/_applicationbuilder.py +++ b/telegram/ext/_applicationbuilder.py @@ -70,12 +70,14 @@ _BOT_CHECKS = [ ("connect_timeout", "connect_timeout"), ("read_timeout", "read_timeout"), ("write_timeout", "write_timeout"), + ("http_version", "http_version"), ("get_updates_connection_pool_size", "get_updates_connection_pool_size"), ("get_updates_proxy_url", "get_updates_proxy_url"), ("get_updates_pool_timeout", "get_updates_pool_timeout"), ("get_updates_connect_timeout", "get_updates_connect_timeout"), ("get_updates_read_timeout", "get_updates_read_timeout"), ("get_updates_write_timeout", "get_updates_write_timeout"), + ("get_updates_http_version", "get_updates_http_version"), ("base_file_url", "base_file_url"), ("base_url", "base_url"), ("token", "token"), @@ -137,6 +139,7 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): "_get_updates_read_timeout", "_get_updates_request", "_get_updates_write_timeout", + "_get_updates_http_version", "_job_queue", "_persistence", "_pool_timeout", @@ -154,6 +157,7 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): "_updater", "_write_timeout", "_local_mode", + "_http_version", ) def __init__(self: "InitApplicationBuilder"): @@ -174,6 +178,7 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): self._get_updates_write_timeout: ODVInput[float] = DEFAULT_NONE self._get_updates_pool_timeout: ODVInput[float] = DEFAULT_NONE self._get_updates_request: DVInput["BaseRequest"] = DEFAULT_NONE + self._get_updates_http_version: DVInput[str] = DefaultValue("2") self._private_key: ODVInput[bytes] = DEFAULT_NONE self._private_key_password: ODVInput[bytes] = DEFAULT_NONE self._defaults: ODVInput["Defaults"] = DEFAULT_NONE @@ -199,6 +204,7 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): self._post_shutdown: Optional[Callable[[Application], Coroutine[Any, Any, None]]] = None self._post_stop: Optional[Callable[[Application], Coroutine[Any, Any, None]]] = None self._rate_limiter: ODVInput["BaseRateLimiter"] = DEFAULT_NONE + self._http_version: DVInput[str] = DefaultValue("2") def _build_request(self, get_updates: bool) -> BaseRequest: prefix = "_get_updates_" if get_updates else "_" @@ -226,9 +232,12 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): key: value for key, value in timeouts.items() if not isinstance(value, DefaultValue) } + http_version = DefaultValue.get_value(getattr(self, f"{prefix}http_version")) or "2" + return HTTPXRequest( connection_pool_size=connection_pool_size, proxy_url=proxy_url, + http_version=http_version, **effective_timeouts, ) @@ -401,11 +410,18 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): for attr in ("connect_timeout", "read_timeout", "write_timeout", "pool_timeout"): if not isinstance(getattr(self, f"_{prefix}{attr}"), DefaultValue): raise RuntimeError(_TWO_ARGS_REQ.format(name, attr)) + if not isinstance(getattr(self, f"_{prefix}connection_pool_size"), DefaultValue): raise RuntimeError(_TWO_ARGS_REQ.format(name, "connection_pool_size")) + if not isinstance(getattr(self, f"_{prefix}proxy_url"), DefaultValue): raise RuntimeError(_TWO_ARGS_REQ.format(name, "proxy_url")) + + if not isinstance(getattr(self, f"_{prefix}http_version"), DefaultValue): + raise RuntimeError(_TWO_ARGS_REQ.format(name, "http_version")) + self._bot_check(name) + if self._updater not in (DEFAULT_NONE, None): raise RuntimeError(_TWO_ARGS_REQ.format(name, "updater instance")) @@ -543,6 +559,26 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): self._pool_timeout = pool_timeout return self + def http_version(self: BuilderType, http_version: str) -> BuilderType: + """Sets the HTTP protocol version which is used for the + :paramref:`~telegram.request.HTTPXRequest.http_version` parameter of + :attr:`telegram.Bot.request`. By default, HTTP/2 is used. + + .. seealso:: :meth:`get_updates_http_version` + + .. versionadded:: 20.1 + + Args: + http_version (:obj:`str`): Pass ``"1.1"`` if you'd like to use HTTP/1.1 for making + requests to Telegram. Defaults to ``"2"``, in which case HTTP/2 is used. + + Returns: + :class:`ApplicationBuilder`: The same builder with the updated argument. + """ + self._request_param_check(name="http_version", get_updates=False) + self._http_version = http_version + return self + def get_updates_request(self: BuilderType, get_updates_request: BaseRequest) -> BuilderType: """Sets a :class:`telegram.request.BaseRequest` instance for the :paramref:`~telegram.Bot.get_updates_request` parameter of @@ -664,6 +700,26 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]): self._get_updates_pool_timeout = get_updates_pool_timeout return self + def get_updates_http_version(self: BuilderType, get_updates_http_version: str) -> BuilderType: + """Sets the HTTP protocol version which is used for the + :paramref:`~telegram.request.HTTPXRequest.http_version` parameter which is used in the + :meth:`telegram.Bot.get_updates` request. By default, HTTP/2 is used. + + .. seealso:: :meth:`http_version` + + .. versionadded:: 20.1 + + Args: + get_updates_http_version (:obj:`str`): Pass ``"1.1"`` if you'd like to use HTTP/1.1 for + making requests to Telegram. Defaults to ``"2"``, in which case HTTP/2 is used. + + Returns: + :class:`ApplicationBuilder`: The same builder with the updated argument. + """ + self._request_param_check(name="http_version", get_updates=True) + self._get_updates_http_version = get_updates_http_version + return self + def private_key( self: BuilderType, private_key: Union[bytes, FilePathInput], diff --git a/telegram/request/_httpxrequest.py b/telegram/request/_httpxrequest.py index 41cc8b21d..00df85eed 100644 --- a/telegram/request/_httpxrequest.py +++ b/telegram/request/_httpxrequest.py @@ -82,6 +82,11 @@ class HTTPXRequest(BaseRequest): With a finite pool timeout, you must expect :exc:`telegram.error.TimedOut` exceptions to be thrown when more requests are made simultaneously than there are connections in the connection pool! + http_version (:obj:`str`, optional): If ``"1.1"``, HTTP/1.1 will be used instead of HTTP/2. + Defaults to ``"2"``. + + .. versionadded:: 20.1 + """ __slots__ = ("_client", "_client_kwargs") @@ -94,6 +99,7 @@ class HTTPXRequest(BaseRequest): write_timeout: Optional[float] = 5.0, connect_timeout: Optional[float] = 5.0, pool_timeout: Optional[float] = 1.0, + http_version: str = "2", ): timeout = httpx.Timeout( connect=connect_timeout, @@ -105,10 +111,18 @@ class HTTPXRequest(BaseRequest): max_connections=connection_pool_size, max_keepalive_connections=connection_pool_size, ) + + if http_version not in ("1.1", "2"): + raise ValueError("`http_version` must be either '1.1' or '2'.") + + http1 = http_version == "1.1" + self._client_kwargs = dict( timeout=timeout, proxies=proxy_url, limits=limits, + http1=http1, + http2=not http1, ) try: diff --git a/tests/test_applicationbuilder.py b/tests/test_applicationbuilder.py index 3f1996bd7..e45fb90cd 100644 --- a/tests/test_applicationbuilder.py +++ b/tests/test_applicationbuilder.py @@ -90,6 +90,8 @@ class TestApplicationBuilder: timeout: object proxies: object limits: object + http1: object + http2: object monkeypatch.setattr(httpx, "AsyncClient", Client) @@ -118,11 +120,15 @@ class TestApplicationBuilder: assert get_updates_client.timeout == httpx.Timeout( connect=5.0, read=5.0, write=5.0, pool=1.0 ) + assert not get_updates_client.http1 + assert get_updates_client.http2 is True client = app.bot.request._client assert client.limits == httpx.Limits(max_connections=256, max_keepalive_connections=256) assert client.proxies is None assert client.timeout == httpx.Timeout(connect=5.0, read=5.0, write=5.0, pool=1.0) + assert not client.http1 + assert client.http2 is True assert isinstance(app.update_queue, asyncio.Queue) assert isinstance(app.updater, Updater) @@ -165,6 +171,7 @@ class TestApplicationBuilder: "proxy_url", "bot", "updater", + "http_version", ), ) def test_mutually_exclusive_for_request(self, builder, method): @@ -189,6 +196,7 @@ class TestApplicationBuilder: "get_updates_read_timeout", "get_updates_write_timeout", "get_updates_proxy_url", + "get_updates_http_version", "bot", "updater", ), @@ -216,12 +224,14 @@ class TestApplicationBuilder: "get_updates_read_timeout", "get_updates_write_timeout", "get_updates_proxy_url", + "get_updates_http_version", "connection_pool_size", "connect_timeout", "pool_timeout", "read_timeout", "write_timeout", "proxy_url", + "http_version", "bot", "update_queue", "rate_limiter", @@ -251,6 +261,7 @@ class TestApplicationBuilder: "get_updates_read_timeout", "get_updates_write_timeout", "get_updates_proxy_url", + "get_updates_http_version", "connection_pool_size", "connect_timeout", "pool_timeout", @@ -258,6 +269,7 @@ class TestApplicationBuilder: "write_timeout", "proxy_url", "bot", + "http_version", ] + [entry[0] for entry in _BOT_CHECKS], ) @@ -308,19 +320,23 @@ class TestApplicationBuilder: timeout: object proxies: object limits: object + http1: object + http2: object monkeypatch.setattr(httpx, "AsyncClient", Client) builder = ApplicationBuilder().token(bot.token) builder.connection_pool_size(1).connect_timeout(2).pool_timeout(3).read_timeout( 4 - ).write_timeout(5).proxy_url("proxy_url") + ).write_timeout(5).proxy_url("proxy_url").http_version("1.1") app = builder.build() client = app.bot.request._client assert client.timeout == httpx.Timeout(pool=3, connect=2, read=4, write=5) assert client.limits == httpx.Limits(max_connections=1, max_keepalive_connections=1) assert client.proxies == "proxy_url" + assert client.http1 is True + assert client.http2 is False builder = ApplicationBuilder().token(bot.token) builder.get_updates_connection_pool_size(1).get_updates_connect_timeout( @@ -329,6 +345,8 @@ class TestApplicationBuilder: 5 ).get_updates_proxy_url( "proxy_url" + ).get_updates_http_version( + "1.1" ) app = builder.build() client = app.bot._request[0]._client @@ -336,6 +354,8 @@ class TestApplicationBuilder: assert client.timeout == httpx.Timeout(pool=3, connect=2, read=4, write=5) assert client.limits == httpx.Limits(max_connections=1, max_keepalive_connections=1) assert client.proxies == "proxy_url" + assert client.http1 is True + assert client.http2 is False def test_custom_application_class(self, bot, builder): class CustomApplication(Application): diff --git a/tests/test_request.py b/tests/test_request.py index af074f842..384d1dadf 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -338,6 +338,8 @@ class TestHTTPXRequest: timeout: object proxies: object limits: object + http1: object + http2: object monkeypatch.setattr(httpx, "AsyncClient", Client) @@ -347,6 +349,7 @@ class TestHTTPXRequest: assert request._client.limits == httpx.Limits( max_connections=1, max_keepalive_connections=1 ) + assert request._client.http2 is True request = HTTPXRequest( connection_pool_size=42, @@ -400,6 +403,30 @@ class TestHTTPXRequest: async with httpx_request: await httpx_request.do_request(url="https://python-telegram-bot.org", method="GET") + async def test_http_version_error(self): + with pytest.raises(ValueError, match="`http_version` must be either"): + HTTPXRequest(http_version="1.0") + + async def test_http_1_response(self): + httpx_request = HTTPXRequest(http_version="1.1") + async with httpx_request: + resp = await httpx_request._client.request( + url="https://python-telegram-bot.org", + method="GET", + headers={"User-Agent": httpx_request.USER_AGENT}, + ) + assert resp.http_version == "HTTP/1.1" + + async def test_http_2_response(self): + httpx_request = HTTPXRequest() + async with httpx_request: + resp = await httpx_request._client.request( + url="https://python-telegram-bot.org", + method="GET", + headers={"User-Agent": httpx_request.USER_AGENT}, + ) + assert resp.http_version == "HTTP/2" + async def test_do_request_after_shutdown(self, httpx_request): await httpx_request.shutdown() with pytest.raises(RuntimeError, match="not initialized"):