-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5344 and PYTHON-5403 Allow Instantiated MongoClients to Send Client Metadata On-Demand #2358
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.
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
Changes from 15 commits
b1899d6
e18737f
a17f6ef
f9a097a
21c4eef
9f9c9d8
265ef3b
8a9c4cb
a4d1116
42e4b81
92b3046
08dbbf7
c54a7da
1c835ae
082c4ab
8c49277
0217c9c
c6a8975
825f8ed
f635625
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,225 @@ | ||||||
# Copyright 2013-present MongoDB, Inc. | ||||||
# | ||||||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||||||
# you may not use this file except in compliance with the License. | ||||||
# You may obtain a copy of the License at | ||||||
# | ||||||
# http://www.apache.org/licenses/LICENSE-2.0 | ||||||
# | ||||||
# Unless required by applicable law or agreed to in writing, software | ||||||
# distributed under the License is distributed on an "AS IS" BASIS, | ||||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||
# See the License for the specific language governing permissions and | ||||||
# limitations under the License. | ||||||
from __future__ import annotations | ||||||
|
||||||
import asyncio | ||||||
import os | ||||||
import pathlib | ||||||
import time | ||||||
import unittest | ||||||
from test.asynchronous import AsyncIntegrationTest | ||||||
from test.asynchronous.unified_format import generate_test_classes | ||||||
from test.utils_shared import CMAPListener | ||||||
from typing import Any, Optional | ||||||
|
||||||
import pytest | ||||||
|
||||||
from pymongo import AsyncMongoClient, MongoClient | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
from pymongo.driver_info import DriverInfo | ||||||
from pymongo.monitoring import ConnectionClosedEvent | ||||||
|
||||||
try: | ||||||
from mockupdb import MockupDB, OpMsgReply | ||||||
|
||||||
_HAVE_MOCKUPDB = True | ||||||
except ImportError: | ||||||
_HAVE_MOCKUPDB = False | ||||||
|
||||||
pytestmark = pytest.mark.mockupdb | ||||||
|
||||||
_IS_SYNC = False | ||||||
|
||||||
# Location of JSON test specifications. | ||||||
if _IS_SYNC: | ||||||
_TEST_PATH = os.path.join(pathlib.Path(__file__).resolve().parent, "handshake", "unified") | ||||||
else: | ||||||
_TEST_PATH = os.path.join( | ||||||
pathlib.Path(__file__).resolve().parent.parent, "handshake", "unified" | ||||||
) | ||||||
|
||||||
# Generate unified tests. | ||||||
globals().update(generate_test_classes(_TEST_PATH, module=__name__)) | ||||||
|
||||||
|
||||||
def _get_handshake_driver_info(request): | ||||||
assert "client" in request | ||||||
return request["client"] | ||||||
|
||||||
|
||||||
class TestClientMetadataProse(AsyncIntegrationTest): | ||||||
async def asyncSetUp(self): | ||||||
await super().asyncSetUp() | ||||||
self.server = MockupDB() | ||||||
self.handshake_req = None | ||||||
|
||||||
def respond(r): | ||||||
if "ismaster" in r: | ||||||
# then this is a handshake request | ||||||
self.handshake_req = r | ||||||
return r.reply(OpMsgReply(minWireVersion=0, maxWireVersion=13)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think its needed so that MockupDB would work(?) without specification, the |
||||||
|
||||||
self.server.autoresponds(respond) | ||||||
self.server.run() | ||||||
self.addAsyncCleanup(self.server.stop) | ||||||
|
||||||
async def send_ping_and_get_metadata( | ||||||
self, client: AsyncMongoClient, is_handshake: bool | ||||||
) -> tuple[str, Optional[str], Optional[str], dict[str, Any]]: | ||||||
# reset if handshake request | ||||||
if is_handshake: | ||||||
self.handshake_req: Optional[dict] = None | ||||||
|
||||||
await client.admin.command("ping") | ||||||
metadata = _get_handshake_driver_info(self.handshake_req) | ||||||
driver_metadata = metadata["driver"] | ||||||
name, version, platform = ( | ||||||
driver_metadata["name"], | ||||||
driver_metadata["version"], | ||||||
metadata["platform"], | ||||||
) | ||||||
return name, version, platform, metadata | ||||||
|
||||||
async def check_metadata_added( | ||||||
self, | ||||||
client: AsyncMongoClient, | ||||||
add_name: str, | ||||||
add_version: Optional[str], | ||||||
add_platform: Optional[str], | ||||||
) -> None: | ||||||
# send initial metadata | ||||||
name, version, platform, metadata = await self.send_ping_and_get_metadata(client, True) | ||||||
# wait for connection to become idle | ||||||
await asyncio.sleep(0.005) | ||||||
blink1073 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we refactor this into an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec specifically stated to wait 5ms |
||||||
|
||||||
# add new metadata | ||||||
client.append_metadata(DriverInfo(add_name, add_version, add_platform)) | ||||||
new_name, new_version, new_platform, new_metadata = await self.send_ping_and_get_metadata( | ||||||
client, True | ||||||
) | ||||||
self.assertEqual(new_name, f"{name}|{add_name}" if add_name is not None else name) | ||||||
self.assertEqual( | ||||||
new_version, | ||||||
f"{version}|{add_version}" if add_version is not None else version, | ||||||
) | ||||||
self.assertEqual( | ||||||
new_platform, | ||||||
f"{platform}|{add_platform}" if add_platform is not None else platform, | ||||||
) | ||||||
|
||||||
metadata.pop("driver") | ||||||
metadata.pop("platform") | ||||||
new_metadata.pop("driver") | ||||||
new_metadata.pop("platform") | ||||||
self.assertEqual(metadata, new_metadata) | ||||||
|
||||||
async def test_append_metadata(self): | ||||||
client = AsyncMongoClient( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of these clients should be created with one of our |
||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
driver=DriverInfo("library", "1.2", "Library Platform"), | ||||||
) | ||||||
await self.check_metadata_added(client, "framework", "2.0", "Framework Platform") | ||||||
await client.close() | ||||||
|
||||||
async def test_append_metadata_platform_none(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
driver=DriverInfo("library", "1.2", "Library Platform"), | ||||||
) | ||||||
await self.check_metadata_added(client, "framework", "2.0", None) | ||||||
await client.close() | ||||||
|
||||||
async def test_append_metadata_version_none(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
driver=DriverInfo("library", "1.2", "Library Platform"), | ||||||
) | ||||||
await self.check_metadata_added(client, "framework", None, "Framework Platform") | ||||||
await client.close() | ||||||
|
||||||
async def test_append_metadata_platform_version_none(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
driver=DriverInfo("library", "1.2", "Library Platform"), | ||||||
) | ||||||
await self.check_metadata_added(client, "framework", None, None) | ||||||
await client.close() | ||||||
|
||||||
async def test_multiple_successive_metadata_updates(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, maxIdleTimeMS=1, connect=False | ||||||
) | ||||||
client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) | ||||||
await self.check_metadata_added(client, "framework", "2.0", "Framework Platform") | ||||||
await client.close() | ||||||
|
||||||
async def test_multiple_successive_metadata_updates_platform_none(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
) | ||||||
client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) | ||||||
await self.check_metadata_added(client, "framework", "2.0", None) | ||||||
await client.close() | ||||||
|
||||||
async def test_multiple_successive_metadata_updates_version_none(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
) | ||||||
client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) | ||||||
await self.check_metadata_added(client, "framework", None, "Framework Platform") | ||||||
await client.close() | ||||||
|
||||||
async def test_multiple_successive_metadata_updates_platform_version_none(self): | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
) | ||||||
client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) | ||||||
await self.check_metadata_added(client, "framework", None, None) | ||||||
await client.close() | ||||||
|
||||||
async def test_doesnt_update_established_connections(self): | ||||||
listener = CMAPListener() | ||||||
client = AsyncMongoClient( | ||||||
"mongodb://" + self.server.address_string, | ||||||
maxIdleTimeMS=1, | ||||||
driver=DriverInfo("library", "1.2", "Library Platform"), | ||||||
event_listeners=[listener], | ||||||
) | ||||||
|
||||||
# send initial metadata | ||||||
name, version, platform, metadata = await self.send_ping_and_get_metadata(client, True) | ||||||
self.assertIsNotNone(name) | ||||||
self.assertIsNotNone(version) | ||||||
self.assertIsNotNone(platform) | ||||||
|
||||||
# add data | ||||||
add_name, add_version, add_platform = "framework", "2.0", "Framework Platform" | ||||||
client.append_metadata(DriverInfo(add_name, add_version, add_platform)) | ||||||
# check new data isn't sent | ||||||
self.handshake_req: Optional[dict] = None | ||||||
await client.admin.command("ping") | ||||||
self.assertIsNone(self.handshake_req) | ||||||
self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) | ||||||
|
||||||
await client.close() | ||||||
|
||||||
|
||||||
if __name__ == "__main__": | ||||||
unittest.main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,7 @@ | |
from pymongo.asynchronous.database import AsyncDatabase | ||
from pymongo.asynchronous.encryption import AsyncClientEncryption | ||
from pymongo.asynchronous.helpers import anext | ||
from pymongo.driver_info import DriverInfo | ||
from pymongo.encryption_options import _HAVE_PYMONGOCRYPT | ||
from pymongo.errors import ( | ||
AutoReconnect, | ||
|
@@ -840,6 +841,11 @@ async def _cursor_close(self, target, *args, **kwargs): | |
self.__raise_if_unsupported("close", target, NonLazyCursor, AsyncCommandCursor) | ||
return await target.close() | ||
|
||
async def _clientOperation_appendMetadata(self, target, *args, **kwargs): | ||
info_opts = kwargs["driver_info_options"] | ||
driver_info = DriverInfo(info_opts["name"], info_opts["version"], info_opts["platform"]) | ||
target.append_metadata(driver_info) | ||
|
||
async def _clientEncryptionOperation_createDataKey(self, target, *args, **kwargs): | ||
if "opts" in kwargs: | ||
kwargs.update(camel_to_snake_args(kwargs.pop("opts"))) | ||
|
@@ -925,7 +931,6 @@ async def run_entity_operation(self, spec): | |
) | ||
else: | ||
arguments = {} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whitespace removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oops accident! |
||
if isinstance(target, AsyncMongoClient): | ||
method_name = f"_clientOperation_{opname}" | ||
elif isinstance(target, AsyncDatabase): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update this docstring to follow our standard format (params, versionadded, etc...)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made my best attempt at it, but lmk if I was off / am still missing something / have it incorrectly formatted >.<