Skip to content

PYTHON-6027 Consolidate non-IO code in Topology into shared base class - #3107

Merged
sleepyStick merged 7 commits into
mongodb:mainfrom
sleepyStick:PYTHON-6027
Oct 14, 2026
Merged

sleepyStick merged 7 commits into
mongodb:mainfrom
sleepyStick:PYTHON-6027

Conversation

@sleepyStick

@sleepyStick sleepyStick commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

PYTHON-6027

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?

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

Existing test helpers still access the removed name-mangled events executor attribute.

1 open finding
What changed in this PR

Consolidates duplicated synchronous/asynchronous topology logic into a shared base class.

Changes:

  • Adds generic pool typing.
  • Extracts common topology state and methods.
  • Retains I/O-specific behavior in concrete topology classes.
File Description
pymongo/​typings.py Adds shared pool typing.
pymongo/​topology_shared.py Introduces the shared topology base.
pymongo/​synchronous/​topology.py Uses the shared base.
pymongo/​asynchronous/​topology.py Uses the shared base.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pymongo/topology_shared.py
@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.90323% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pymongo/topology_shared.py 91.53% 10 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

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.

🟢 Approval recommended

The refactor preserves behavior across both topology variants, and construction through the shared base succeeds.

1 open finding

🧠 Review effort: Balanced

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.

🟢 Approval recommended

The refactor preserves existing behavior and all affected internal attribute references were updated consistently.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@sleepyStick
sleepyStick marked this pull request as ready for review October 8, 2026 20:27
@sleepyStick
sleepyStick requested a review from a team as a code owner October 8, 2026 20:27
@codeowners-service-app

Copy link
Copy Markdown

Assigned caseyclements for team dbx-python because aclark4life is out of office.



class _ErrorContext:
"""An error with context for SDAM error handling."""

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.

Now a private method so maybe:
"Shared base for the sync and async classes."

@sleepyStick sleepyStick Oct 14, 2026 •

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.

i assume this comment is for _AgnosticTopologyBase? and if so, fixed in 47d6c9c (it didnt make sense for _ErrorContext imo)

@aclark4life aclark4life 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.

LGTM

@sleepyStick
sleepyStick merged commit 07162b9 into mongodb:main Oct 14, 2026
53 of 54 checks passed
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