PYTHON-6023 Consolidate all non-IO code in AsyncCursor/Cursor into _A… - #3045
PYTHON-6023 Consolidate all non-IO code in AsyncCursor/Cursor into _A…#3045sleepyStick wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
🟡 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
_AgnosticCommandCursorBaseinpymongo/cursor_shared.pyand moves shared cursor behaviors (__init__,batch_size, token helpers, etc.) into it. - Updates both
pymongo/asynchronous/command_cursor.pyandpymongo/synchronous/command_cursor.pyto inherit from the new agnostic base. - Adds
_AgnosticCollectiontyping 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.
| class AsyncCommandCursor( | ||
| _AgnosticCommandCursorBase[_DocumentType], _AsyncCursorBase[_DocumentType] | ||
| ): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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]
...
|
|
||
|
|
||
| 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 Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| class AsyncCommandCursor( | ||
| _AgnosticCommandCursorBase[_DocumentType], _AsyncCursorBase[_DocumentType] | ||
| ): |
There was a problem hiding this comment.
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]
...
…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
Checklist for Reviewer