`requests`: Allow session headers to be of type None #7773

janrito · 2022-05-03T11:45:31Z

Session headers can be unset by setting them to none, see: https://github.com/psf/requests/blob/40956723f27daf5e0d9759208ca69cef236ab339/requests/sessions.py#L61

Akuli · 2022-05-03T14:09:19Z

Currently many functions accept headers: _TextMapping, where _TextMapping is defined to be MutableMapping[str, str]. Can you fix that too? The correct type is probably MutableMapping[str, str] | MutableMapping[str, str | None], because MutableMapping is invariant and so a dict[str, str] is not a valid MutableMapping[str, str | None].

janrito · 2022-05-03T14:17:21Z

@Akuli does it make sense for headers to be None?

in a session, None is used to remove headers from a specific request. Empty headers should be invalid I think?

janrito · 2022-05-03T14:20:39Z

session = requests.Session()
session.headers.update({"Content-Type": "application/json"})

# request with content type
session.get("http://api.com")

# request with no content type
session.get("http://api.com", headers={"Content-Type": None})

nsoranzo · 2022-05-03T22:26:15Z

stubs/requests/requests/sessions.pyi

@@ -72,7 +72,7 @@ _Verify: TypeAlias = bool | str

 class Session(SessionRedirectMixin):
    __attrs__: Any
-    headers: CaseInsensitiveDict[str]
+    headers: CaseInsensitiveDict[str | None]


As mentioned in https://docs.python-requests.org/en/latest/user/quickstart/#custom-headers , header values can also be bytestring.

Suggested change

headers: CaseInsensitiveDict[str | None]

headers: CaseInsensitiveDict[str | bytes | None]

Interesting. I think I added None to the wrong place. We want to allow str or bytes on actual headers, but None only on updates

This reverts commit d9ca9a7.

…eaders values

… well as str and bytes

JelleZijlstra · 2022-05-04T14:47:56Z

stubs/requests/requests/sessions.pyi

@@ -67,12 +67,14 @@ _Params: TypeAlias = Union[
    str | bytes,
 ]
 _TextMapping: TypeAlias = MutableMapping[str, str]
+_HeadersMapping = TypeAlias = MutableMapping[str, str | bytes]


This is inconvenient because MutableMapping is invariant in its value type, so a dict[str, str] would not be acceptable.

What about

MutableMapping[str, str] | MutableMapping[str, bytes]

?

…leMapping[str, bytes]` + simple assignment in alias definition

nsoranzo · 2022-05-04T18:26:38Z

stubs/requests/requests/sessions.pyi

@@ -67,12 +67,14 @@ _Params: TypeAlias = Union[
    str | bytes,
 ]
 _TextMapping: TypeAlias = MutableMapping[str, str]
+_HeadersMapping: TypeAlias = MutableMapping[str, str] | MutableMapping[str, bytes]
+_HeadersUpdateMapping: TypeAlias = MutableMapping[str, str] | MutableMapping[str, bytes] | MutableMapping[str, None]


I'm not sure that's correct, header values can be some str and some bytes?

Mmm I see - would we have to specify all combinations of mutable mappings for str, bytes and None?

JelleZijlstra · 2022-05-08T04:13:29Z

stubs/requests/requests/models.pyi

@@ -95,7 +95,7 @@ class Response:
    __attrs__: Any
    _content: bytes | None  # undocumented
    status_code: int
-    headers: CaseInsensitiveDict[str]
+    headers: CaseInsensitiveDict[str | bytes]


Are you sure this is correct? These headers are from the HTTP response, and I'd expect requests to return the same type consistently.

This seems to be the cause of a false positive on this line found by mypy-primer: https://github.com/yurijmikhalevich/rclip/blob/98a2454b988986f6ed67ae6e9fe3342ea82e0f92/rclip/utils.py#L92

I think you are right: https://github.com/psf/requests/blob/40956723f27daf5e0d9759208ca69cef236ab339/requests/models.py#L484

+ these are casted to native strings here https://github.com/psf/requests/blob/40956723f27daf5e0d9759208ca69cef236ab339/requests/models.py#L484

janrito · 2022-05-13T13:13:48Z

I'm unsure what the next steps are

Akuli · 2022-05-13T15:16:03Z

You removed | bytes from the wrong place. It's fine to have it in session headers, but not in response headers.

This reverts commit 6f7eb28.

janrito · 2022-05-16T16:00:27Z

You removed | bytes from the wrong place. It's fine to have it in session headers, but not in response headers.

whelp f9aaf0c, 0d80bdf

AlexWaygood

https://github.com/psf/requests/blob/79f2ec3acc4e24fef6e6ce31ad7b1d4e2f77be31/requests/models.py#L493

AlexWaygood · 2022-05-16T17:26:14Z

Well, my suggested change to work around the invariance of CaseInsensitiveDict appeared to introduce a lot of new false-positives, so, uh... maybe ignore my remarks there and revert the last commit? 🙃

This reverts commit f189735.

janrito · 2022-05-16T19:15:01Z

I find it quite challenging to have a local workflow for this. Not even testing other projects, but my own. I'd love some pointers if anyone has some

github-actions · 2022-05-16T19:22:22Z

Diff from mypy_primer, showing the effect of this PR on open source code:

rotki (https://github.com/rotki/rotki)
+ rotkehlchen/premium/premium.py:219: error: Unused "type: ignore" comment
+ rotkehlchen/premium/premium.py:245: error: Unused "type: ignore" comment
+ rotkehlchen/premium/premium.py:270: error: Unused "type: ignore" comment
+ rotkehlchen/premium/premium.py:301: error: Unused "type: ignore" comment
+ rotkehlchen/premium/premium.py:328: error: Unused "type: ignore" comment
+ rotkehlchen/exchanges/kraken.py:546: error: Unused "type: ignore" comment

JelleZijlstra · 2022-05-16T20:23:45Z

I find it quite challenging to have a local workflow for this. Not even testing other projects, but my own. I'd love some pointers if anyone has some

One quick-and-dirty way is to just edit the stub files used by your type checker (somewhere in site-packages) in place. You can also use $MYPYPATH.

Akuli · 2022-05-16T20:26:34Z

To clarify, here's how you would use MYPYPATH:

$ MYPYPATH=/home/yourname/typeshed/stubs/requests mypy project_using_requests

This lets mypy import from /home/yourname/typeshed/stubs/requests. There's also --custom-typeshed-dir, but it unfortunately affects stdlib stubs only.

Akuli

Primer is happy and the code makes sense to me (None can be used to override whatever was configured in the session). Thanks for contributing! This one was trickier than expected.

rpdelaney · 2022-05-23T16:27:49Z

After this change, my type checker is throwing an error:

error: Argument "headers" to "get" has incompatible type "MappingProxyType[str, str]"; expected "Optional[MutableMapping[str, str]]" [arg-type]

Do headers have to be mutable? Can we also accept MappingProxyType?

Due to recent typeshed change: python/typeshed#7773

python#7773 changed `requests.session.Session` methods to accept None for header values, but didn't do quite the same for the functions in `requests.api`. I think this was a mistake. The functions in `requests.api` just pass through the `headers` argument without doing anything in particular to it. Furthermore, it's useful to be able to pass None as a header value: because `requests.utils.default_headers` sets an `Accept-Encoding` header by default, the easiest way to send a request with no `Accept-Encoding` header is something like `requests.get(url, headers={"Accept-Encoding": None})`. It's annoying to have to construct a `Session` just to pass type-checking. It's a little confusing for the type alias to be called `_HeadersUpdateMapping` in `requests.sessions` but `_HeadersMapping` in `requests.api`; this is because the latter name was already used in other type stubs (`tensorflow.keras.callbacks`), so it seemed best to avoid breaking API.

#7773 changed `requests.session.Session` methods to accept None for header values, but didn't do quite the same for the functions in `requests.api`. I think this was a mistake. The functions in `requests.api` just pass through the `headers` argument without doing anything in particular to it. Furthermore, it's useful to be able to pass None as a header value: because `requests.utils.default_headers` sets an `Accept-Encoding` header by default, the easiest way to send a request with no `Accept-Encoding` header is something like `requests.get(url, headers={"Accept-Encoding": None})`. It's annoying to have to construct a `Session` just to pass type-checking. It's a little confusing for the type alias to be called `_HeadersUpdateMapping` in `requests.sessions` but `_HeadersMapping` in `requests.api`; this is because the latter name was already used in other type stubs (`tensorflow.keras.callbacks`), so it seemed best to avoid breaking API.

Allow session headers to be of type None

d9ca9a7

AlexWaygood changed the title ~~Allow session headers to be of type None~~ requests: Allow session headers to be of type None May 3, 2022

nsoranzo reviewed May 3, 2022

View reviewed changes

nsoranzo mentioned this pull request May 3, 2022

Update Python dependencies galaxyproject/galaxy#13836

Merged

janrito added 4 commits May 4, 2022 15:29

Revert "Allow session headers to be of type None"

249ce7c

This reverts commit d9ca9a7.

Allow for bytes in header value

0602cc3

Add _HeadersMapping alias, which allows bytes as well as strings as h…

b1ad88d

…eaders values

Add _HeadersMappingUpdate alias, which allows for an empty mapping as…

9996858

… well as str and bytes

JelleZijlstra reviewed May 4, 2022

View reviewed changes

janrito added 2 commits May 4, 2022 15:55

fix type annotation rather than assignment

3368ce6

`MutableMapping[str, str | bytes] -> MutableMapping[str, str] | Mutab…

0e8ff7e

…leMapping[str, bytes]` + simple assignment in alias definition

nsoranzo reviewed May 4, 2022

View reviewed changes

str | bytes | None

b5ec834

Akuli mentioned this pull request May 7, 2022

Run mypy_primer in all pull requests #7804

Merged

Akuli closed this May 7, 2022

Akuli reopened this May 7, 2022

This comment has been minimized.

Sign in to view

JelleZijlstra reviewed May 8, 2022

View reviewed changes

reverse bytes type on session headers

6f7eb28

+ these are casted to native strings here https://github.com/psf/requests/blob/40956723f27daf5e0d9759208ca69cef236ab339/requests/models.py#L484

This comment has been minimized.

Sign in to view

janrito added 2 commits May 16, 2022 16:58

Revert "reverse bytes type on session headers"

f9aaf0c

This reverts commit 6f7eb28.

Response header dict is always str

0d80bdf

This comment has been minimized.

Sign in to view

AlexWaygood reviewed May 16, 2022

View reviewed changes

janrito added 2 commits May 16, 2022 17:24

PreparedRequests cast headers to native strings as well

072c537

https://github.com/psf/requests/blob/79f2ec3acc4e24fef6e6ce31ad7b1d4e2f77be31/requests/models.py#L493

Fix CaseInsensitiveDict[str | bytes] invariance

f189735

This comment has been minimized.

Sign in to view

Revert "Fix CaseInsensitiveDict[str | bytes] invariance"

ccb434d

This reverts commit f189735.

Akuli approved these changes May 16, 2022

View reviewed changes

Akuli merged commit 87abd8a into python:master May 16, 2022

janrito deleted the requests-session-header-allow-none branch May 17, 2022 09:00

rpdelaney pushed a commit to rpdelaney/downforeveryone that referenced this pull request May 23, 2022

Ignore arg-type on headers

ae5cb19

Due to recent typeshed change: python/typeshed#7773

Akuli mentioned this pull request May 23, 2022

requests: allow immutable headers #7932

Merged

AlexWaygood mentioned this pull request Dec 22, 2022

set session.headers to MutableMapping #9395

Merged

cjwatson mentioned this pull request Feb 6, 2024

requests: Allow passing None header values #11370

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`requests`: Allow session headers to be of type None #7773

`requests`: Allow session headers to be of type None #7773

janrito commented May 3, 2022

Akuli commented May 3, 2022

janrito commented May 3, 2022

janrito commented May 3, 2022 •

edited

Loading

nsoranzo May 3, 2022

janrito May 4, 2022

JelleZijlstra May 4, 2022

janrito May 4, 2022 •

edited

Loading

janrito May 4, 2022

nsoranzo May 4, 2022

janrito May 4, 2022

janrito May 6, 2022

This comment has been minimized.

JelleZijlstra May 8, 2022

janrito May 9, 2022

janrito May 9, 2022

This comment has been minimized.

janrito commented May 13, 2022

Akuli commented May 13, 2022

janrito commented May 16, 2022

This comment has been minimized.

AlexWaygood left a comment •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

AlexWaygood commented May 16, 2022

janrito commented May 16, 2022

github-actions bot commented May 16, 2022

JelleZijlstra commented May 16, 2022

Akuli commented May 16, 2022

Akuli left a comment

rpdelaney commented May 23, 2022 •

edited

Loading

	headers: CaseInsensitiveDict[str \| None]
	headers: CaseInsensitiveDict[str \| bytes \| None]

requests: Allow session headers to be of type None #7773

requests: Allow session headers to be of type None #7773

Conversation

janrito commented May 3, 2022

Akuli commented May 3, 2022

janrito commented May 3, 2022

janrito commented May 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janrito May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

janrito commented May 13, 2022

Akuli commented May 13, 2022

janrito commented May 16, 2022

This comment has been minimized.

AlexWaygood left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

AlexWaygood commented May 16, 2022

janrito commented May 16, 2022

github-actions bot commented May 16, 2022

JelleZijlstra commented May 16, 2022

Akuli commented May 16, 2022

Akuli left a comment

Choose a reason for hiding this comment

rpdelaney commented May 23, 2022 • edited Loading

`requests`: Allow session headers to be of type None #7773

`requests`: Allow session headers to be of type None #7773

janrito commented May 3, 2022 •

edited

Loading

janrito May 4, 2022 •

edited

Loading

AlexWaygood left a comment •

edited

Loading

rpdelaney commented May 23, 2022 •

edited

Loading