Regression in requests hooks stubs

[inakimalerba]

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_hooks

CI 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]]]]")

[AlexWaygood]

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 #7094

I'm not familiar with with the requests API to be able to immediately say whether those changes are correct or not.

[brandonchinn178]

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 .append after?

s.hooks['response'] = lambda ...
s.hooks['response'].append(...)

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

s.hooks['response'] = [lambda ...]

which I think is a fine limitation

EDIT: OH! Actually, this is completely broken for sessions, because _Hooks is now an immutable map: #7732. We should probably partially revert it, where _Hooks should be mutable but _HooksInput should stay immutable.

[Akuli]

cki_lib/session.py:47: error: Unsupported target for indexed assignment ("Mapping[...

Indeed, it's an immutable Mapping.

We could make it mutable when accessed through s.hooks but 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]

If you specify a hooks argument, why would you also mutate it afterwards through the attribute?

Because that's how you're supposed to do it for sessions: https://docs.python-requests.org/en/latest/user/advanced/#event-hooks

You can also add hooks to a Session instance. Any hooks you add will then be called on every request made to the session. For example:

>>> s = requests.Session()
>>> s.hooks['response'].append(print_url)
>>> s.get('https://httpbin.org/')
 https://httpbin.org/
 <Response [200]>

A Session can have multiple hooks, which will be called in the order they are added.

Changing _Hooks would work, because all the requests.post() functions use _HooksInput

[Akuli]

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]

Session doesn't take any arguments:

https://github.com/psf/requests/blob/79f2ec3acc4e24fef6e6ce31ad7b1d4e2f77be31/requests/sessions.py#L389

[Akuli]

Ah, so the two types are already different. Indeed, we just need to make _Hooks mutable and keep _HooksInput immutable.

[inakimalerba]

@Akuli thank you for the fix ! :)