-
Notifications
You must be signed in to change notification settings - Fork 1.2k
PYTHON-6040 Preserve version-to-name mapping in Client Metadata #3054
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
base: main
Are you sure you want to change the base?
Changes from all commits
ac6ed46
5f2e74c
b3d18d1
b035cfb
61ba3a2
7e863c9
d45d987
0194f47
b652b78
a016133
b50cb0f
62e83d4
7ff1773
affb338
6ef9ab0
e3ead2a
917bc0a
66ce419
e8c253c
dbc559d
7ecbb94
15e62f8
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 |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| import platform | ||
| import sys | ||
| from collections.abc import MutableMapping | ||
| from contextlib import AbstractContextManager, nullcontext | ||
| from pathlib import Path | ||
| from typing import TYPE_CHECKING, Any, Optional | ||
|
|
||
|
|
@@ -37,6 +38,7 @@ | |
| WAIT_QUEUE_TIMEOUT, | ||
| has_c, | ||
| ) | ||
| from pymongo.lock import _create_lock | ||
|
|
||
| if TYPE_CHECKING: | ||
| from pymongo.auth_shared import MongoCredential | ||
|
|
@@ -200,6 +202,25 @@ def _metadata_env() -> dict[str, Any]: | |
| _MAX_METADATA_SIZE = 512 | ||
|
|
||
|
|
||
| def _truncate_utf8(content: str, overflow: int) -> str: | ||
| """Trim `overflow` UTF-8 bytes from the end of content, keeping a valid prefix.""" | ||
| if overflow <= 0: | ||
| return content | ||
| data = content.encode("utf-8") | ||
| if len(data) <= overflow: | ||
| return "" | ||
| return data[: len(data) - overflow].decode("utf-8", errors="ignore") | ||
|
|
||
|
|
||
| def _normalize_driver(driver: DriverInfo) -> DriverInfo: | ||
| """Treat None and "" as equivalent unset fields for deduplication.""" | ||
| return driver._replace( | ||
| name=driver.name or "", | ||
| version=driver.version or "", | ||
| platform=driver.platform or "", | ||
| ) | ||
|
|
||
|
|
||
| # See: https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.md#limitations | ||
| def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: | ||
| """Perform metadata truncation.""" | ||
|
|
@@ -226,34 +247,44 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: | |
| overflow = encoded_size - _MAX_METADATA_SIZE | ||
| plat = metadata.get("platform", "") | ||
| if plat: | ||
| plat = plat[:-overflow] | ||
| plat = _truncate_utf8(plat, overflow) | ||
| if plat: | ||
| metadata["platform"] = plat | ||
| else: | ||
| metadata.pop("platform", None) | ||
| encoded_size = len(bson.encode(metadata)) | ||
| if encoded_size <= _MAX_METADATA_SIZE: | ||
| return | ||
| # 5. Truncate driver info. | ||
| overflow = encoded_size - _MAX_METADATA_SIZE | ||
| # 5. Truncate driver info, keeping name and version 1:1 index-aligned. | ||
|
Contributor
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. What if we store pairs of |
||
| driver = metadata.get("driver", {}) | ||
| if driver: | ||
| # Truncate driver version. | ||
| driver_version = driver.get("version")[:-overflow] | ||
| if len(driver_version) >= len(_METADATA["driver"]["version"]): | ||
| metadata["driver"]["version"] = driver_version | ||
| else: | ||
| metadata["driver"]["version"] = _METADATA["driver"]["version"] | ||
| encoded_size = len(bson.encode(metadata)) | ||
| if encoded_size <= _MAX_METADATA_SIZE: | ||
| return | ||
| # Truncate driver name. | ||
| overflow = encoded_size - _MAX_METADATA_SIZE | ||
| driver_name = driver.get("name")[:-overflow] | ||
| if len(driver_name) >= len(_METADATA["driver"]["name"]): | ||
| metadata["driver"]["name"] = driver_name | ||
| else: | ||
| metadata["driver"]["name"] = _METADATA["driver"]["name"] | ||
| # Trim wrapper version and name content first, dropping paired segments | ||
| # only as a last resort, so name and version stay 1:1 aligned. | ||
| while True: | ||
| encoded_size = len(bson.encode(metadata)) | ||
| if encoded_size <= _MAX_METADATA_SIZE: | ||
| break | ||
| overflow = encoded_size - _MAX_METADATA_SIZE | ||
| previous = (driver.get("name", ""), driver.get("version", "")) | ||
|
Contributor
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. This can go away as well if we remove the check at the bottom of the loop. |
||
| n_parts = driver.get("name", "").split("|") | ||
| v_parts = driver.get("version", "").split("|") | ||
|
|
||
| if len(v_parts) > 1 and v_parts[-1]: | ||
| v_parts[-1] = _truncate_utf8(v_parts[-1], overflow) | ||
| driver["version"] = "|".join(v_parts) | ||
| elif len(n_parts) > 1 and n_parts[-1]: | ||
| n_parts[-1] = _truncate_utf8(n_parts[-1], overflow) | ||
| driver["name"] = "|".join(n_parts) | ||
| elif len(n_parts) > 1: | ||
| n_parts.pop() | ||
| v_parts.pop() | ||
| driver["name"] = "|".join(n_parts) | ||
| driver["version"] = "|".join(v_parts) | ||
| else: | ||
| break | ||
|
|
||
| if previous == (driver.get("name"), driver.get("version")): | ||
|
Contributor
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. Do we need this? The only way to reach this condition is if we have shortened the metadata, which will eventually cause the |
||
| break | ||
|
|
||
|
|
||
| # If the first getaddrinfo call of this interpreter's life is on a thread, | ||
|
|
@@ -277,6 +308,7 @@ class PoolOptions: | |
| """ | ||
|
|
||
| __slots__ = ( | ||
| "__appended_drivers", | ||
| "__appname", | ||
| "__compression_settings", | ||
| "__connect_timeout", | ||
|
|
@@ -288,6 +320,7 @@ class PoolOptions: | |
| "__max_idle_time_seconds", | ||
| "__max_pool_size", | ||
| "__metadata", | ||
| "__metadata_lock", | ||
| "__min_pool_size", | ||
| "__pause_enabled", | ||
| "__server_api", | ||
|
|
@@ -336,6 +369,11 @@ def __init__( | |
| self.__load_balanced = load_balanced | ||
| self.__credentials = credentials | ||
| self.__metadata = copy.deepcopy(_METADATA) | ||
| self.__appended_drivers: list[DriverInfo] = [] | ||
| # Only the synchronous client can append metadata from multiple threads. | ||
| self.__metadata_lock: AbstractContextManager[bool | None] = ( | ||
| _create_lock() if is_sync else nullcontext() | ||
| ) | ||
|
|
||
| if appname: | ||
| self.__metadata["application"] = {"name": appname} | ||
|
|
@@ -353,11 +391,19 @@ def __init__( | |
| self.__metadata["driver"]["name"], | ||
| "c", | ||
| ) | ||
| self.__metadata["driver"]["version"] = "{}|{}".format( | ||
| self.__metadata["driver"]["version"], | ||
| "", | ||
| ) | ||
| if not is_sync: | ||
| self.__metadata["driver"]["name"] = "{}|{}".format( | ||
| self.__metadata["driver"]["name"], | ||
| "async", | ||
| ) | ||
| self.__metadata["driver"]["version"] = "{}|{}".format( | ||
| self.__metadata["driver"]["version"], | ||
| "", | ||
| ) | ||
| if driver: | ||
| self._update_metadata(driver) | ||
|
|
||
|
|
@@ -368,28 +414,38 @@ def __init__( | |
| _truncate_metadata(self.__metadata) | ||
|
|
||
| def _update_metadata(self, driver: DriverInfo) -> None: | ||
| """Updates the client's metadata""" | ||
| if driver.name and driver.name.lower() in self.__metadata["driver"]["name"].lower().split( | ||
| "|" | ||
| ): | ||
| return | ||
|
|
||
| metadata = copy.deepcopy(self.__metadata) | ||
|
|
||
| if driver.name: | ||
| metadata["driver"]["name"] = "{}|{}".format( | ||
| metadata["driver"]["name"], | ||
| driver.name, | ||
| ) | ||
| if driver.version: | ||
| """Updates the client's metadata.""" | ||
| with self.__metadata_lock: | ||
| driver = _normalize_driver(driver) | ||
| if driver in self.__appended_drivers: | ||
| return | ||
|
|
||
| name_delims = self.__metadata["driver"]["name"].count("|") | ||
| version_delims = self.__metadata["driver"]["version"].count("|") | ||
| metadata = copy.deepcopy(self.__metadata) | ||
|
|
||
| metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name) | ||
| metadata["driver"]["version"] = "{}|{}".format( | ||
| metadata["driver"]["version"], | ||
| driver.version, | ||
| metadata["driver"]["version"], driver.version | ||
| ) | ||
| if driver.platform: | ||
| metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) | ||
|
|
||
| self.__metadata = metadata | ||
| if driver.platform: | ||
| if "platform" in metadata: | ||
| metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) | ||
| else: | ||
| metadata["platform"] = driver.platform | ||
|
|
||
| _truncate_metadata(metadata) | ||
|
|
||
| self.__metadata = metadata | ||
|
|
||
| # Only track drivers whose appended name/version pair survived | ||
| # truncation (i.e. both gained a segment), so __appended_drivers | ||
| # stays bounded and the dedup membership check stays fast. | ||
| if ( | ||
| metadata["driver"]["name"].count("|") > name_delims | ||
| and metadata["driver"]["version"].count("|") > version_delims | ||
| ): | ||
| self.__appended_drivers.append(driver) | ||
|
|
||
| @property | ||
| def _credentials(self) -> Optional[MongoCredential]: | ||
|
|
||
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.
Do we need this when we do it again just below inside the truncation loop?