PYTHON-5344 and PYTHON-5403 Allow Instantiated MongoClients to Send Client Metadata On-Demand#2358
Conversation
sleepyStick
changed the title
sleepyStick
changed the title
pymongo/asynchronous/mongo_client.py
Outdated
|
|
||
| def append_metadata(self, driver_info: DriverInfo) -> None: | ||
| """ | ||
| Appends the given metadata to existing driver metadata. |
There was a problem hiding this comment.
Could you update this docstring to follow our standard format (params, versionadded, etc...)?
There was a problem hiding this comment.
I made my best attempt at it, but lmk if I was off / am still missing something / have it incorrectly formatted >.<
|
|
||
| import pytest | ||
|
|
||
| from pymongo import AsyncMongoClient, MongoClient |
There was a problem hiding this comment.
| from pymongo import AsyncMongoClient, MongoClient | |
| from pymongo import AsyncMongoClient |
| 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.
Is maxWireVersion=13 specified by the spec or by MockupDB?
There was a problem hiding this comment.
I think its needed so that MockupDB would work(?) without specification, the maxWireVersion is 6, and PyMongo raises a configuration error: pymongo.errors.ConfigurationError: Server at localhost:56617 reports wire version 0, but this version of PyMongo requires at least 7 (MongoDB 4.0).
| # 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) |
There was a problem hiding this comment.
Can we refactor this into an async_wait_until with some predicate?
There was a problem hiding this comment.
The spec specifically stated to wait 5ms
| self.assertEqual(metadata, new_metadata) | ||
|
|
||
| async def test_append_metadata(self): | ||
| client = AsyncMongoClient( |
There was a problem hiding this comment.
All of these clients should be created with one of our PyMongoTestCase helpers, not raw AsyncMongoClient().
| ) | ||
| else: | ||
| arguments = {} | ||
|
|
pymongo/pool_options.py
Outdated
| def _update_metadata(self, driver: DriverInfo): | ||
| """Updates the client's metadata""" | ||
| if driver.name: | ||
| self.__metadata["driver"]["name"] = "{}|{}".format( |
There was a problem hiding this comment.
Could we make this method atomic? As in, first create a copy of __metadata, then mutate, then update? That way the driver handshake will never serialize half of the metadata.
ShaneHarvey
left a comment
ShaneHarvey
left a comment
There was a problem hiding this comment.
API changes LGTM. Could you also add the changelog entry?
4 participants
https://jira.mongodb.org/browse/PYTHON-5344
https://jira.mongodb.org/browse/PYTHON-5403 (ticket for async tests)
Notes:
test/mockupdb) to utilize synchro for the async -> sync tests.