Skip to content

fix: close sockets when synchronous driver setup fails - #1482

Open
Shubham-Padkonde wants to merge 2 commits into
GoogleCloudPlatform:mainfrom
Shubham-Padkonde:fix/close-failed-driver-sockets
Open

Shubham-Padkonde wants to merge 2 commits into
GoogleCloudPlatform:mainfrom
Shubham-Padkonde:fix/close-failed-driver-sockets

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

When a synchronous driver raises after the connector opens its TLS socket, the failed connection leaves that socket open. Domain-based connections also retain it in the failover tracking list. For example, the pg8000 adapter raises KeyError for a missing database argument before handing the socket to the driver.

Close the socket and remove it from failover tracking when driver setup raises, then preserve the existing exception and refresh behavior. Successful connections keep their sockets open and tracked.

Tests exercise the real pg8000 adapter failure for direct and domain connections; both regressions fail before the fix. The existing successful domain-connection test now also checks that its socket stays open.

Validation:

  • Linux, Python 3.12: all 201 unit tests pass; 99% coverage.
  • Ruff and mypy pass.
  • Windows, Python 3.14: 171 passed, 24 skipped, 6 failed in socket/local-server tests; the new regressions pass. Linux is the complete passing validation.
  • Live Cloud SQL integration tests were not run.

@Shubham-Padkonde
Shubham-Padkonde requested a review from a team as a code owner October 4, 2026 07:43
hessjcg
hessjcg previously approved these changes Oct 12, 2026

@hessjcg hessjcg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Thank you for the contribution!

kgala2
kgala2 previously approved these changes Oct 12, 2026
@kgala2

kgala2 commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

/gcbrun

@Shubham-Padkonde
Shubham-Padkonde dismissed stale reviews from kgala2 and hessjcg via 541c12e October 17, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants