Update vendored Requests to 2.34.2#13982
Conversation
50c73b6 to
ed0104f
Compare
|
Thanks for this! |
|
I'll push up one more quick cleanup piece, but then can take it out of draft. There's a couple open questions about how I solved the typing conflicts but otherwise should be set for when we cut 2.34.0. Will just need to update version numbers. |
ed0104f to
8c3e17e
Compare
nateprewitt
left a comment
There was a problem hiding this comment.
Ok, should be good for a look.
| return | ||
|
|
||
| raise _NotAPIContent(content_type, response.request.method) | ||
| raise _NotAPIContent(content_type, response.request.method) # type: ignore[arg-type] |
There was a problem hiding this comment.
response.request.method is potentially None here but that will never happen in pip's usage. The easiest option here is to ignore the None case. Otherwise we could handle None plumbing in the exception or add an or "" fallback.
| ) -> None: | ||
| if self._ssl_context is not None: | ||
| pool_kwargs.setdefault("ssl_context", self._ssl_context) | ||
| return super().init_poolmanager( # type: ignore[misc, no-any-return] | ||
| super().init_poolmanager( # type: ignore[misc] |
There was a problem hiding this comment.
This gets set on the Session and shouldn't return. It looks like this got added when truststore was added but isn't actually used.
| request: MockRequest | ||
| connection: MockConnection | ||
| url: str | ||
| class MockResponse(Response): |
There was a problem hiding this comment.
I've subclassed the mock to satisfy typing. The changes here are centralized, the other option is we'll need to suppress usage in ~6 places.
|
|
||
|
|
||
| def message_from_dict(headers: dict[str, HeaderValue]) -> Message: | ||
| def message_from_dict(headers: Mapping[str, HeaderValue]) -> Message: |
There was a problem hiding this comment.
This is because CaseInsensitiveDict isn't a dict, we need a wider type. Mapping or MutableMapping works, since we're only reading I went with the most specific.
| "Tag": ["-".join(parts) for parts in tags], | ||
| } | ||
| ) | ||
| wheel_info: dict[str, HeaderValue] = { |
There was a problem hiding this comment.
mypy infers typing here to be dict[str, str | Sequence[str]] because of "Tag". We know it's HeaderValue (str | list[str]) but need to coerce the ambiguity.
| resp = MockResponse(b"") | ||
| resp.url = url | ||
| resp.headers = headers | ||
| resp.headers.update(headers) |
There was a problem hiding this comment.
{Prepared}Request and Response both use CaseInsensitiveDict for their headers. Setting it to dict works but has some subtle failure modes when casing differs. The correct way to do this is either update the existing instance or create a new one.
| _real_session = PipSession() | ||
|
|
||
| def _fake_session_get(*args: Any, **kwargs: Any) -> dict[str, str]: | ||
| def _fake_session_get(*args: Any, **kwargs: Any) -> Response: |
There was a problem hiding this comment.
Not sure why this was typed dict[str, str]. It looks like it's always been a Response.
| --- a/src/pip/_vendor/requests/compat.py | ||
| +++ b/src/pip/_vendor/requests/compat.py | ||
| @@ -7,7 +7,6 @@ between Python 2 and Python 3. It remains for backwards | ||
| compatibility until the next major version. | ||
| """ | ||
|
|
||
| -import importlib | ||
| import sys | ||
|
|
||
| # ------- | ||
| @@ -30,12 +29,6 @@ import sys | ||
| def _resolve_char_detection(): | ||
| """Find supported character detection libraries.""" | ||
| chardet = None | ||
| - for lib in ("chardet", "charset_normalizer"): | ||
| - if chardet is None: | ||
| - try: | ||
| - chardet = importlib.import_module(lib) | ||
| - except ImportError: | ||
| - pass | ||
| return chardet | ||
|
|
||
|
|
There was a problem hiding this comment.
Some of the patches to the same files are spread out. I had this issue last time I updated the vendored copy where you needed to do multiple diffs. I've consolidated them together for each file for simplicity of future updates since I was already touching everything. Let me know if you'd prefer I go back to the old way.
This PR is meant to prepare a patch ahead of time for
pipwith Requests' migration to inline typing. This was originally raised in #13966.Some types within
pipwill need to be updated to match what's being released in Requests. This PR will start in draft with only the necessary vendoring updates and then have the types added as a second commit to isolate the changes.