Add full request typing signatures for session HTTP methods #8463

max-muoto · 2024-06-15T17:49:32Z

What do these changes do?

Improve the request typing by using 3.11's new Unpack operator to properly type the kwargs that live on each HTTP method. Using typing_extensions, this is supported on older versions of Python as well.

To do this, I did add typing-extensions as a requirement on versions of Python before 3.11. I think this trade-off is well worth it, as it drastically improves the static typing checking experience, and the intellisense experience for VSCode users on Pylance. typing-extensions will also let aiohttp use other typing backports without having to wait multiple years until the minimum supported version Python catches up.

Are there changes in behavior for the user?

Users will be improved type-checking when making requests, if improper/unexpected types are being used, they might notice new typing warnings.

Is it a substantial burden for the maintainers to support this?

No, updates to the request arguments simply need to be mirrored with the typed dict added, which will update all of the core HTTP methods.

Related issue number

Fixes #4749

Checklist

I think the code is well written
Unit tests for the changes exist (not applicable, no logic changes)
Documentation reflects the changes (type-checking changes)
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES/ folder
- name it <issue_or_pr_num>.<type>.rst (e.g. 588.bugfix.rst)
- if you don't have an issue number, change it to the pull request
  number after creating the PR
  - .bugfix: A bug fix for something the maintainers deemed an
    improper undesired behavior that got corrected to match
    pre-agreed expectations.
  - .feature: A new behavior, public APIs. That sort of stuff.
  - .deprecation: A declaration of future API removals and breaking
    changes in behavior.
  - .breaking: When something public is removed in a breaking way.
    Could be deprecated in an earlier release.
  - .doc: Notable updates to the documentation structure or build
    process.
  - .packaging: Notes for downstreams about unobvious side effects
    and tooling. Changes in the test invocation considerations and
    runtime assumptions.
  - .contrib: Stuff that affects the contributor experience. e.g.
    Running tests, building the docs, setting up the development
    environment.
  - .misc: Changes that are hard to assign to any of the above
    categories.
- Make sure to use full sentences with correct case and punctuation,
  for example:
```
Fixed issue with non-ascii contents in doctest text files
-- by :user:`contributor-gh-handle`.
```
  Use the past tense or the present tense a non-imperative mood,
  referring to what's changed compared to the last released version
  of this project.

aiohttp/client.py

codecov · 2024-06-15T18:13:17Z

Codecov Report

Attention: Patch coverage is 80.00000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 97.61%. Comparing base (8504b71) to head (703e89f).
Report is 833 commits behind head on master.

Files with missing lines	Patch %	Lines
aiohttp/client.py	80.00%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8463      +/-   ##
==========================================
- Coverage   97.64%   97.61%   -0.03%     
==========================================
  Files         107      107              
  Lines       33067    33103      +36     
  Branches     3885     3894       +9     
==========================================
+ Hits        32288    32314      +26     
- Misses        562      570       +8     
- Partials      217      219       +2

Flag	Coverage Δ
CI-GHA	`97.53% <80.00%> (-0.03%)`	⬇️
OS-Linux	`97.19% <80.00%> (-0.03%)`	⬇️
OS-Windows	`95.64% <ø> (ø)`
OS-macOS	`96.85% <80.00%> (-0.04%)`	⬇️
Py-3.10.11	`97.00% <80.00%> (-0.04%)`	⬇️
Py-3.10.14	`96.96% <80.00%> (-0.03%)`	⬇️
Py-3.11.9	`97.18% <80.00%> (-0.03%)`	⬇️
Py-3.12.4	`97.30% <80.00%> (-0.03%)`	⬇️
Py-3.8.10	`95.41% <ø> (ø)`
Py-3.8.18	`96.85% <80.00%> (-0.03%)`	⬇️
Py-3.9.13	`96.98% <80.00%> (-0.04%)`	⬇️
Py-3.9.19	`96.94% <80.00%> (-0.03%)`	⬇️
Py-pypy7.3.16	`96.51% <80.00%> (-0.03%)`	⬇️
VM-macos	`96.85% <80.00%> (-0.04%)`	⬇️
VM-ubuntu	`97.19% <80.00%> (-0.03%)`	⬇️
VM-windows	`95.64% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

webknjaz

This is my initial review with infra focus, I'll let @Dreamsorcerer approve the typing bits.

CHANGES/4749.misc.rst

aiohttp/client.py

requirements/base.txt

requirements/base.in

max-muoto · 2024-06-16T16:23:05Z

This is my initial review with infra focus, I'll let @Dreamsorcerer approve the typing bits.

Thanks for the review!

requirements/constraints.txt

aiohttp/client.py

for more information, see https://pre-commit.ci

aiohttp/client.py

…muoto/aiohttp into feat-improve-request-typing

max-muoto · 2024-06-30T20:00:51Z

@Dreamsorcerer Everything is purely 3.11+ overloads (without typing-extensions as a depenency), if you want to take another look now.

aiohttp/client.py

for more information, see https://pre-commit.ci

aiohttp/client.py

…muoto/aiohttp into feat-improve-request-typing

aiohttp/client.py

patchback · 2024-07-06T00:32:25Z

Backport to 3.10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.10/9386854800af026dfa60bcef80615eae1cea94ac/pr-8463

Backported as #8484

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

(cherry picked from commit 9386854)

Dreamsorcerer · 2024-07-06T00:32:44Z

Thanks

… for session HTTP methods (#8484) **This is a backport of PR #8463 as merged into master (9386854).** Co-authored-by: Max Muoto <maxmuoto@gmail.com>

andersk · 2024-08-25T02:47:37Z

aiohttp/client.py

+    auth: Union[BasicAuth, None]
+    allow_redirects: bool
+    max_redirects: int
+    compress: Union[str, None]


This contradicts the documentation for compress as bool:

aiohttp/docs/client_reference.rst

Lines 431 to 434 in 48a5e07

:param bool compress: Set to ``True`` if request has to be compressed

with deflate encoding. If `compress` can not be combined

with a *Content-Encoding* and *Content-Length* headers.

``None`` by default (optional).

It's copied from _request(), not something related to this PR specifically. Feel free to open a PR to fix it. Looking at the code, it's the docs that are inaccurate, though we could expand the typing to allow bool:

aiohttp/aiohttp/client_reqrep.py

Lines 434 to 436 in 48a5e07

elif self.compress:

if not isinstance(self.compress, str):

self.compress = "deflate"

So, basically:
None/False/"" = Disable compression
str = use this compression method
Non-str (e.g. True) = use deflate

I'm not entirely clear how the str is used though, I think maybe only deflate and gzip are valid..

pquentin · 2024-10-03T11:47:25Z

Hello! For what it's worth, those types make it difficult to use keyword arguments and **kwargs. Which is something that libraries building on top of aiohttp will need to do. The only solution I found was to call request() without type errors was to add a class in my own code: elastic/elastic-transport-python#190.

Dreamsorcerer · 2024-10-03T12:12:10Z

I don't think that has anything to do with these types. I'm pretty sure your type error is because you pass (e.g.) data/headers/timeout and then also unpack an arbitrary dict of kwargs. The dict could contain those same keys and therefore cause a runtime error. Using a TypedDict as you've done, just helps ensure that the keys can't overlap.

The only change we've done here is to add typing information so that mypy can give you such errors, instead of having no type checking on that code at all (you'd have seen the same error if we copy/pasted the parameters and didn't use a TypedDict/Unpack).

We could consider exporting that TypedDict though, if it's useful for people..

Dreamsorcerer · 2024-10-03T12:20:20Z

I'd also note that using the TypedDict in your PR is also increasing the type safety of your code regardless. As you're now checking that the ssl argument is of the correct type. The previous code used Any and therefore did no type checking on that parameter.

pquentin · 2024-10-03T12:57:37Z

Thanks for answering! And yes we used Dict[str, Any] to get a release done, prior to that we did not have explicit typing: elastic/elastic-transport-python@93a7d5e. This was fine because aiohttp did not have any either.

We could consider exporting that TypedDict though, if it's useful for people..

It's not in its current form, because it requires filling everything. It would be useful if it included total=False. Not sure if that has other side effects.

Dreamsorcerer · 2024-10-03T13:03:58Z

It's not in its current form, because it requires filling everything. It would be useful if it included total=False.

It does (otherwise your code wouldn't pass, because every parameter would need to be used).

pquentin · 2024-10-03T13:17:36Z

Sorry, I just remembered why this does not work for me: it's not total=False, it's because I also pass keyword arguments directly, as mentioned above. So using _RequestOptions gives me this error:

nox > mypy --strict --show-error-codes elastic_transport/
elastic_transport/_node/_http_aiohttp.py:181: error: "request" of "ClientSession" gets multiple values for keyword argument "data"  [misc]
elastic_transport/_node/_http_aiohttp.py:181: error: "request" of "ClientSession" gets multiple values for keyword argument "headers"  [misc]
elastic_transport/_node/_http_aiohttp.py:181: error: "request" of "ClientSession" gets multiple values for keyword argument "timeout"  [misc]

Anyway, this confirms that my workaround is the best option possible for my use case. Thanks.

max-muoto requested a review from asvetlov as a code owner June 15, 2024 17:49

max-muoto commented Jun 15, 2024

View reviewed changes

aiohttp/client.py Show resolved Hide resolved

max-muoto requested a review from webknjaz as a code owner June 15, 2024 17:52

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jun 15, 2024

max-muoto commented Jun 15, 2024

View reviewed changes

aiohttp/client.py Show resolved Hide resolved

max-muoto changed the title ~~feat: Improve request typing through unpack~~ feat: Full request typing signatures for session HTTP methods Jun 15, 2024

max-muoto changed the title ~~feat: Full request typing signatures for session HTTP methods~~ Add full request typing signatures for session HTTP methods Jun 15, 2024

webknjaz reviewed Jun 16, 2024

View reviewed changes

CHANGES/4749.misc.rst Outdated Show resolved Hide resolved

aiohttp/client.py Outdated Show resolved Hide resolved

requirements/base.txt Outdated Show resolved Hide resolved

webknjaz requested a review from Dreamsorcerer June 16, 2024 11:29

max-muoto commented Jun 16, 2024

View reviewed changes

requirements/base.in Outdated Show resolved Hide resolved

webknjaz reviewed Jun 16, 2024

View reviewed changes

requirements/constraints.txt Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Jun 17, 2024

View reviewed changes

aiohttp/client.py Outdated Show resolved Hide resolved

Dreamsorcerer reviewed Jun 17, 2024

View reviewed changes

aiohttp/client.py Show resolved Hide resolved

Dreamsorcerer reviewed Jun 17, 2024

View reviewed changes

aiohttp/client.py Show resolved Hide resolved

Dreamsorcerer reviewed Jun 17, 2024

View reviewed changes

aiohttp/client.py Outdated Show resolved Hide resolved

Dreamsorcerer added the backport-3.10 Trigger automatic backporting to the 3.10 release branch by Patchback robot label Jun 17, 2024

Add overloads

680d4f5

max-muoto force-pushed the feat-improve-request-typing branch from a5a61a7 to 680d4f5 Compare June 30, 2024 19:49

pre-commit-ci bot and others added 2 commits June 30, 2024 19:49

[pre-commit.ci] auto fixes from pre-commit.com hooks

9212617

for more information, see https://pre-commit.ci

Merge branch 'master' into feat-improve-request-typing

cb998f9

github-advanced-security bot found potential problems Jun 30, 2024

View reviewed changes

aiohttp/client.py Fixed Show fixed Hide fixed

aiohttp/client.py Fixed Show fixed Hide fixed

aiohttp/client.py Fixed Show fixed Hide fixed

aiohttp/client.py Fixed Show fixed Hide fixed

aiohttp/client.py Fixed Show fixed Hide fixed

max-muoto added 2 commits June 30, 2024 14:55

Add changes

2df4510

Merge branch 'feat-improve-request-typing' of https://github.com/max-…

c3051af

…muoto/aiohttp into feat-improve-request-typing

Dreamsorcerer reviewed Jul 4, 2024

View reviewed changes

aiohttp/client.py Outdated Show resolved Hide resolved

Avoid overloading

c8368cf

max-muoto commented Jul 5, 2024

View reviewed changes

aiohttp/client.py Outdated Show resolved Hide resolved

Remove unused import

ac10ab0

[pre-commit.ci] auto fixes from pre-commit.com hooks

3d4155f

for more information, see https://pre-commit.ci

github-advanced-security bot found potential problems Jul 5, 2024

View reviewed changes

max-muoto added 2 commits July 5, 2024 19:16

Address feedback

e4af6b1

Merge branch 'feat-improve-request-typing' of https://github.com/max-…

703e89f

…muoto/aiohttp into feat-improve-request-typing

github-advanced-security bot found potential problems Jul 6, 2024

View reviewed changes

Dreamsorcerer merged commit 9386854 into aio-libs:master Jul 6, 2024
36 of 38 checks passed

patchback bot pushed a commit that referenced this pull request Jul 6, 2024

Add full request typing signatures for session HTTP methods (#8463)

ce798c7

(cherry picked from commit 9386854)

patchback bot mentioned this pull request Jul 6, 2024

[PR #8463/93868548 backport][3.10] Add full request typing signatures for session HTTP methods #8484

Merged

5 tasks

andersk reviewed Aug 25, 2024

View reviewed changes

pquentin mentioned this pull request Sep 30, 2024

Add explicit Python 3.13 support elastic/elastic-transport-python#188

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add full request typing signatures for session HTTP methods #8463

Add full request typing signatures for session HTTP methods #8463

max-muoto commented Jun 15, 2024 •

edited

Loading

codecov bot commented Jun 15, 2024 •

edited

Loading

webknjaz left a comment

max-muoto commented Jun 16, 2024

max-muoto commented Jun 30, 2024 •

edited

Loading

patchback bot commented Jul 6, 2024 •

edited

Loading

Dreamsorcerer commented Jul 6, 2024

andersk Aug 25, 2024

Dreamsorcerer Aug 25, 2024

Dreamsorcerer Aug 25, 2024

Dreamsorcerer Aug 25, 2024

pquentin commented Oct 3, 2024

Dreamsorcerer commented Oct 3, 2024 •

edited

Loading

Dreamsorcerer commented Oct 3, 2024

pquentin commented Oct 3, 2024

Dreamsorcerer commented Oct 3, 2024

pquentin commented Oct 3, 2024 •

edited

Loading

	:param bool compress: Set to ``True`` if request has to be compressed
	with deflate encoding. If `compress` can not be combined
	with a Content-Encoding and Content-Length headers.
	``None`` by default (optional).

	elif self.compress:
	if not isinstance(self.compress, str):
	self.compress = "deflate"

Add full request typing signatures for session HTTP methods #8463

Add full request typing signatures for session HTTP methods #8463

Conversation

max-muoto commented Jun 15, 2024 • edited Loading

What do these changes do?

Are there changes in behavior for the user?

Is it a substantial burden for the maintainers to support this?

Related issue number

Checklist

codecov bot commented Jun 15, 2024 • edited Loading

Codecov Report

webknjaz left a comment

Choose a reason for hiding this comment

max-muoto commented Jun 16, 2024

max-muoto commented Jun 30, 2024 • edited Loading

patchback bot commented Jul 6, 2024 • edited Loading

Backport to 3.10: 💚 backport PR created

Dreamsorcerer commented Jul 6, 2024

andersk Aug 25, 2024

Choose a reason for hiding this comment

Dreamsorcerer Aug 25, 2024

Choose a reason for hiding this comment

Dreamsorcerer Aug 25, 2024

Choose a reason for hiding this comment

Dreamsorcerer Aug 25, 2024

Choose a reason for hiding this comment

pquentin commented Oct 3, 2024

Dreamsorcerer commented Oct 3, 2024 • edited Loading

Dreamsorcerer commented Oct 3, 2024

pquentin commented Oct 3, 2024

Dreamsorcerer commented Oct 3, 2024

pquentin commented Oct 3, 2024 • edited Loading

max-muoto commented Jun 15, 2024 •

edited

Loading

codecov bot commented Jun 15, 2024 •

edited

Loading

max-muoto commented Jun 30, 2024 •

edited

Loading

patchback bot commented Jul 6, 2024 •

edited

Loading

Dreamsorcerer commented Oct 3, 2024 •

edited

Loading

pquentin commented Oct 3, 2024 •

edited

Loading