Skip to content

PYTHON-6023 Consolidate all non-IO code in AsyncCursor/Cursor into _A… - #3045

Open
sleepyStick wants to merge 5 commits into
mongodb:mainfrom
sleepyStick:PYTHON-6023
Open

PYTHON-6023 Consolidate all non-IO code in AsyncCursor/Cursor into _A…#3045
sleepyStick wants to merge 5 commits into
mongodb:mainfrom
sleepyStick:PYTHON-6023

Conversation

@sleepyStick

Copy link
Copy Markdown
Contributor

…gnosticCursor

PYTHON-6023

Changes in this PR

batch_size, _has_next, _post_batch_resume_token, _end_session, _unpack_response, _get_namespace are moved out of AsyncCommandCursor/CommandCursor and into a new shared _AgnosticCommandCursorBase class.

Test Plan

existing tests should continue to pass

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 September 8, 2026 19:42
@sleepyStick
sleepyStick requested a review from a team as a code owner September 8, 2026 19:42
@sleepyStick
sleepyStick requested review from aclark4life and a lite review from Copilot September 8, 2026 19:42

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 refactor widens the sync/async command cursor constructor type hints (and related attribute typing) in a way that is misleading for each API and should be narrowed before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refactors the command cursor implementations by extracting shared, non-I/O logic from AsyncCommandCursor/CommandCursor into a new shared base in pymongo/cursor_shared.py, reducing duplication between sync and async variants.

Changes:

  • Introduces _AgnosticCommandCursorBase in pymongo/cursor_shared.py and moves shared cursor behaviors (__init__, batch_size, token helpers, etc.) into it.
  • Updates both pymongo/asynchronous/command_cursor.py and pymongo/synchronous/command_cursor.py to inherit from the new agnostic base.
  • Adds _AgnosticCollection typing alias to support shared typing across sync/async collection types.
File summaries
File Description
pymongo/typings.py Adds _AgnosticCollection alias for shared sync/async typing.
pymongo/cursor_shared.py Adds _AgnosticCommandCursorBase and migrates shared command-cursor logic into it.
pymongo/asynchronous/command_cursor.py Switches AsyncCommandCursor to inherit from _AgnosticCommandCursorBase.
pymongo/synchronous/command_cursor.py Switches CommandCursor to inherit from _AgnosticCommandCursorBase.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 3
  • Review effort level: Lite

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

Comment on lines +44 to +46
class AsyncCommandCursor(
_AgnosticCommandCursorBase[_DocumentType], _AsyncCursorBase[_DocumentType]
):

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.

maybe i'm misunderstanding the comment, but I if the non-io code is moved into _AgnosticCommandCursorBase, there is no sync vs async so i don't think this matters?

@aclark4life aclark4life Sep 9, 2026

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.

the type-checking value is lost for callers of AsyncCommandCursor / CommandCursor who get  Any / union-typed self._collection instead of the collection doc type. So maybe we do something like:

# pymongo/asynchronous/command_cursor.py
class AsyncCommandCursor(
    _AgnosticCommandCursorBase[_DocumentType], _AsyncCursorBase[_DocumentType]
):
    _collection: AsyncCollection[_DocumentType]
    ...

# pymongo/synchronous/command_cursor.py
class CommandCursor(_AgnosticCommandCursorBase[_DocumentType], _CursorBase[_DocumentType]):
    _collection: Collection[_DocumentType]
    ...

Comment thread pymongo/cursor_shared.py Outdated


class CommandCursor(_CursorBase[_DocumentType]):
class CommandCursor(_AgnosticCommandCursorBase[_DocumentType], _CursorBase[_DocumentType]):
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pymongo/cursor_shared.py 88.23% 3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

Comment on lines +44 to +46
class AsyncCommandCursor(
_AgnosticCommandCursorBase[_DocumentType], _AsyncCursorBase[_DocumentType]
):

@aclark4life aclark4life Sep 9, 2026

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.

the type-checking value is lost for callers of AsyncCommandCursor / CommandCursor who get  Any / union-typed self._collection instead of the collection doc type. So maybe we do something like:

# pymongo/asynchronous/command_cursor.py
class AsyncCommandCursor(
    _AgnosticCommandCursorBase[_DocumentType], _AsyncCursorBase[_DocumentType]
):
    _collection: AsyncCollection[_DocumentType]
    ...

# pymongo/synchronous/command_cursor.py
class CommandCursor(_AgnosticCommandCursorBase[_DocumentType], _CursorBase[_DocumentType]):
    _collection: Collection[_DocumentType]
    ...

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