Revert to HTTP/1.1 as Default and make HTTP/2 an Optional Dependency (#3576)

Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
This commit is contained in:
Poolitzer 2023-03-12 16:30:39 +01:00 committed by GitHub
parent ec20f27a82
commit 800598ced4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 164 additions and 41 deletions

View file

@ -50,7 +50,8 @@ jobs:
# - without ratelimiter
# - without webhooks
# - without arbitrary callback data
# - without socks suppport
# - without socks support
# - without http2 support
TO_TEST="test_no_passport.py or test_datetime.py or test_defaults.py or test_jobqueue.py or test_applicationbuilder.py or test_ratelimiter.py or test_updater.py or test_callbackdatacache.py or test_request.py"
pytest -v --cov -k "${TO_TEST}"
status=$?

View file

@ -135,9 +135,8 @@ 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[http2] ~= 0.23.3 <https://www.python-httpx.org>`_ 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.
The only required dependency is `httpx ~= 0.23.3 <https://www.python-httpx.org>`_ for
``telegram.request.HTTPXRequest``, the default networking backend.
``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.
@ -151,6 +150,7 @@ PTB can be installed with optional dependencies:
* ``pip install python-telegram-bot[passport]`` installs the `cryptography>=39.0.1 <https://cryptography.io/en/stable>`_ library. Use this, if you want to use Telegram Passport related functionality.
* ``pip install python-telegram-bot[socks]`` installs `httpx[socks] <https://www.python-httpx.org/#dependencies>`_. Use this, if you want to work behind a Socks5 server.
* ``pip install python-telegram-bot[http2]`` installs `httpx[http2] <https://www.python-httpx.org/#dependencies>`_. Use this, if you want to use HTTP/2.
* ``pip install python-telegram-bot[rate-limiter]`` installs `aiolimiter~=1.0.0 <https://aiolimiter.readthedocs.io/en/stable/>`_. Use this, if you want to use ``telegram.ext.AIORateLimiter``.
* ``pip install python-telegram-bot[webhooks]`` installs the `tornado~=6.2 <https://www.tornadoweb.org/en/stable/>`_ library. Use this, if you want to use ``telegram.ext.Updater.start_webhook``/``telegram.ext.Application.run_webhook``.
* ``pip install python-telegram-bot[callback-data]`` installs the `cachetools~=5.3.0 <https://cachetools.readthedocs.io/en/latest/>`_ library. Use this, if you want to use `arbitrary callback_data <https://github.com/python-telegram-bot/python-telegram-bot/wiki/Arbitrary-callback_data>`_.

View file

@ -136,9 +136,8 @@ 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[http2] ~= 0.23.3 <https://www.python-httpx.org>`_ 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.
The only required dependency is `httpx ~= 0.23.3 <https://www.python-httpx.org>`_ for
``telegram.request.HTTPXRequest``, the default networking backend.
``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.
@ -152,6 +151,7 @@ PTB can be installed with optional dependencies:
* ``pip install python-telegram-bot-raw[passport]`` installs the `cryptography>=39.0.1 <https://cryptography.io/en/stable>`_ library. Use this, if you want to use Telegram Passport related functionality.
* ``pip install python-telegram-bot-raw[socks]`` installs `httpx[socks] <https://www.python-httpx.org/#dependencies>`_. Use this, if you want to work behind a Socks5 server.
* ``pip install python-telegram-bot[http2]`` installs `httpx[http2] <https://www.python-httpx.org/#dependencies>`_. Use this, if you want to use HTTP/2.
To install multiple optional dependencies, separate them by commas, e.g. ``pip install python-telegram-bot-raw[passport,socks]``.

View file

@ -11,6 +11,7 @@
# versions and only increase the lower bound if necessary
httpx[socks] # socks
httpx[http2] # http2
cryptography!=3.4,!=3.4.1,!=3.4.2,!=3.4.3,>=39.0.1 # passport
aiolimiter~=1.0.0 # rate-limiter!ext

View file

@ -6,5 +6,4 @@
# versions and only increase the lower bound if necessary
# httpx has no stable release yet, so let's be cautious for now
# HTTP/2 is more performant and stable than HTTP/1.1, specially for concurrent requests
httpx[http2] ~= 0.23.3
httpx ~= 0.23.3

View file

@ -92,12 +92,14 @@ from telegram._utils.argumentparsing import parse_sequence_arg
from telegram._utils.defaultvalue import DEFAULT_NONE, DefaultValue
from telegram._utils.files import is_local_file, parse_file_input
from telegram._utils.types import DVInput, FileInput, JSONDict, ODVInput, ReplyMarkup
from telegram._utils.warnings import warn
from telegram._webhookinfo import WebhookInfo
from telegram.constants import InlineQueryLimit
from telegram.error import InvalidToken
from telegram.request import BaseRequest, RequestData
from telegram.request._httpxrequest import HTTPXRequest
from telegram.request._requestparameter import RequestParameter
from telegram.warnings import PTBUserWarning
if TYPE_CHECKING:
from telegram import (
@ -241,6 +243,38 @@ class Bot(TelegramObject, AsyncContextManager["Bot"]):
HTTPXRequest() if request is None else request,
)
# this section is about issuing a warning when using HTTP/2 and connect to a self hosted
# bot api instance, which currently only supports HTTP/1.1. Checking if a custom base url
# is set is the best way to do that.
warning_string = ""
if (
isinstance(self._request[0], HTTPXRequest)
and self._request[0].http_version == "2"
and not base_url.startswith("https://api.telegram.org/bot")
):
warning_string = "get_updates_request"
if (
isinstance(self._request[1], HTTPXRequest)
and self._request[1].http_version == "2"
and not base_url.startswith("https://api.telegram.org/bot")
):
if warning_string:
warning_string += " and request"
else:
warning_string = "request"
if warning_string:
warn(
f"You set the HTTP version for the {warning_string} HTTPXRequest instance to "
f"HTTP/2. The self hosted bot api instances only support HTTP/1.1. You should "
f"either run a HTTP proxy in front of it which supports HTTP/2 or use HTTP/1.1.",
PTBUserWarning,
stacklevel=2,
)
if private_key:
if not CRYPTO_INSTALLED:
raise RuntimeError(

View file

@ -178,7 +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._get_updates_http_version: DVInput[str] = DefaultValue("1.1")
self._private_key: ODVInput[bytes] = DEFAULT_NONE
self._private_key_password: ODVInput[bytes] = DEFAULT_NONE
self._defaults: ODVInput["Defaults"] = DEFAULT_NONE
@ -204,7 +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")
self._http_version: DVInput[str] = DefaultValue("1.1")
def _build_request(self, get_updates: bool) -> BaseRequest:
prefix = "_get_updates_" if get_updates else "_"
@ -232,7 +232,7 @@ 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"
http_version = DefaultValue.get_value(getattr(self, f"{prefix}http_version")) or "1.1"
return HTTPXRequest(
connection_pool_size=connection_pool_size,
@ -564,15 +564,32 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]):
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.
:attr:`telegram.Bot.request`. By default, HTTP/1.1 is used.
.. seealso:: :meth:`get_updates_http_version`
Note:
Users have observed stability issues with HTTP/2, which happen due to how the `h2
library handles <https://github.com/python-hyper/h2/issues/1181>` cancellations of
keepalive connections. See `#3556 <https://github.com/python-telegram-bot/
python-telegram-bot/issues/3556>`_ for a discussion.
If you want to use HTTP/2, you must install PTB with the optional requirement
``http2``, i.e.
.. code-block:: bash
pip install python-telegram-bot[http2]
Keep in mind that the HTTP/1.1 implementation may be considered the `"more
robust option at this time" <https://www.python-httpx.org/http2#enabling-http2>`_.
.. versionadded:: 20.1
.. versionchanged:: NEXT.VERSION
Reset the default version to 1.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.
http_version (:obj:`str`): Pass ``"2"`` if you'd like to use HTTP/2 for making
requests to Telegram. Defaults to ``"1.1"``, in which case HTTP/1.1 is used.
Returns:
:class:`ApplicationBuilder`: The same builder with the updated argument.
@ -705,15 +722,31 @@ class ApplicationBuilder(Generic[BT, CCT, UD, CD, BD, JQ]):
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.
:meth:`telegram.Bot.get_updates` request. By default, HTTP/1.1 is used.
.. seealso:: :meth:`http_version`
Note:
Users have observed stability issues with HTTP/2, which happen due to how the `h2
library handles <https://github.com/python-hyper/h2/issues/1181>` cancellations of
keepalive connections. See `#3556 <https://github.com/python-telegram-bot/
python-telegram-bot/issues/3556>`_ for a discussion.
You will also need to install the http2 dependency. Keep in mind that the HTTP/1.1
implementation may be considered the `"more robust option at this time"
<https://www.python-httpx.org/http2#enabling-http2>`_.
.. code-block:: bash
pip install httpx[http2]
.. versionadded:: 20.1
.. versionchanged:: NEXT.VERSION
Reset the default version to 1.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.
get_updates_http_version (:obj:`str`): Pass ``"2"`` if you'd like to use HTTP/2 for
making requests to Telegram. Defaults to ``"1.1"``, in which case HTTP/1.1 is used.
Returns:
:class:`ApplicationBuilder`: The same builder with the updated argument.

View file

@ -82,14 +82,16 @@ 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"``.
http_version (:obj:`str`, optional): If ``"2"``, HTTP/2 will be used instead of HTTP/1.1.
Defaults to ``"1.1"``.
.. versionadded:: 20.1
.. versionchanged:: NEXT.VERSION
Reset the default version to 1.1.
"""
__slots__ = ("_client", "_client_kwargs")
__slots__ = ("_client", "_client_kwargs", "_http_version")
def __init__(
self,
@ -99,8 +101,9 @@ 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",
http_version: str = "1.1",
):
self._http_version = http_version
timeout = httpx.Timeout(
connect=connect_timeout,
read=read_timeout,
@ -130,14 +133,28 @@ class HTTPXRequest(BaseRequest):
try:
self._client = self._build_client()
except ImportError as exc:
if "httpx[socks]" not in str(exc):
if "httpx[http2]" not in str(exc) and "httpx[socks]" not in str(exc):
raise exc
if "httpx[socks]" in str(exc):
raise RuntimeError(
"To use Socks5 proxies, PTB must be installed via `pip install "
"python-telegram-bot[socks]`."
) from exc
raise RuntimeError(
"To use Socks5 proxies, PTB must be installed via `pip install "
"python-telegram-bot[socks]`."
"To use HTTP/2, PTB must be installed via `pip install "
"python-telegram-bot[http2]`."
) from exc
@property
def http_version(self) -> str:
"""
:obj:`str`: Used HTTP version, see :paramref:`http_version`.
.. versionadded:: NEXT.VERSION
"""
return self._http_version
def _build_client(self) -> httpx.AsyncClient:
return httpx.AsyncClient(**self._client_kwargs) # type: ignore[arg-type]

View file

@ -118,15 +118,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
assert get_updates_client.http1 is True
assert not get_updates_client.http2
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 client.http1 is True
assert not client.http2
assert isinstance(app.update_queue, asyncio.Queue)
assert isinstance(app.updater, Updater)

View file

@ -74,12 +74,30 @@ async def httpx_request():
@pytest.mark.skipif(
TEST_WITH_OPT_DEPS, reason="Only relevant if the optional dependency is not installed"
)
class TestNoSocks:
class TestNoSocksHTTP2WithoutRequest:
async def test_init(self, bot):
with pytest.raises(RuntimeError, match=r"python-telegram-bot\[socks\]"):
HTTPXRequest(proxy_url="socks5://foo")
with pytest.raises(RuntimeError, match=r"python-telegram-bot\[http2\]"):
HTTPXRequest(http_version="2")
@pytest.mark.skipif(not TEST_WITH_OPT_DEPS, reason="Optional dependencies not installed")
class TestHTTP2WithRequest:
async def test_http_2_response(self):
httpx_request = HTTPXRequest(http_version="2")
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"
# I picked not TEST_XXX because that's the default, meaning it will run by default for an end-user
# who runs pytest.
@pytest.mark.skipif(not TEST_WITH_OPT_DEPS, reason="No need to run this twice")
class TestRequestWithoutRequest:
test_flag = None
@ -321,6 +339,7 @@ class TestRequestWithoutRequest:
assert self.test_flag == (1, 2, 3, 4)
@pytest.mark.skipif(not TEST_WITH_OPT_DEPS, reason="No need to run this twice")
class TestHTTPXRequestWithoutRequest:
test_flag = None
@ -345,7 +364,8 @@ class TestHTTPXRequestWithoutRequest:
assert request._client.limits == httpx.Limits(
max_connections=1, max_keepalive_connections=1
)
assert request._client.http2 is True
assert request._client.http1 is True
assert not request._client.http2
request = HTTPXRequest(
connection_pool_size=42,
@ -413,16 +433,6 @@ class TestHTTPXRequestWithoutRequest:
)
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"):
@ -592,6 +602,7 @@ class TestHTTPXRequestWithoutRequest:
)
@pytest.mark.skipif(not TEST_WITH_OPT_DEPS, reason="No need to run this twice")
class TestHTTPXRequestWithRequest:
async def test_do_request_wait_for_pool(self, httpx_request):
"""The pool logic is buried rather deeply in httpxcore, so we make actual requests here

View file

@ -1648,6 +1648,33 @@ class TestBotWithoutRequest:
bot.callback_data_cache.clear_callback_data()
bot.callback_data_cache.clear_callback_queries()
async def test_http2_runtime_error(self, recwarn):
Bot("12345:ABCDE", base_url="http://", request=HTTPXRequest(http_version="2"))
Bot(
"12345:ABCDE",
base_url="http://",
get_updates_request=HTTPXRequest(http_version="2"),
)
Bot(
"12345:ABCDE",
base_url="http://",
request=HTTPXRequest(http_version="2"),
get_updates_request=HTTPXRequest(http_version="2"),
)
# this exists to make sure the error is also raised by extbot
ExtBot("12345:ABCDE", base_url="http://", request=HTTPXRequest(http_version="2"))
assert len(recwarn) == 4
assert "You set the HTTP version for the request HTTPXRequest instance" in str(
recwarn[0].message
)
assert "You set the HTTP version for the get_updates_request HTTPXRequest instance" in str(
recwarn[1].message
)
assert (
"You set the HTTP version for the get_updates_request and request HTTPXRequest "
"instance" in str(recwarn[2].message)
)
class TestBotWithRequest:
"""