Skip to content

[PR #3733/6b0bc4ef backport][3.13] Fix connector not waiting for connections to close #11074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
May 29, 2025
Merged

[PR #3733/6b0bc4ef backport][3.13] Fix connector not waiting for connections to close #11074

merged 8 commits into from
May 29, 2025

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented May 28, 2025 •
edited
Loading

What do these changes do?

This PR fixes an issue where BaseConnector.close() would return immediately without waiting for connections to actually close. This is a partial backport of #3733 that includes only the bug fix without any breaking API changes.

The fix ensures that close() properly waits for all connection closures to complete by:

  • Collecting all proto.closed futures from connections being closed
  • Using asyncio.gather() to wait for all closures to complete
  • Optimizing for Python 3.12+ by using eager task execution when available

Are there changes in behavior for the user?

No breaking changes. The close() method will now properly wait for connections to close as expected, which is the correct behavior. The method continues to return an awaitable that emits a deprecation warning if not awaited.

Note: When using the connector as a non-async context manager (deprecated pattern), the issue of not waiting for connections to close still remains. This is because __exit__ calls _close() synchronously and cannot await the connection closures. Users should use the async context manager pattern (async with) instead. We cannot remove the sync context manager in 3.x as that would be a breaking change.

Related issue number

Partial backport of #3733
fixes #1925 fixes #3736

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES/ folde

@bdraco
Wait for connections to close in connector.close()
15695d6 
Thie is a partial backport of #3733 without the breaking
changes

fixes #1925
@bdraco bdraco changed the title Wait for connections to close in connector.close() [3.13] Wait for connections to close in connector.close() May 28, 2025
@bdraco bdraco changed the title [3.13] Wait for connections to close in connector.close() [3.13] Fix connector not waiting for connections to close May 28, 2025
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label May 28, 2025
@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented May 28, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #11074 will not alter performance

Comparing backport_close_fix (9998fb1) with 3.13 (05d39fe)

Summary

✅ 59 untouched benchmarks

@codecov Codecov
Copy link

codecov bot commented May 28, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.24%. Comparing base (05d39fe) to head (9998fb1).
Report is 1 commits behind head on 3.13.

Additional details and impacted files 
@@           Coverage Diff           @@
##             3.13   #11074   +/-   ##
=======================================
  Coverage   98.24%   98.24%           
=======================================
  Files         130      130           
  Lines       41612    41664   +52     
  Branches     2286     2291    +5     
=======================================
+ Hits        40880    40933   +53     
  Misses        557      557           
+ Partials      175      174    -1
Flag Coverage Δ
CI-GHA 98.13% <98.82%> (+<0.01%) ⬆️
OS-Linux 97.87% <98.82%> (-0.01%) ⬇️
OS-Windows 95.47% <98.82%> (+0.01%) ⬆️
OS-macOS 97.08% <98.82%> (-0.01%) ⬇️
Py-3.10.11 96.97% <97.64%> (+<0.01%) ⬆️
Py-3.10.17 97.47% <97.64%> (-0.02%) ⬇️
Py-3.11.12 97.56% <97.64%> (+0.02%) ⬆️
Py-3.11.9 97.05% <97.64%> (-0.01%) ⬇️
Py-3.12.10 97.92% <97.64%> (-0.01%) ⬇️
Py-3.13.3 97.92% <97.64%> (+0.01%) ⬆️
Py-3.9.13 96.86% <97.64%> (+<0.01%) ⬆️
Py-3.9.22 97.37% <97.64%> (-0.02%) ⬇️
Py-pypy7.3.16 83.17% <97.64%> (-1.57%) ⬇️
VM-macos 97.08% <98.82%> (-0.01%) ⬇️
VM-ubuntu 97.87% <98.82%> (-0.01%) ⬇️
VM-windows 95.47% <98.82%> (+0.01%) ⬆️

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.

@bdraco bdraco added backport-3.11 backport-3.12 Trigger automatic backporting to the 3.12 release branch by Patchback robot labels May 28, 2025
@bdraco bdraco changed the title [3.13] Fix connector not waiting for connections to close [PR #3733/6b0bc4ef backport][3.13] Fix connector not waiting for connections to close May 28, 2025
@bdraco bdraco marked this pull request as ready for review May 29, 2025 00:00
@bdraco bdraco requested review from webknjaz and asvetlov as code owners May 29, 2025 00:00
@bdraco bdraco merged commit e550c78 into 3.13 May 29, 2025
36 checks passed
@bdraco bdraco deleted the backport_close_fix branch May 29, 2025 00:01
@patchback Patchback
Copy link
Contributor

patchback bot commented May 29, 2025 •
edited
Loading

Backport to 3.11: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.11/e550c78a943be34ffd01b58acbcc0f06485bbbd9/pr-11074

Backported as #11076

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

patchback bot pushed a commit that referenced this pull request May 29, 2025
@bdraco @patchback
[PR #3733/6b0bc4ef backport][3.13] Fix connector not waiting for conn…
6c4447d 
…ections to close (#11074)

fixes #1925 fixes #3736

(cherry picked from commit e550c78)
@patchback patchback bot mentioned this pull request May 29, 2025
Merged
5 tasks
@patchback Patchback
Copy link
Contributor

patchback bot commented May 29, 2025 •
edited
Loading

Backport to 3.12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.12/e550c78a943be34ffd01b58acbcc0f06485bbbd9/pr-11074

Backported as #11077

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

patchback bot pushed a commit that referenced this pull request May 29, 2025
@bdraco @patchback
[PR #3733/6b0bc4ef backport][3.13] Fix connector not waiting for conn…
016f478 
…ections to close (#11074)

fixes #1925 fixes #3736

(cherry picked from commit e550c78)
@patchback patchback bot mentioned this pull request May 29, 2025
Merged
5 tasks
bdraco added a commit that referenced this pull request May 29, 2025
@patchback @bdraco
[PR #11074/e550c78a backport][3.11] Fix connector not waiting for con…
9f6c54d 
…nections to close (#11076)

Co-authored-by: J. Nick Koston <nick@koston.org>
fixes #1925 fixes #3736
bdraco added a commit that referenced this pull request May 29, 2025
@patchback @bdraco
[PR #11074/e550c78a backport][3.12] Fix connector not waiting for con…
0abffd5 
…nections to close (#11077)

Co-authored-by: J. Nick Koston <nick@koston.org>
fixes #1925 fixes #3736
@bdraco bdraco mentioned this pull request May 30, 2025
Closed
1 task
@bdraco bdraco mentioned this pull request May 31, 2025
Merged
This was referenced May 31, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webknjaz webknjaz Awaiting requested review from webknjaz webknjaz is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

Assignees
No one assigned
Labels
backport-3.12 Trigger automatic backporting to the 3.12 release branch by Patchback robot bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@bdraco