From cab734afc42ebb1164d37fd0cf40dee8304ed23e Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Wed, 18 Sep 2024 16:07:34 +0200 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=8F=B7=EF=B8=8F=20Elminiate=20an=20An?= =?UTF-8?q?y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the types-requests stubs to annotate the kwargs. Ideally we don't import _Names from a stub namespace but let the type checker infer the ParamSpec.kwargs from its use. But ParamSpec can't be used as where a Type is expected. --- src/ape_pie/typing.py | 41 ++++++++++++++++++++++++++++++++++-- tests/test_config_adapter.py | 3 ++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/ape_pie/typing.py b/src/ape_pie/typing.py index 4cfe6bf..7858e9d 100644 --- a/src/ape_pie/typing.py +++ b/src/ape_pie/typing.py @@ -1,4 +1,41 @@ -from typing import Any, Protocol +from __future__ import annotations + +from http.cookiejar import CookieJar +from typing import TYPE_CHECKING, Protocol, TypedDict + +if TYPE_CHECKING: + # imports from stubs + from _typeshed import Incomplete + from requests.api import _HeadersMapping + from requests.sessions import ( + _Auth, + _Cert, + _Data, + _Files, + _HooksInput, + _Params, + _TextMapping, + _Timeout, + _Verify, + ) + + +class RequestKwargs(TypedDict, total=False): + # Supposed to be ParamSpec.kwargs of `requests.request` + params: _Params + data: _Data + headers: _HeadersMapping + cookies: CookieJar | _TextMapping | None + files: _Files | None + auth: _Auth | None + timeout: _Timeout | None + allow_redirects: bool + proxies: _TextMapping | None + hooks: _HooksInput | None + stream: bool | None + verify: _Verify | None + cert: _Cert | None + json: Incomplete class ConfigAdapter(Protocol): @@ -8,7 +45,7 @@ def get_client_base_url(self) -> str: # pragma: no cover """ ... - def get_client_session_kwargs(self) -> dict[str, Any]: # pragma: no cover + def get_client_session_kwargs(self) -> RequestKwargs: # pragma: no cover """ Return kwargs to feed to :class:`requests.Session` when using the client. diff --git a/tests/test_config_adapter.py b/tests/test_config_adapter.py index b4da204..af3ebc6 100644 --- a/tests/test_config_adapter.py +++ b/tests/test_config_adapter.py @@ -9,6 +9,7 @@ from ape_pie import APIClient from ape_pie.exceptions import InvalidURLError +from ape_pie.typing import RequestKwargs class TestConfigAdapter: @@ -17,7 +18,7 @@ def get_client_base_url(): return "https://from-factory.example.com" @staticmethod - def get_client_session_kwargs(): + def get_client_session_kwargs() -> RequestKwargs: return { "verify": False, "timeout": 20, From b75aea04f7a24c5f6fcd6fc2632a8b927a29863f Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Wed, 18 Sep 2024 16:29:48 +0200 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=90=9B=20Avoid=20mutation=20surprises?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ape_pie/client.py | 25 ++++++++++--------------- tests/test_client_api.py | 24 +++++++++++++++++++++++- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/ape_pie/client.py b/src/ape_pie/client.py index 5c4bfe0..646222f 100644 --- a/src/ape_pie/client.py +++ b/src/ape_pie/client.py @@ -18,16 +18,13 @@ from requests import Response, Session from .exceptions import InvalidURLError -from .typing import ConfigAdapter +from .typing import ConfigAdapter, RequestKwargs if TYPE_CHECKING: # TODO Use typing.Self once we drop support for Py310 from typing_extensions import Self -sentinel = object() - - def is_base_url(url: str | furl) -> bool: """ Check if a URL is not a relative path/URL. @@ -75,7 +72,7 @@ class APIClient(Session): def __init__( self, base_url: str, - request_kwargs: dict[str, Any] | None = None, + request_kwargs: RequestKwargs | None = None, **kwargs: Any, # subclasses may require additional configuration ): # base class does not take any kwargs @@ -85,16 +82,14 @@ def __init__( self.base_url = base_url - # set the attributes that requests.Session supports directly, but only if an - # actual value was provided. - for attr in self.__attrs__: - val = request_kwargs.pop(attr, sentinel) - if val is sentinel: - continue - setattr(self, attr, val) - - # store the remainder so we can inject it in the ``request`` method. - self._request_kwargs = request_kwargs + self._request_kwargs = {} + # set the attributes that requests.Session supports directly + for k, v in request_kwargs.items(): + if k in self.__attrs__: + setattr(self, k, v) + else: + # store the remainder so we can inject it in the ``request`` method. + self._request_kwargs[k] = v def __enter__(self): self._in_context_manager = True diff --git a/tests/test_client_api.py b/tests/test_client_api.py index 5772fd2..46bb355 100644 --- a/tests/test_client_api.py +++ b/tests/test_client_api.py @@ -3,6 +3,7 @@ from requests.auth import HTTPBasicAuth from ape_pie import APIClient +from ape_pie.typing import RequestKwargs @pytest.mark.parametrize("attr", Session.__attrs__) @@ -32,9 +33,30 @@ def test_can_override_defaults(attr, value): assert attr in Session.__attrs__ vanilla_session = Session() - client = APIClient("https://example.com", {attr: value}) + client = APIClient("https://example.com", {attr: value}) # type: ignore[assignment] vanilla_value = getattr(vanilla_session, attr) client_value = getattr(client, attr) assert client_value != vanilla_value assert client_value == value + + +def test_config_does_not_mutate(): + imported_settings: RequestKwargs = { + "auth": ("djkhaled", "youplayedyourself"), + } + + class MyConfig: + @staticmethod + def get_client_base_url(): + return "https://example.com" + + @staticmethod + def get_client_session_kwargs(): + return imported_settings + + one = APIClient.configure_from(MyConfig) + another_one = APIClient.configure_from(MyConfig) + + assert one.auth == ("djkhaled", "youplayedyourself") + assert another_one.auth == ("djkhaled", "youplayedyourself") From 2da85905ae1eee98461cc34d653fad0928667bc2 Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Wed, 18 Sep 2024 16:41:53 +0200 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=91=B7=20Run=20isort=20and=20mypy=20o?= =?UTF-8?q?n=20just=20{src,=20tests}?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Was already like this for black. Running tox locally with a local venv in the project directory was failing (and taking unnecessarily long)... Actually still does: flake8 + isort + black > uv 😉 --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 0584d83..412a58c 100644 --- a/tox.ini +++ b/tox.ini @@ -29,7 +29,7 @@ commands = mypy src [testenv:isort] extras = dev skipsdist = True -commands = isort --check-only --diff . +commands = isort --check-only --diff src tests [testenv:black] extras = dev @@ -39,7 +39,7 @@ commands = black --check src tests docs [testenv:flake8] extras = dev skipsdist = True -commands = flake8 . +commands = flake8 src tests [testenv:docs] basepython=python