Skip to content

PYTHON-6028 Consolidate non-IO code in _ClientConnectionRetryable into shared base class - #3173

Open
sleepyStick wants to merge 8 commits into
mongodb:mainfrom
sleepyStick:PYTHON-6028
Open

sleepyStick wants to merge 8 commits into
mongodb:mainfrom
sleepyStick:PYTHON-6028

Conversation

@sleepyStick

Copy link
Copy Markdown
Contributor

PYTHON-6028

Changes in this PR

Test Plan

Checklist

Checklist for Author

  • Did you update the changelog (if necessary)?
  • Is there test coverage?
  • Is any followup work tracked in a JIRA ticket? If so, add link(s).

Checklist for Reviewer

  • Does the title of the PR reference a JIRA Ticket?
  • Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
  • Is all relevant documentation (README or docstring) updated?

@sleepyStick
sleepyStick marked this pull request as ready for review November 4, 2026 18:56
@sleepyStick
sleepyStick requested a review from a team as a code owner November 4, 2026 18:56
@sleepyStick
sleepyStick requested review from NoahStapp and a balanced review from Copilot November 4, 2026 18:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The retry helper subclass lacks __slots__ = (), introducing an instance dictionary and avoidable per-operation memory overhead despite its slotted base.

1 open issue
What changed in this PR

This PR consolidates non-I/O retry logic into a shared base class used by PyMongo’s asynchronous and synchronous clients.

Changes:

  • Moves retry-state initialization, eligibility checks, last-error handling, and retry logging into _AgnosticClientConnectionRetryableBase.
  • Updates both client helpers to inherit shared behavior while retaining their I/O-specific implementations.
File Description
pymongo/​synchronous/​mongo_client.py Updates the generated synchronous helper to inherit the shared base.
pymongo/​mongo_client_shared.py Adds the generic base class containing common retry state and logic.
pymongo/​asynchronous/​mongo_client.py Replaces duplicated non-I/O logic with inheritance from the shared base.

Review effort: Balanced

Comment thread pymongo/asynchronous/mongo_client.py Resolved
Comment thread pymongo/mongo_client_shared.py Outdated
Comment on lines +40 to +46
from pymongo.typings import (
_Address,
_AgnosticBulk,
_AgnosticClientBulk,
_AgnosticClientSession,
_AgnosticMongoClient,
_AgnosticServer,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should all be under if TYPE_CHECKING since they're used purely for type hints.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah good catch, fixed in 485777e

Comment thread pymongo/asynchronous/mongo_client.py Outdated

class _ClientConnectionRetryable(Generic[T]):
class _ClientConnectionRetryable(
_AgnosticClientConnectionRetryableBase[T, "Server", _WriteCall[T] | _ReadCall[T]], Generic[T]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need Generic[T] here since the parent class is already Generic[T].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

makes sense, removed in 485777e

@sleepyStick
sleepyStick requested a review from NoahStapp November 4, 2026 20:38
Comment thread drivers-evergreen-tools

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Intentional to include these changes here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤦 that was entirely accidental 😭
should be gone now!!

@sleepyStick
sleepyStick requested a review from NoahStapp November 5, 2026 00:51

This branch has not been deployed

No deployments
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