Add full request typing signatures for session HTTP methods

[max-muoto]

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.

[codecov]

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.

[max-muoto]

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

Thanks for the review!

[max-muoto]

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

[patchback]

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.

[Dreamsorcerer]

Thanks

[andersk]

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).

[Dreamsorcerer]

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"

[Dreamsorcerer]

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

[Dreamsorcerer]

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

[pquentin]

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]

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]

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]

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]

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]

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.