From b1899d6f2ec92c99203be71fd4ae83957ea0299d Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Fri, 30 May 2025 10:56:51 -0700 Subject: [PATCH 1/6] WIP - sync test pass but not async.. --- pymongo/asynchronous/mongo_client.py | 15 ++ pymongo/pool_options.py | 3 + pymongo/synchronous/mongo_client.py | 15 ++ test/asynchronous/test_client_metadata.py | 161 ++++++++++++++++++++++ test/test_client_metadata.py | 161 ++++++++++++++++++++++ tools/synchro.py | 1 + 6 files changed, 356 insertions(+) create mode 100644 test/asynchronous/test_client_metadata.py create mode 100644 test/test_client_metadata.py diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 72755263c9..632acfe707 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -70,6 +70,7 @@ from pymongo.asynchronous.settings import TopologySettings from pymongo.asynchronous.topology import Topology, _ErrorContext from pymongo.client_options import ClientOptions +from pymongo.driver_info import DriverInfo from pymongo.errors import ( AutoReconnect, BulkWriteError, @@ -1040,6 +1041,20 @@ async def target() -> bool: self._kill_cursors_executor = executor self._opened = False + def _append_metadata(self, driver_info: DriverInfo) -> None: + metadata = self._options.pool_options.metadata + for k, v in driver_info._asdict().items(): + if v is None: + continue + if k in metadata: + metadata[k] = f"{metadata[k]}|{v}" + elif k in metadata["driver"]: + metadata["driver"][k] = "{}|{}".format( + metadata["driver"][k], + v, + ) + self._options.pool_options._set_metadata(metadata) + def _should_pin_cursor(self, session: Optional[AsyncClientSession]) -> Optional[bool]: return self._options.load_balanced and not (session and session.in_transaction) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index a2e309cc56..33cd97978f 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -522,3 +522,6 @@ def server_api(self) -> Optional[ServerApi]: def load_balanced(self) -> Optional[bool]: """True if this Pool is configured in load balanced mode.""" return self.__load_balanced + + def _set_metadata(self, new_data: dict[str, Any]) -> None: + self.__metadata = new_data diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 99a517e5c1..af458b7c2a 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -62,6 +62,7 @@ from bson.timestamp import Timestamp from pymongo import _csot, common, helpers_shared, periodic_executor from pymongo.client_options import ClientOptions +from pymongo.driver_info import DriverInfo from pymongo.errors import ( AutoReconnect, BulkWriteError, @@ -1040,6 +1041,20 @@ def target() -> bool: self._kill_cursors_executor = executor self._opened = False + def _append_metadata(self, driver_info: DriverInfo) -> None: + metadata = self._options.pool_options.metadata + for k, v in driver_info._asdict().items(): + if v is None: + continue + if k in metadata: + metadata[k] = f"{metadata[k]}|{v}" + elif k in metadata["driver"]: + metadata["driver"][k] = "{}|{}".format( + metadata["driver"][k], + v, + ) + self._options.pool_options._set_metadata(metadata) + def _should_pin_cursor(self, session: Optional[ClientSession]) -> Optional[bool]: return self._options.load_balanced and not (session and session.in_transaction) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py new file mode 100644 index 0000000000..cedfbad1c8 --- /dev/null +++ b/test/asynchronous/test_client_metadata.py @@ -0,0 +1,161 @@ +# 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 time +import unittest +from test.utils_shared import CMAPListener +from typing import Optional + +import pytest +from mockupdb import MockupDB, OpMsgReply + +from pymongo import AsyncMongoClient, MongoClient +from pymongo.driver_info import DriverInfo +from pymongo.monitoring import ConnectionClosedEvent + +pytestmark = pytest.mark.mockupdb + +_IS_SYNC = False + + +def _get_handshake_driver_info(request): + assert "client" in request + return request["client"] + + +class TestClientMetadataProse(unittest.TestCase): + def setUp(self): + self.server = MockupDB() + self.handshake_req = None + + def respond(r): + # Only save the very first request from the driver. + if self.handshake_req is None: + self.handshake_req = r + return r.reply(OpMsgReply(minWireVersion=0, maxWireVersion=13)) + + self.server.autoresponds(respond) + self.server.run() + self.addCleanup(self.server.stop) + + async def check_metadata_added( + self, add_name: Optional[str], add_version: Optional[str], add_platform: Optional[str] + ) -> None: + client = AsyncMongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + + # send initial metadata + 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"], + ) + await asyncio.sleep(0.005) + + # add data + client._append_metadata(DriverInfo(add_name, add_version, add_platform)) + # reset + self.handshake_req = None + await client.admin.command("ping") + new_metadata = _get_handshake_driver_info(self.handshake_req) + # compare + self.assertEqual( + new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name + ) + self.assertEqual( + new_metadata["driver"]["version"], + f"{version}|{add_version}" if add_version is not None else version, + ) + self.assertEqual( + new_metadata["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) + + await client.close() + + async def test_append_metadata(self): + await self.check_metadata_added("framework", "2.0", "Framework Platform") + + async def test_append_metadata_platform_none(self): + await self.check_metadata_added("framework", "2.0", None) + + async def test_append_metadata_version_none(self): + await self.check_metadata_added("framework", None, "Framework Platform") + + async def test_append_platform_version_none(self): + await self.check_metadata_added("framework", None, None) + + 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 + 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"], + ) + # feels like i should do something to check that it is initially sent + 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 = None + await client.admin.command("ping") + # if it was an actual handshake request, client data would be in the ping request which would start the handshake i think + self.assertNotIn("client", self.handshake_req) + self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) + + await client.close() + + +# THESE ARE MY NOTES TO SELF, PLAESE IGNORE +# two options +# emit events with a flag, so when testing (like now), we can emit more stuff +# or we can mock it? but not sure if that ruins the spirit of the test -> this would be easier tho.. +# we'd send, mock server would receive and send back to us? +# use mockup DB! +# usually, create mockupDB instance and then tell it to automatically respond to automatic responses, i'll need to change that for this but i also don't want to mess up SDAM stuff? +# i can get logs from server (in mongo orchestration) if mockup DB is too annoying (maybe get log?) +# ask the team generally but jib thinks we should emit events with a flag +# make sure to state pros and cons for each ticket + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py new file mode 100644 index 0000000000..48f8d58d9c --- /dev/null +++ b/test/test_client_metadata.py @@ -0,0 +1,161 @@ +# 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 time +import unittest +from test.utils_shared import CMAPListener +from typing import Optional + +import pytest +from mockupdb import MockupDB, OpMsgReply + +from pymongo import MongoClient +from pymongo.driver_info import DriverInfo +from pymongo.monitoring import ConnectionClosedEvent + +pytestmark = pytest.mark.mockupdb + +_IS_SYNC = True + + +def _get_handshake_driver_info(request): + assert "client" in request + return request["client"] + + +class TestClientMetadataProse(unittest.TestCase): + def setUp(self): + self.server = MockupDB() + self.handshake_req = None + + def respond(r): + # Only save the very first request from the driver. + if self.handshake_req is None: + self.handshake_req = r + return r.reply(OpMsgReply(minWireVersion=0, maxWireVersion=13)) + + self.server.autoresponds(respond) + self.server.run() + self.addCleanup(self.server.stop) + + def check_metadata_added( + self, add_name: Optional[str], add_version: Optional[str], add_platform: Optional[str] + ) -> None: + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + + # send initial metadata + 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"], + ) + time.sleep(0.005) + + # add data + client._append_metadata(DriverInfo(add_name, add_version, add_platform)) + # reset + self.handshake_req = None + client.admin.command("ping") + new_metadata = _get_handshake_driver_info(self.handshake_req) + # compare + self.assertEqual( + new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name + ) + self.assertEqual( + new_metadata["driver"]["version"], + f"{version}|{add_version}" if add_version is not None else version, + ) + self.assertEqual( + new_metadata["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) + + client.close() + + def test_append_metadata(self): + self.check_metadata_added("framework", "2.0", "Framework Platform") + + def test_append_metadata_platform_none(self): + self.check_metadata_added("framework", "2.0", None) + + def test_append_metadata_version_none(self): + self.check_metadata_added("framework", None, "Framework Platform") + + def test_append_platform_version_none(self): + self.check_metadata_added("framework", None, None) + + def test_doesnt_update_established_connections(self): + listener = CMAPListener() + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + event_listeners=[listener], + ) + + # send initial metadata + 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"], + ) + # feels like i should do something to check that it is initially sent + 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 = None + client.admin.command("ping") + # if it was an actual handshake request, client data would be in the ping request which would start the handshake i think + self.assertNotIn("client", self.handshake_req) + self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) + + client.close() + + +# THESE ARE MY NOTES TO SELF, PLAESE IGNORE +# two options +# emit events with a flag, so when testing (like now), we can emit more stuff +# or we can mock it? but not sure if that ruins the spirit of the test -> this would be easier tho.. +# we'd send, mock server would receive and send back to us? +# use mockup DB! +# usually, create mockupDB instance and then tell it to automatically respond to automatic responses, i'll need to change that for this but i also don't want to mess up SDAM stuff? +# i can get logs from server (in mongo orchestration) if mockup DB is too annoying (maybe get log?) +# ask the team generally but jib thinks we should emit events with a flag +# make sure to state pros and cons for each ticket + +if __name__ == "__main__": + unittest.main() diff --git a/tools/synchro.py b/tools/synchro.py index bfe8f71125..327940f8ff 100644 --- a/tools/synchro.py +++ b/tools/synchro.py @@ -209,6 +209,7 @@ def async_only_test(f: str) -> bool: "test_client.py", "test_client_bulk_write.py", "test_client_context.py", + "test_client_metadata.py", "test_collation.py", "test_collection.py", "test_collection_management.py", From e18737fda66cb44565bdded6aa2c8f452a4b7460 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Mon, 2 Jun 2025 14:58:28 -0700 Subject: [PATCH 2/6] move test to correct folder --- test/asynchronous/test_client_metadata.py | 161 -------------------- test/{ => mockupdb}/test_client_metadata.py | 35 ++--- tools/synchro.py | 1 - 3 files changed, 15 insertions(+), 182 deletions(-) delete mode 100644 test/asynchronous/test_client_metadata.py rename test/{ => mockupdb}/test_client_metadata.py (81%) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py deleted file mode 100644 index cedfbad1c8..0000000000 --- a/test/asynchronous/test_client_metadata.py +++ /dev/null @@ -1,161 +0,0 @@ -# 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 time -import unittest -from test.utils_shared import CMAPListener -from typing import Optional - -import pytest -from mockupdb import MockupDB, OpMsgReply - -from pymongo import AsyncMongoClient, MongoClient -from pymongo.driver_info import DriverInfo -from pymongo.monitoring import ConnectionClosedEvent - -pytestmark = pytest.mark.mockupdb - -_IS_SYNC = False - - -def _get_handshake_driver_info(request): - assert "client" in request - return request["client"] - - -class TestClientMetadataProse(unittest.TestCase): - def setUp(self): - self.server = MockupDB() - self.handshake_req = None - - def respond(r): - # Only save the very first request from the driver. - if self.handshake_req is None: - self.handshake_req = r - return r.reply(OpMsgReply(minWireVersion=0, maxWireVersion=13)) - - self.server.autoresponds(respond) - self.server.run() - self.addCleanup(self.server.stop) - - async def check_metadata_added( - self, add_name: Optional[str], add_version: Optional[str], add_platform: Optional[str] - ) -> None: - client = AsyncMongoClient( - "mongodb://" + self.server.address_string, - maxIdleTimeMS=1, - driver=DriverInfo("library", "1.2", "Library Platform"), - ) - - # send initial metadata - 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"], - ) - await asyncio.sleep(0.005) - - # add data - client._append_metadata(DriverInfo(add_name, add_version, add_platform)) - # reset - self.handshake_req = None - await client.admin.command("ping") - new_metadata = _get_handshake_driver_info(self.handshake_req) - # compare - self.assertEqual( - new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name - ) - self.assertEqual( - new_metadata["driver"]["version"], - f"{version}|{add_version}" if add_version is not None else version, - ) - self.assertEqual( - new_metadata["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) - - await client.close() - - async def test_append_metadata(self): - await self.check_metadata_added("framework", "2.0", "Framework Platform") - - async def test_append_metadata_platform_none(self): - await self.check_metadata_added("framework", "2.0", None) - - async def test_append_metadata_version_none(self): - await self.check_metadata_added("framework", None, "Framework Platform") - - async def test_append_platform_version_none(self): - await self.check_metadata_added("framework", None, None) - - 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 - 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"], - ) - # feels like i should do something to check that it is initially sent - 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 = None - await client.admin.command("ping") - # if it was an actual handshake request, client data would be in the ping request which would start the handshake i think - self.assertNotIn("client", self.handshake_req) - self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) - - await client.close() - - -# THESE ARE MY NOTES TO SELF, PLAESE IGNORE -# two options -# emit events with a flag, so when testing (like now), we can emit more stuff -# or we can mock it? but not sure if that ruins the spirit of the test -> this would be easier tho.. -# we'd send, mock server would receive and send back to us? -# use mockup DB! -# usually, create mockupDB instance and then tell it to automatically respond to automatic responses, i'll need to change that for this but i also don't want to mess up SDAM stuff? -# i can get logs from server (in mongo orchestration) if mockup DB is too annoying (maybe get log?) -# ask the team generally but jib thinks we should emit events with a flag -# make sure to state pros and cons for each ticket - -if __name__ == "__main__": - unittest.main() diff --git a/test/test_client_metadata.py b/test/mockupdb/test_client_metadata.py similarity index 81% rename from test/test_client_metadata.py rename to test/mockupdb/test_client_metadata.py index 48f8d58d9c..54077d6e23 100644 --- a/test/test_client_metadata.py +++ b/test/mockupdb/test_client_metadata.py @@ -13,22 +13,25 @@ # limitations under the License. from __future__ import annotations -import asyncio import time import unittest from test.utils_shared import CMAPListener from typing import Optional import pytest -from mockupdb import MockupDB, OpMsgReply from pymongo import MongoClient from pymongo.driver_info import DriverInfo from pymongo.monitoring import ConnectionClosedEvent -pytestmark = pytest.mark.mockupdb +try: + from mockupdb import MockupDB, OpMsgReply + + _HAVE_MOCKUPDB = True +except ImportError: + _HAVE_MOCKUPDB = False -_IS_SYNC = True +pytestmark = pytest.mark.mockupdb def _get_handshake_driver_info(request): @@ -39,12 +42,17 @@ def _get_handshake_driver_info(request): class TestClientMetadataProse(unittest.TestCase): def setUp(self): self.server = MockupDB() + # there are two handshake requests, i believe one is from the monitor, and the other is from the client + self.monitor_handshake = False self.handshake_req = None def respond(r): # Only save the very first request from the driver. if self.handshake_req is None: - self.handshake_req = r + if not self.monitor_handshake: + self.monitor_handshake = True + else: + self.handshake_req = r return r.reply(OpMsgReply(minWireVersion=0, maxWireVersion=13)) self.server.autoresponds(respond) @@ -73,11 +81,11 @@ def check_metadata_added( # add data client._append_metadata(DriverInfo(add_name, add_version, add_platform)) - # reset + # make sure new metadata is being sent self.handshake_req = None client.admin.command("ping") new_metadata = _get_handshake_driver_info(self.handshake_req) - # compare + self.assertEqual( new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name ) @@ -128,7 +136,6 @@ def test_doesnt_update_established_connections(self): driver_metadata["version"], metadata["platform"], ) - # feels like i should do something to check that it is initially sent self.assertIsNotNone(name) self.assertIsNotNone(version) self.assertIsNotNone(platform) @@ -139,23 +146,11 @@ def test_doesnt_update_established_connections(self): # check new data isn't sent self.handshake_req = None client.admin.command("ping") - # if it was an actual handshake request, client data would be in the ping request which would start the handshake i think self.assertNotIn("client", self.handshake_req) self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) client.close() -# THESE ARE MY NOTES TO SELF, PLAESE IGNORE -# two options -# emit events with a flag, so when testing (like now), we can emit more stuff -# or we can mock it? but not sure if that ruins the spirit of the test -> this would be easier tho.. -# we'd send, mock server would receive and send back to us? -# use mockup DB! -# usually, create mockupDB instance and then tell it to automatically respond to automatic responses, i'll need to change that for this but i also don't want to mess up SDAM stuff? -# i can get logs from server (in mongo orchestration) if mockup DB is too annoying (maybe get log?) -# ask the team generally but jib thinks we should emit events with a flag -# make sure to state pros and cons for each ticket - if __name__ == "__main__": unittest.main() diff --git a/tools/synchro.py b/tools/synchro.py index 327940f8ff..bfe8f71125 100644 --- a/tools/synchro.py +++ b/tools/synchro.py @@ -209,7 +209,6 @@ def async_only_test(f: str) -> bool: "test_client.py", "test_client_bulk_write.py", "test_client_context.py", - "test_client_metadata.py", "test_collation.py", "test_collection.py", "test_collection_management.py", From f9a097a795b54dedbb364dad94deffdca75a1ee8 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Mon, 2 Jun 2025 15:39:49 -0700 Subject: [PATCH 3/6] fix typing --- test/mockupdb/test_client_metadata.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/mockupdb/test_client_metadata.py b/test/mockupdb/test_client_metadata.py index 54077d6e23..bba1119877 100644 --- a/test/mockupdb/test_client_metadata.py +++ b/test/mockupdb/test_client_metadata.py @@ -44,7 +44,7 @@ def setUp(self): self.server = MockupDB() # there are two handshake requests, i believe one is from the monitor, and the other is from the client self.monitor_handshake = False - self.handshake_req = None + self.handshake_req: Optional[dict] = None def respond(r): # Only save the very first request from the driver. @@ -60,7 +60,7 @@ def respond(r): self.addCleanup(self.server.stop) def check_metadata_added( - self, add_name: Optional[str], add_version: Optional[str], add_platform: Optional[str] + self, add_name: str, add_version: Optional[str], add_platform: Optional[str] ) -> None: client = MongoClient( "mongodb://" + self.server.address_string, @@ -84,6 +84,7 @@ def check_metadata_added( # make sure new metadata is being sent self.handshake_req = None client.admin.command("ping") + # self.assertIsNotNone(self.handshake_req) new_metadata = _get_handshake_driver_info(self.handshake_req) self.assertEqual( @@ -144,8 +145,10 @@ def test_doesnt_update_established_connections(self): 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 = None + self.handshake_req: Optional[dict] = None client.admin.command("ping") + self.assertIsNotNone(self.handshake_req) + assert self.handshake_req is not None # so mypy knows that it's not None self.assertNotIn("client", self.handshake_req) self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) From 21c4eefd8927dcc4eb0c914afc91c9e98db36580 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Wed, 4 Jun 2025 15:02:57 -0700 Subject: [PATCH 4/6] add another test --- test/mockupdb/test_client_metadata.py | 76 ++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/test/mockupdb/test_client_metadata.py b/test/mockupdb/test_client_metadata.py index bba1119877..e37bc2bbd3 100644 --- a/test/mockupdb/test_client_metadata.py +++ b/test/mockupdb/test_client_metadata.py @@ -84,7 +84,7 @@ def check_metadata_added( # make sure new metadata is being sent self.handshake_req = None client.admin.command("ping") - # self.assertIsNotNone(self.handshake_req) + assert self.handshake_req is not None new_metadata = _get_handshake_driver_info(self.handshake_req) self.assertEqual( @@ -154,6 +154,80 @@ def test_doesnt_update_established_connections(self): client.close() + def test_append_metadata_multiple_times(self): + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + + # send initial metadata + 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"], + ) + time.sleep(0.005) + + # add data + add_name, add_version, add_platform = "framework", "2.0", "Framework Platform" + client._append_metadata(DriverInfo(add_name, add_version, add_platform)) + # make sure new metadata is being sent + self.handshake_req = None + client.admin.command("ping") + assert self.handshake_req is not None + new_metadata = _get_handshake_driver_info(self.handshake_req) + + self.assertEqual( + new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name + ) + self.assertEqual( + new_metadata["driver"]["version"], + f"{version}|{add_version}" if add_version is not None else version, + ) + self.assertEqual( + new_metadata["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) + time.sleep(0.005) + + # add data again + add_name2, add_version2, add_platform2 = "framework2", "3.0", "Framework Platform2" + client._append_metadata(DriverInfo(add_name2, add_version2, add_platform2)) + # make sure new metadata is being sent + self.handshake_req = None + client.admin.command("ping") + assert self.handshake_req is not None + new_metadata2 = _get_handshake_driver_info(self.handshake_req) + + self.assertEqual( + new_metadata2["driver"]["name"], + f"{name}|{add_name}|{add_name2}" if add_name2 is not None else name, + ) + self.assertEqual( + new_metadata2["driver"]["version"], + f"{version}|{add_version}|{add_version2}" if add_version2 is not None else version, + ) + self.assertEqual( + new_metadata2["platform"], + f"{platform}|{add_platform}|{add_platform2}" if add_platform2 is not None else platform, + ) + + new_metadata2.pop("driver") + new_metadata2.pop("platform") + self.assertEqual(metadata, new_metadata2) + + client.close() + if __name__ == "__main__": unittest.main() From 9f9c9d82c7593d40222dd12d90539366edc21491 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Wed, 4 Jun 2025 16:59:04 -0700 Subject: [PATCH 5/6] update/add the new prose tests --- test/mockupdb/test_client_metadata.py | 217 ++++++++++++-------------- 1 file changed, 97 insertions(+), 120 deletions(-) diff --git a/test/mockupdb/test_client_metadata.py b/test/mockupdb/test_client_metadata.py index e37bc2bbd3..01ccbdad82 100644 --- a/test/mockupdb/test_client_metadata.py +++ b/test/mockupdb/test_client_metadata.py @@ -16,7 +16,7 @@ import time import unittest from test.utils_shared import CMAPListener -from typing import Optional +from typing import Any, Optional import pytest @@ -43,32 +43,24 @@ class TestClientMetadataProse(unittest.TestCase): def setUp(self): self.server = MockupDB() # there are two handshake requests, i believe one is from the monitor, and the other is from the client - self.monitor_handshake = False self.handshake_req: Optional[dict] = None def respond(r): - # Only save the very first request from the driver. - if self.handshake_req is None: - if not self.monitor_handshake: - self.monitor_handshake = True - else: - self.handshake_req = r + if "ismaster" in r: + # then this is a handshake request + self.handshake_req = r return r.reply(OpMsgReply(minWireVersion=0, maxWireVersion=13)) self.server.autoresponds(respond) self.server.run() self.addCleanup(self.server.stop) - def check_metadata_added( - self, add_name: str, add_version: Optional[str], add_platform: Optional[str] - ) -> None: - client = MongoClient( - "mongodb://" + self.server.address_string, - maxIdleTimeMS=1, - driver=DriverInfo("library", "1.2", "Library Platform"), - ) - - # send initial metadata + def send_ping_and_get_metadata( + self, client: MongoClient, is_handshake: bool + ) -> tuple[str, Optional[str], Optional[str], dict[str, Any]]: + # reset + if is_handshake: + self.handshake_req: Optional[dict] = None client.admin.command("ping") metadata = _get_handshake_driver_info(self.handshake_req) driver_metadata = metadata["driver"] @@ -77,25 +69,36 @@ def check_metadata_added( driver_metadata["version"], metadata["platform"], ) + return name, version, platform, metadata + + def check_metadata_added( + self, + client: MongoClient, + add_name: str, + add_version: Optional[str], + add_platform: Optional[str], + ) -> None: + # send initial metadata + name, version, platform, metadata = self.send_ping_and_get_metadata(client, True) time.sleep(0.005) - # add data + # add new metadata client._append_metadata(DriverInfo(add_name, add_version, add_platform)) - # make sure new metadata is being sent - self.handshake_req = None - client.admin.command("ping") - assert self.handshake_req is not None - new_metadata = _get_handshake_driver_info(self.handshake_req) - - self.assertEqual( - new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name + new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( + client, True ) + print("IN SEND PING AND GET METADATA") + print(name, version, platform) + print(metadata) + print(new_name, new_version, new_platform) + print(new_metadata) + self.assertEqual(new_name, f"{name}|{add_name}" if add_name is not None else name) self.assertEqual( - new_metadata["driver"]["version"], + new_version, f"{version}|{add_version}" if add_version is not None else version, ) self.assertEqual( - new_metadata["platform"], + new_platform, f"{platform}|{add_platform}" if add_platform is not None else platform, ) @@ -105,126 +108,100 @@ def check_metadata_added( new_metadata.pop("platform") self.assertEqual(metadata, new_metadata) - client.close() - def test_append_metadata(self): - self.check_metadata_added("framework", "2.0", "Framework Platform") + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + self.check_metadata_added(client, "framework", "2.0", "Framework Platform") + client.close() def test_append_metadata_platform_none(self): - self.check_metadata_added("framework", "2.0", None) + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + self.check_metadata_added(client, "framework", "2.0", None) + client.close() def test_append_metadata_version_none(self): - self.check_metadata_added("framework", None, "Framework Platform") - - def test_append_platform_version_none(self): - self.check_metadata_added("framework", None, None) + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + self.check_metadata_added(client, "framework", None, "Framework Platform") + client.close() - def test_doesnt_update_established_connections(self): - listener = CMAPListener() + def test_append_metadata_platform_version_none(self): client = MongoClient( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, driver=DriverInfo("library", "1.2", "Library Platform"), - event_listeners=[listener], ) + self.check_metadata_added(client, "framework", None, None) + client.close() - # send initial metadata - 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"], + def test_multiple_successive_metadata_updates(self): + client = MongoClient( + "mongodb://" + self.server.address_string, maxIdleTimeMS=1, connect=False ) - self.assertIsNotNone(name) - self.assertIsNotNone(version) - self.assertIsNotNone(platform) + client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + self.check_metadata_added(client, "framework", "2.0", "Framework Platform") + client.close() - # 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 - client.admin.command("ping") - self.assertIsNotNone(self.handshake_req) - assert self.handshake_req is not None # so mypy knows that it's not None - self.assertNotIn("client", self.handshake_req) - self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) + def test_multiple_successive_metadata_updates_platform_none(self): + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + self.check_metadata_added(client, "framework", "2.0", None) + client.close() + def test_multiple_successive_metadata_updates_version_none(self): + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + self.check_metadata_added(client, "framework", None, "Framework Platform") + client.close() + + def test_multiple_successive_metadata_updates_platform_version_none(self): + client = MongoClient( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + self.check_metadata_added(client, "framework", None, None) client.close() - def test_append_metadata_multiple_times(self): + def test_doesnt_update_established_connections(self): + listener = CMAPListener() client = MongoClient( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, driver=DriverInfo("library", "1.2", "Library Platform"), + event_listeners=[listener], ) # send initial metadata - 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"], - ) - time.sleep(0.005) + name, version, platform, metadata = 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)) - # make sure new metadata is being sent - self.handshake_req = None - client.admin.command("ping") - assert self.handshake_req is not None - new_metadata = _get_handshake_driver_info(self.handshake_req) - - self.assertEqual( - new_metadata["driver"]["name"], f"{name}|{add_name}" if add_name is not None else name - ) - self.assertEqual( - new_metadata["driver"]["version"], - f"{version}|{add_version}" if add_version is not None else version, - ) - self.assertEqual( - new_metadata["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) - time.sleep(0.005) - - # add data again - add_name2, add_version2, add_platform2 = "framework2", "3.0", "Framework Platform2" - client._append_metadata(DriverInfo(add_name2, add_version2, add_platform2)) - # make sure new metadata is being sent - self.handshake_req = None + # check new data isn't sent + self.handshake_req: Optional[dict] = None client.admin.command("ping") - assert self.handshake_req is not None - new_metadata2 = _get_handshake_driver_info(self.handshake_req) - - self.assertEqual( - new_metadata2["driver"]["name"], - f"{name}|{add_name}|{add_name2}" if add_name2 is not None else name, - ) - self.assertEqual( - new_metadata2["driver"]["version"], - f"{version}|{add_version}|{add_version2}" if add_version2 is not None else version, - ) - self.assertEqual( - new_metadata2["platform"], - f"{platform}|{add_platform}|{add_platform2}" if add_platform2 is not None else platform, - ) - - new_metadata2.pop("driver") - new_metadata2.pop("platform") - self.assertEqual(metadata, new_metadata2) + self.assertIsNone(self.handshake_req) + self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) client.close() From 265ef3b54fb4936d57df33f2c1b3bfc2fafedd70 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Thu, 5 Jun 2025 08:42:18 -0700 Subject: [PATCH 6/6] make public --- pymongo/asynchronous/mongo_client.py | 5 ++++- pymongo/synchronous/mongo_client.py | 5 ++++- test/mockupdb/test_client_metadata.py | 12 ++++++------ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 632acfe707..6390bcdd67 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -1041,7 +1041,10 @@ async def target() -> bool: self._kill_cursors_executor = executor self._opened = False - def _append_metadata(self, driver_info: DriverInfo) -> None: + def append_metadata(self, driver_info: DriverInfo) -> None: + """ + Appends the given metadata to existing driver metadata. + """ metadata = self._options.pool_options.metadata for k, v in driver_info._asdict().items(): if v is None: diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index af458b7c2a..f36ae491d6 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -1041,7 +1041,10 @@ def target() -> bool: self._kill_cursors_executor = executor self._opened = False - def _append_metadata(self, driver_info: DriverInfo) -> None: + def append_metadata(self, driver_info: DriverInfo) -> None: + """ + Appends the given metadata to existing driver metadata. + """ metadata = self._options.pool_options.metadata for k, v in driver_info._asdict().items(): if v is None: diff --git a/test/mockupdb/test_client_metadata.py b/test/mockupdb/test_client_metadata.py index 01ccbdad82..27eb0fdeea 100644 --- a/test/mockupdb/test_client_metadata.py +++ b/test/mockupdb/test_client_metadata.py @@ -83,7 +83,7 @@ def check_metadata_added( time.sleep(0.005) # add new metadata - client._append_metadata(DriverInfo(add_name, add_version, add_platform)) + client.append_metadata(DriverInfo(add_name, add_version, add_platform)) new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( client, True ) @@ -148,7 +148,7 @@ def test_multiple_successive_metadata_updates(self): client = MongoClient( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, connect=False ) - client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", "2.0", "Framework Platform") client.close() @@ -157,7 +157,7 @@ def test_multiple_successive_metadata_updates_platform_none(self): "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", "2.0", None) client.close() @@ -166,7 +166,7 @@ def test_multiple_successive_metadata_updates_version_none(self): "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", None, "Framework Platform") client.close() @@ -175,7 +175,7 @@ def test_multiple_successive_metadata_updates_platform_version_none(self): "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - client._append_metadata(DriverInfo("library", "1.2", "Library Platform")) + client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", None, None) client.close() @@ -196,7 +196,7 @@ def test_doesnt_update_established_connections(self): # add data add_name, add_version, add_platform = "framework", "2.0", "Framework Platform" - client._append_metadata(DriverInfo(add_name, add_version, add_platform)) + client.append_metadata(DriverInfo(add_name, add_version, add_platform)) # check new data isn't sent self.handshake_req: Optional[dict] = None client.admin.command("ping")