-
-
Notifications
You must be signed in to change notification settings - Fork 828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Event hooks #1215
Event hooks #1215
Changes from all commits
f07bfc5
3767797
4450ea2
d3e0472
7247802
e9ca1c1
b61972c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
import typing | ||
import warnings | ||
from base64 import b64encode | ||
from collections.abc import MutableMapping | ||
from pathlib import Path | ||
|
||
import certifi | ||
|
@@ -410,6 +411,48 @@ def __repr__(self) -> str: | |
) | ||
|
||
|
||
class EventHooks(MutableMapping): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we apply my suggestion of using a plain I'm also not sure we need to enforce that the only possible keys be So perhaps we could replace this with a simple factory helper… def create_event_hooks(initial: Dict[str, List[Callable]] = None) -> Dict[str, List[Callable]]):
event_hooks = {"request": [], "response": []}
event_hooks.update(initial or {})
return event_hooks |
||
def __init__(self, *args: typing.Any, **kwargs: typing.Any) -> None: | ||
value = dict(*args, **kwargs) | ||
self._dict = { | ||
"request": self._as_list(value.get("request", [])), | ||
"response": self._as_list(value.get("response", [])), | ||
} | ||
|
||
def _as_list(self, value: typing.Any) -> list: | ||
if not isinstance(value, (list, tuple)): | ||
return [value] | ||
return list(value) | ||
|
||
def __getitem__(self, key: typing.Any) -> typing.List[typing.Callable]: | ||
return self._dict[key] | ||
|
||
def __setitem__( | ||
self, | ||
key: str, | ||
value: typing.Union[typing.Callable, typing.List[typing.Callable]], | ||
) -> None: | ||
if key in self._dict: | ||
self._dict[key] = self._as_list(value) | ||
|
||
def __delitem__(self, key: str) -> None: | ||
if key in self._dict: | ||
self._dict[key] = [] | ||
|
||
def __iter__(self) -> typing.Iterator[str]: | ||
return iter(self._dict.keys()) | ||
|
||
def __len__(self) -> int: | ||
return len(self._dict) | ||
|
||
def __str__(self) -> str: | ||
return str(self._dict) | ||
|
||
def __repr__(self) -> str: | ||
class_name = self.__class__.__name__ | ||
return f"{class_name}({self._dict!r})" | ||
|
||
|
||
DEFAULT_TIMEOUT_CONFIG = Timeout(timeout=5.0) | ||
DEFAULT_LIMITS = Limits(max_connections=100, max_keepalive_connections=20) | ||
DEFAULT_MAX_REDIRECTS = 20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd like us to prefer a stricter typing here, and a more straightforward style that requires using a list of callables:
It would make it clearer that
client.event_hooks
is really just always a dict of lists, rather than "a dict of sometimes a callable, sometimes a list of callables, depending on what you've set" (not even sure that's what this PR intends, actually?).Hence this suggestion (which would need to be applied across
_client.py
as well):