Repository navigation
Regression in requests hooks stubs #7776
Description
Since f330e12 it's not possible to add new hooks to requests.Session.
According to the documentation, the recommended way to add a hook to every request is by adding them to requests.Session.hook:
s = requests.Session()
s.hooks['response'].append(print_url)This snippet now fails with the following error:
error: Unsupported target for indexed assignment ("Mapping[str, Union[Callable[[Response], Any], List[Callable[[Response], Any]]]]")
Example project
source: https://gitlab.com/cki-project/cki-lib/-/blob/main/cki_lib/session.py#L47
response_hooks: typing.List[typing.Callable[[requests.Response], typing.Any]] = []
session.hooks['response'] = response_hooksCI job: https://gitlab.com/cki-project/cki-lib/-/jobs/2407190024
cki_lib/session.py:47: error: Unsupported target for indexed assignment ("Mapping[str, Union[Callable[[Response], Any], List[Callable[[Response], Any]]]]")
Activity
AlexWaygood commentedon May 5, 2022
I think the changes to the stub that means this no longer type-check were made slightly further back, in
requests.Session: Allow passing of a single hook instead of a list of hooks #7094I'm not familiar with with the
requestsAPI to be able to immediately say whether those changes are correct or not.brandonchinn178 commentedon May 18, 2022
The change is correct; requests allows hooks to either be a list or a function (which it'll turn into a list):
https://github.com/psf/requests/blob/79f2ec3acc4e24fef6e6ce31ad7b1d4e2f77be31/requests/hooks.py#L27-L28
Unfortunately, this makes typing an issue; what if you set a function as a hook and then call
.appendafter?are perfectly valid uses of the requests API separately, but will fail when used together. This is more of a question for the requests API design, but my two cents are to revert #7094 and say "if you're using requests + typeshed, you MUST operate on the list" (cc @UncleOwen). To completely override all previously set hooks, this would then require the user doing explicitly
which I think is a fine limitation
EDIT: OH! Actually, this is completely broken for sessions, because
_Hooksis now an immutable map: #7732. We should probably partially revert it, where_Hooksshould be mutable but_HooksInputshould stay immutable.Akuli commentedon May 18, 2022
Indeed, it's an immutable
Mapping.We could make it mutable when accessed through
s.hooksbut immutable when passed as argument, which is technically wrong, but works fine in practice. If you specify a hooks argument, why would you also mutate it afterwards through the attribute?brandonchinn178 commentedon May 18, 2022
Because that's how you're supposed to do it for sessions: https://docs.python-requests.org/en/latest/user/advanced/#event-hooks
Changing
_Hookswould work, because all therequests.post()functions use_HooksInputAkuli commentedon May 18, 2022
That mutates it through the attribute, but doesn't specify it as an argument when constructing the
Session. My question is why would you need to do both, which is when mismatching the types would cause problems.brandonchinn178 commentedon May 18, 2022
Sessiondoesn't take any arguments:https://github.com/psf/requests/blob/79f2ec3acc4e24fef6e6ce31ad7b1d4e2f77be31/requests/sessions.py#L389
Akuli commentedon May 18, 2022
Ah, so the two types are already different. Indeed, we just need to make
_Hooksmutable and keep_HooksInputimmutable.Use `dict` for requests.Session().hooks
Fix requests.Session().hooks (#7871)
inakimalerba commentedon May 19, 2022
@Akuli thank you for the fix ! :)