Skip to content

Commit

Permalink
Merge pull request #6 from maykinmedia/mutation-bug
Browse files Browse the repository at this point in the history
Avoid surprising mutation and remove Any from the kwargs type.
  • Loading branch information
CharString authored Sep 19, 2024
2 parents a4cc5ab + 2da8590 commit c976a95
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 21 deletions.
25 changes: 10 additions & 15 deletions src/ape_pie/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
41 changes: 39 additions & 2 deletions src/ape_pie/typing.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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.
Expand Down
24 changes: 23 additions & 1 deletion tests/test_client_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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")
3 changes: 2 additions & 1 deletion tests/test_config_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from ape_pie import APIClient
from ape_pie.exceptions import InvalidURLError
from ape_pie.typing import RequestKwargs


class TestConfigAdapter:
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit c976a95

Please sign in to comment.