Refactor ClientRequest #11012

Dreamsorcerer · 2025-05-25T13:05:57Z

Goals

Separate logic between the proxy version and the main version, which have different requirements. This reduces need for asserts etc. where we know an attribute will not be None in one case, but not the other. The proxy code now uses the internal ClientRequestBase, while ClientRequest is for the main request and will be exposed to middlewares.
Rename some attributes/methods to better reflect what middleware users should be accessing (and matching what we've actually documented).
Ensure body is always Payload (it was only not when None was passed previously), and strip out the redundant code that tries to send an empty body.
Make it easier to make additions to ClientRequest in future releases without breaking subclasses.

TODO

If we're happy with this generally, then I still need to fix up the tests, which will take a substantial amount of work.

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

aiohttp/client_reqrep.py

@@ -28,8 +29,9 @@
 from multidict import CIMultiDict, CIMultiDictProxy, MultiDict, MultiDictProxy
 from yarl import URL

-from . import hdrs, helpers, http, multipart, payload
+from . import hdrs, multipart, payload


aiohttp/client_reqrep.py

…into clientrequest-refactor

aiohttp/client_reqrep.py

Dreamsorcerer · 2025-05-25T13:22:38Z

aiohttp/client_reqrep.py

-    body: Any = b""
-    auth = None
-    response = None
+class ClientResponse(HeadersMixin):


There are 0 changes to ClientResponse, I've just moved it up so I can reference it in ClientRequest.

I was under the impression the design was to be able to GC ClientRequest but hold on to ClientResponse

I don't that makes a difference? I've just added a reference to ClientResponse (the class, not an instance) as a class attribute in ClientRequest, so need it defined first.

codecov · 2025-05-25T13:34:08Z

❌ 217 Tests Failed:

Tests completed	Failed	Passed	Skipped
3874	217	3657	40

View the top 3 failed test(s) by shortest run time

tests.test_client_request::test_connection_key_with_proxy

Stack Traces | 0.002s run time

#x1B[0m#x1B[94masync#x1B[39;49;00m #x1B[94mdef#x1B[39;49;00m#x1B[90m #x1B[39;49;00m#x1B[92mtest_connection_key_with_proxy#x1B[39;49;00m() -> #x1B[94mNone#x1B[39;49;00m:#x1B[90m#x1B[39;49;00m
    #x1B[90m    #x1B[39;49;00m#x1B[33m"""Verify the proxy headers are included in the ConnectionKey when a proxy is used."""#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
        proxy = URL(#x1B[33m"#x1B[39;49;00m#x1B[33mhttp://proxy.example.com#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m)#x1B[90m#x1B[39;49;00m
>       req = ClientRequest(#x1B[90m#x1B[39;49;00m
            #x1B[33m"#x1B[39;49;00m#x1B[33mGET#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m,#x1B[90m#x1B[39;49;00m
            URL(#x1B[33m"#x1B[39;49;00m#x1B[33mhttp://example.com#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m),#x1B[90m#x1B[39;49;00m
            proxy=proxy,#x1B[90m#x1B[39;49;00m
            proxy_headers={#x1B[33m"#x1B[39;49;00m#x1B[33mX-Proxy#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m: #x1B[33m"#x1B[39;49;00m#x1B[33mtrue#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m},#x1B[90m#x1B[39;49;00m
            loop=asyncio.get_running_loop(),#x1B[90m#x1B[39;49;00m
        )#x1B[90m#x1B[39;49;00m
#x1B[1m#x1B[31mE       TypeError: ClientRequest.__init__() missing 18 required keyword-only arguments: 'params', 'headers', 'skip_auto_headers', 'data', 'cookies', 'auth', 'version', 'compress', 'chunked', 'expect100', 'response_class', 'proxy_auth', 'timer', 'session', 'ssl', 'traces', 'trust_env', and 'server_hostname'#x1B[0m

proxy      = URL('http://proxy.example.com')

#x1B[1m#x1B[31mtests/test_client_request.py#x1B[0m:1524: TypeError

tests.test_client_request::test_get_with_data[pyloop]

Stack Traces | 0.002s run time

loop = <_UnixSelectorEventLoop running=False closed=False debug=False>

    #x1B[0m#x1B[94masync#x1B[39;49;00m #x1B[94mdef#x1B[39;49;00m#x1B[90m #x1B[39;49;00m#x1B[92mtest_get_with_data#x1B[39;49;00m(loop: asyncio.AbstractEventLoop) -> #x1B[94mNone#x1B[39;49;00m:#x1B[90m#x1B[39;49;00m
        #x1B[94mfor#x1B[39;49;00m meth #x1B[95min#x1B[39;49;00m ClientRequest.GET_METHODS:#x1B[90m#x1B[39;49;00m
>           req = ClientRequest(#x1B[90m#x1B[39;49;00m
                meth, URL(#x1B[33m"#x1B[39;49;00m#x1B[33mhttp://python.org/#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m), data={#x1B[33m"#x1B[39;49;00m#x1B[33mlife#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m: #x1B[33m"#x1B[39;49;00m#x1B[33m42#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m}, loop=loop#x1B[90m#x1B[39;49;00m
            )#x1B[90m#x1B[39;49;00m
#x1B[1m#x1B[31mE           TypeError: ClientRequest.__init__() missing 19 required keyword-only arguments: 'params', 'headers', 'skip_auto_headers', 'cookies', 'auth', 'version', 'compress', 'chunked', 'expect100', 'response_class', 'proxy', 'proxy_auth', 'timer', 'session', 'ssl', 'proxy_headers', 'traces', 'trust_env', and 'server_hostname'#x1B[0m

loop       = <_UnixSelectorEventLoop running=False closed=False debug=False>
meth       = 'OPTIONS'

#x1B[1m#x1B[31mtests/test_client_request.py#x1B[0m:810: TypeError

tests.test_client_request::test_connection_key_without_proxy

Stack Traces | 0.003s run time

#x1B[0m#x1B[94masync#x1B[39;49;00m #x1B[94mdef#x1B[39;49;00m#x1B[90m #x1B[39;49;00m#x1B[92mtest_connection_key_without_proxy#x1B[39;49;00m() -> #x1B[94mNone#x1B[39;49;00m:#x1B[90m#x1B[39;49;00m
    #x1B[90m    #x1B[39;49;00m#x1B[33m"""Verify the proxy headers are not included in the ConnectionKey when a proxy is used."""#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
        #x1B[90m# If proxy is unspecified, proxy_headers should be ignored#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
>       req = ClientRequest(#x1B[90m#x1B[39;49;00m
            #x1B[33m"#x1B[39;49;00m#x1B[33mGET#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m,#x1B[90m#x1B[39;49;00m
            URL(#x1B[33m"#x1B[39;49;00m#x1B[33mhttp://example.com#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m),#x1B[90m#x1B[39;49;00m
            proxy_headers={#x1B[33m"#x1B[39;49;00m#x1B[33mX-Proxy#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m: #x1B[33m"#x1B[39;49;00m#x1B[33mtrue#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m},#x1B[90m#x1B[39;49;00m
            loop=asyncio.get_running_loop(),#x1B[90m#x1B[39;49;00m
        )#x1B[90m#x1B[39;49;00m
#x1B[1m#x1B[31mE       TypeError: ClientRequest.__init__() missing 19 required keyword-only arguments: 'params', 'headers', 'skip_auto_headers', 'data', 'cookies', 'auth', 'version', 'compress', 'chunked', 'expect100', 'response_class', 'proxy', 'proxy_auth', 'timer', 'session', 'ssl', 'traces', 'trust_env', and 'server_hostname'#x1B[0m


#x1B[1m#x1B[31mtests/test_client_request.py#x1B[0m:1538: TypeError

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

bdraco · 2025-05-25T13:34:51Z

aiohttp/client_reqrep.py

@@ -190,623 +193,352 @@ class ConnectionKey(NamedTuple):
    proxy_headers_hash: Optional[int]  # hash(CIMultiDict)


-class ClientRequest:


Maybe we can back-port a limited version of this to keep back-ports more manageable. Right now automatic back-porting fails far too often, and we aren't any closer to releasing 4.x so we are going to have to deal with that for a long time which makes the risk of back-port errors higher.

Maybe. Will have to move backportable parts to another PR and merge, then update this one.

codspeed-hq · 2025-05-25T14:08:59Z

CodSpeed Performance Report

Merging #11012 will degrade performances by 9.34%

_{Comparing clientrequest-refactor (0cdf184) with master (4824648)}

Summary

❌ 1 regressions
✅ 58 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
❌	`test_send_client_request_one_hundred[pyloop]`	2.5 ms	2.8 ms	-9.34%

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

tests/test_benchmarks_client_request.py

…into clientrequest-refactor

aiohttp/client_reqrep.py

…into clientrequest-refactor

bdraco · 2025-05-26T06:56:30Z

Sorry about the conflicts

Refactor ClientRequest

18564e9

Dreamsorcerer requested a review from asvetlov as a code owner May 25, 2025 13:05

Dreamsorcerer added the backport:skip Skip backport bot label May 25, 2025

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

f4d733f

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

github-advanced-security bot found potential problems May 25, 2025

View reviewed changes

Dreamsorcerer added 2 commits May 25, 2025 14:11

Add kwargs

22d724a

Merge branch 'clientrequest-refactor' of github.com:aio-libs/aiohttp …

6fa8996

…into clientrequest-refactor

Dreamsorcerer commented May 25, 2025

View reviewed changes

aiohttp/client_reqrep.py Outdated Show resolved Hide resolved

Tweaks

8257048

Dreamsorcerer commented May 25, 2025

View reviewed changes

Dreamsorcerer added 2 commits May 25, 2025 14:26

Fix

7171e0a

Fix

9023653

bdraco reviewed May 25, 2025

View reviewed changes

Dreamsorcerer added 2 commits May 25, 2025 14:50

Fix benchmarks

6a1e81a

Fix benchmarks

ef47363

Dreamsorcerer and others added 2 commits May 25, 2025 15:56

Remove unneeded code

477d52d

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

8fbac96

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

github-advanced-security bot found potential problems May 25, 2025

View reviewed changes

tests/test_benchmarks_client_request.py Fixed Show fixed Hide fixed

Dreamsorcerer added 3 commits May 25, 2025 16:06

Fix

cb5f104

Merge branch 'clientrequest-refactor' of github.com:aio-libs/aiohttp …

3820c39

…into clientrequest-refactor

Merge branch 'master' into clientrequest-refactor

2559a0b

Dreamsorcerer commented May 25, 2025

View reviewed changes

aiohttp/client_reqrep.py Show resolved Hide resolved

Dreamsorcerer added 3 commits May 25, 2025 17:32

Readd shortcut

19dfe3a

Merge branch 'clientrequest-refactor' of github.com:aio-libs/aiohttp …

580387b

…into clientrequest-refactor

Tweaks

0cdf184

Dreamsorcerer mentioned this pull request May 26, 2025

Move ClientResponse to top of file #11025

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Refactor ClientRequest #11012

Refactor ClientRequest #11012

Uh oh!

Dreamsorcerer commented May 25, 2025 •

edited

Loading

Uh oh!

Check notice

Copilot Autofix

Uh oh!

Uh oh!

Dreamsorcerer May 25, 2025

Uh oh!

bdraco May 25, 2025

Uh oh!

Dreamsorcerer May 25, 2025

Uh oh!

codecov bot commented May 25, 2025 •

edited

Loading

Uh oh!

bdraco May 25, 2025

Uh oh!

Dreamsorcerer May 25, 2025

Uh oh!

codspeed-hq bot commented May 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

bdraco commented May 26, 2025

Uh oh!

Uh oh!

		@@ -190,623 +193,352 @@ class ConnectionKey(NamedTuple):
		proxy_headers_hash: Optional[int] # hash(CIMultiDict)


		class ClientRequest:

Uh oh!

Refactor ClientRequest #11012

Are you sure you want to change the base?

Refactor ClientRequest #11012

Uh oh!

Conversation

Dreamsorcerer commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Goals

TODO

Uh oh!

Check notice

Uh oh!

Copilot Autofix

Uh oh!

Uh oh!

Dreamsorcerer May 25, 2025

Choose a reason for hiding this comment

Uh oh!

bdraco May 25, 2025

Choose a reason for hiding this comment

Uh oh!

Dreamsorcerer May 25, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 217 Tests Failed:

Uh oh!

bdraco May 25, 2025

Choose a reason for hiding this comment

Uh oh!

Dreamsorcerer May 25, 2025

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #11012 will degrade performances by 9.34%

Summary

Benchmarks breakdown

Uh oh!

Uh oh!

Uh oh!

bdraco commented May 26, 2025

Uh oh!

Uh oh!

Dreamsorcerer commented May 25, 2025 •

edited

Loading

codecov bot commented May 25, 2025 •

edited

Loading

codspeed-hq bot commented May 25, 2025 •

edited

Loading