Skip to content

PYTHON-3119 getMore helper should explicitly send inherited comment #904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 25, 2022
Merged

PYTHON-3119 getMore helper should explicitly send inherited comment #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
62cf67d
initial commit, for some reason some spec tests not passing
juliusgeo Mar 17, 2022
1ae5c26
add in more routing to hopefully make it work
juliusgeo Mar 18, 2022
895541a
add support for raw command cursors
juliusgeo Mar 18, 2022
476ceb6
should fix last remaining test errors
juliusgeo Mar 18, 2022
ed6abd3
missed a typo
juliusgeo Mar 18, 2022
d650ba8
forgot to pull when I resynced tests, lol
juliusgeo Mar 19, 2022
b8398fe
add a skiptest because comments on getmores aren't supported on earli…
juliusgeo Mar 19, 2022
a960050
fix blank lines added/removed
juliusgeo Mar 22, 2022
cfc97bd
only send comment on getmore with wire version >9
juliusgeo Mar 24, 2022
fc9b637
updated spec tests from Dmitry's PR
juliusgeo Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ pymongo.egg-info/
.tox
mongocryptd.pid
.idea/
.nova/
Copy link
Member

Choose a reason for hiding this comment

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

what is .nova?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is for my IDE. I can add it to my .git/info/exclude file if you'd rather not add it to the repository .gitignore.

Copy link
Member

Choose a reason for hiding this comment

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

It's fine here, I'm curious what IDE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nova--it's by the same people who made the FTP client Transfer.

1 change: 1 addition & 0 deletions pymongo/aggregation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def get_cursor(self, session, server, sock_info, read_preference):
max_await_time_ms=self._max_await_time_ms,
session=session,
explicit_session=self._explicit_session,
comment=self._options.get("comment"),
)
cmd_cursor._maybe_pin_connection(sock_info)
return cmd_cursor
Expand Down
7 changes: 6 additions & 1 deletion pymongo/collection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2180,7 +2180,12 @@ def _cmd(session, server, sock_info, read_preference):
raise
cursor = {"id": 0, "firstBatch": []}
cmd_cursor = CommandCursor(
coll, cursor, sock_info.address, session=session, explicit_session=explicit_session
coll,
cursor,
sock_info.address,
session=session,
explicit_session=explicit_session,
comment=cmd.get("comment"),
)
cmd_cursor._maybe_pin_connection(sock_info)
return cmd_cursor
Expand Down
5 changes: 5 additions & 0 deletions pymongo/command_cursor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def __init__(
max_await_time_ms: Optional[int] = None,
session: Optional["ClientSession"] = None,
explicit_session: bool = False,
comment: Any = None,
) -> None:
"""Create a new command cursor."""
self.__sock_mgr: Any = None
Expand All @@ -56,6 +57,7 @@ def __init__(
self.__session = session
self.__explicit_session = explicit_session
self.__killed = self.__id == 0
self.__comment = comment
if self.__killed:
self.__end_session(True)

Expand Down Expand Up @@ -224,6 +226,7 @@ def _refresh(self):
self.__max_await_time_ms,
self.__sock_mgr,
False,
self.__comment,
)
)
else: # Cursor id is zero nothing else to return
Expand Down Expand Up @@ -314,6 +317,7 @@ def __init__(
max_await_time_ms: Optional[int] = None,
session: Optional["ClientSession"] = None,
explicit_session: bool = False,
comment: Any = None,
) -> None:
"""Create a new cursor / iterator over raw batches of BSON data.

Expand All @@ -332,6 +336,7 @@ def __init__(
max_await_time_ms,
session,
explicit_session,
comment,
)

def _unpack_response(
Expand Down
1 change: 1 addition & 0 deletions pymongo/cursor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,7 @@ def _refresh(self):
self.__max_await_time_ms,
self.__sock_mgr,
self.__exhaust,
self.__comment,
)
self.__send_message(g)

Expand Down
1 change: 1 addition & 0 deletions pymongo/database.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,7 @@ def _list_collections(self, sock_info, session, read_preference, **kwargs):
sock_info.address,
session=tmp_session,
explicit_session=session is not None,
comment=cmd.get("comment"),
)
cmd_cursor._maybe_pin_connection(sock_info)
return cmd_cursor
Expand Down
15 changes: 12 additions & 3 deletions pymongo/message.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,15 @@ def _gen_find_command(
return cmd


def _gen_get_more_command(cursor_id, coll, batch_size, max_await_time_ms):
def _gen_get_more_command(cursor_id, coll, batch_size, max_await_time_ms, comment, sock_info):
"""Generate a getMore command document."""
cmd = SON([("getMore", cursor_id), ("collection", coll)])
if batch_size:
cmd["batchSize"] = batch_size
if max_await_time_ms is not None:
cmd["maxTimeMS"] = max_await_time_ms
if comment is not None and sock_info.max_wire_version >= 9:
cmd["comment"] = comment
return cmd


Expand Down Expand Up @@ -421,6 +423,7 @@ class _GetMore(object):
"sock_mgr",
"_as_command",
"exhaust",
"comment",
)

name = "getMore"
Expand All @@ -438,6 +441,7 @@ def __init__(
max_await_time_ms,
sock_mgr,
exhaust,
comment,
):
self.db = db
self.coll = coll
Expand All @@ -451,6 +455,7 @@ def __init__(
self.sock_mgr = sock_mgr
self._as_command = None
self.exhaust = exhaust
self.comment = comment

def namespace(self):
return "%s.%s" % (self.db, self.coll)
Expand All @@ -473,9 +478,13 @@ def as_command(self, sock_info):
return self._as_command

cmd = _gen_get_more_command(
self.cursor_id, self.coll, self.ntoreturn, self.max_await_time_ms
self.cursor_id,
self.coll,
self.ntoreturn,
self.max_await_time_ms,
self.comment,
sock_info,
)

if self.session:
self.session._apply_to(cmd, False, self.read_preference, sock_info)
sock_info.add_server_api(cmd)
Expand Down
2 changes: 1 addition & 1 deletion pymongo/mongo_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1763,7 +1763,7 @@ def list_databases(
"firstBatch": res["databases"],
"ns": "admin.$cmd",
}
return CommandCursor(admin["$cmd"], cursor, None)
return CommandCursor(admin["$cmd"], cursor, None, comment=comment)

def list_database_names(
self,
Expand Down
179 changes: 179 additions & 0 deletions test/change_streams/unified/change-streams.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,185 @@
]
}
]
},
{
"description": "Test that comment is set on getMore",
"runOnRequirements": [
{
"minServerVersion": "4.4.0",
"topologies": [
"replicaset"
]
}
],
"operations": [
{
"name": "createChangeStream",
"object": "collection0",
"arguments": {
"pipeline": [],
"comment": {
"key": "value"
}
},
"saveResultAsEntity": "changeStream0"
},
{
"name": "insertOne",
"object": "collection0",
"arguments": {
"document": {
"_id": 1,
"a": 1
}
}
},
{
"name": "iterateUntilDocumentOrError",
"object": "changeStream0"
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"aggregate": "collection0",
"pipeline": [
{
"$changeStream": {}
}
],
"comment": {
"key": "value"
}
}
}
},
{
"commandStartedEvent": {
"command": {
"insert": "collection0",
"documents": [
{
"_id": 1,
"a": 1
}
]
}
}
},
{
"commandStartedEvent": {
"command": {
"getMore": {
"$$type": [
"int",
"long"
]
},
"collection": "collection0",
"comment": {
"key": "value"
}
},
"commandName": "getMore",
"databaseName": "database0"
}
}
]
}
]
},
{
"description": "Test that comment is not set on getMore - pre 4.4",
"runOnRequirements": [
{
"minServerVersion": "3.6.0",
"maxServerVersion": "4.3.99",
"topologies": [
"replicaset"
]
}
],
"operations": [
{
"name": "createChangeStream",
"object": "collection0",
"arguments": {
"pipeline": [],
"comment": "comment"
},
"saveResultAsEntity": "changeStream0"
},
{
"name": "insertOne",
"object": "collection0",
"arguments": {
"document": {
"_id": 1,
"a": 1
}
}
},
{
"name": "iterateUntilDocumentOrError",
"object": "changeStream0"
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"aggregate": "collection0",
"pipeline": [
{
"$changeStream": {}
}
],
"comment": "comment"
}
}
},
{
"commandStartedEvent": {
"command": {
"insert": "collection0",
"documents": [
{
"_id": 1,
"a": 1
}
]
}
}
},
{
"commandStartedEvent": {
"command": {
"getMore": {
"$$type": [
"int",
"long"
]
},
"collection": "collection0",
"comment": {
"$$exists": false
}
},
"commandName": "getMore",
"databaseName": "database0"
}
}
]
}
]
}
]
}
Loading