Avoid redundant _release_connection calls#12221
Avoid redundant _release_connection calls#12221rodrigobnogueira wants to merge 1 commit intoaio-libs:masterfrom
Conversation
2be3b22 to
a68aac5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12221 +/- ##
==========================================
- Coverage 99.11% 99.10% -0.01%
==========================================
Files 130 130
Lines 45324 45359 +35
Branches 2392 2394 +2
==========================================
+ Hits 44921 44955 +34
Misses 271 271
- Partials 132 133 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
a09c0e2 to
0a29170
Compare
aiohttp/client_reqrep.py
Outdated
|
|
||
| self._cleanup_writer() | ||
| self._release_connection() | ||
| if self._connection is not None: |
There was a problem hiding this comment.
This check already happens inside _release_connection(). Is it the function call itself that's adding too much overhead? I'll leave @bdraco to review this one.
There was a problem hiding this comment.
@Dreamsorcerer , I've opened this one because I thought there was some job in the CI that could make the change relevant.
The statement if self._connection is not None: was already in _release_connection when the issue was opened.
Issue #10089 opened: 2024-12-02. release_connection() itself was introduced in 9c07121 (2023-11-03) with the same internal guard from the start.
def _release_connection(self) -> None:
if self._connection is not None:
...Unless I'm missing something, I think this PR and the issue can be closed.
There was a problem hiding this comment.
Yeah, that's my question to @bdraco. If you see my comment there, I think it may be possible to remove one of these calls entirely, but the other 3 need to remain.
0a29170 to
f9ef93d
Compare
f9ef93d to
e487283
Compare
What do these changes do?
This change removes redundant ClientResponse._release_connection() invocations on successful request paths.
Are there changes in behavior for the user?
Yes, internal behavior changed: a successful request now executes one release-path call per response instead of repeated no-op release calls. Connection reuse/cleanup semantics are preserved.
Is it a substantial burden for the maintainers to support this?
No. The change is small, localized, and covered by regression + existing suite coverage.
Related issue number
Fixes #10089
Checklist