PYTHON-6040 Preserve version-to-name mapping in Client Metadata by blink1073 · Pull Request #3054 · mongodb/mongo-python-driver

blink1073 · 2026-09-15T00:38:01Z

Implements the DRIVERS-3251 handshake metadata update: PyMongo's driver.name and driver.version are now pipe-delimited lists with a 1:1 index correspondence. Each name segment (|c, |async, or a framework) has a matching version entry, so the number of | in name always equals the number of | in version.

Changes in this PR

Reject the | delimiter in DriverInfo fields at construction time.
Append empty version entries for the |c and |async name suffixes so name/version stay index-aligned.
Rewrite _update_metadata to always append the delimiter for name and version, deduplicate appended drivers by whole DriverInfo object, and track appended drivers.
Fix _truncate_metadata to keep name/version index-aligned when metadata is truncated to the 512-byte limit.
Update the metadata, dedup, and handshake tests.

Test Plan

Standard evergreen tasks

Checklist

Checklist for Author

Did you update the changelog (if necessary)? N/A
Is there test coverage?
Is any followup work tracked in a JIRA ticket? If so, add link(s). N/A

Checklist for Reviewer

Does the title of the PR reference a JIRA Ticket?
Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
Is all relevant documentation (README or docstring) updated?

Trim the _truncate_metadata comments and number/label the handshake prose tests (backpressure no. 9, delimiter no. 10, index no. 11).

Reapply the 512-byte handshake limit after append_metadata, guard the check/update/record sequence with a lock for thread-safe clients, and document the reserved '|' delimiter on DriverInfo.

Use _create_lock() so the metadata lock is registered with pymongo.lock and reset after a fork, avoiding a deadlock in the child process.

Trim wrapper version content before dropping name/version segments so driver identity is preserved, and recreate the platform field when a platform append follows truncation that removed it.

Revert the 'Equal versions do not collapse' prose test case to the specification and shorten the truncation comment.

Only record drivers that remain representable in the truncated metadata, so __appended_drivers cannot grow without bound and the dedup membership check stays fast. Add a regression test.

Use the name delimiter count before/after the update to decide whether an appended pair survived truncation, instead of a name/version branch that always recorded platform-only (empty name/version) drivers.

Copilot

🟡 Changes recommended

Several required index-correspondence prose cases are missing or do not exercise their stated behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates handshake metadata to preserve one-to-one correspondence between driver names and versions.

Changes:

Rejects reserved delimiters in DriverInfo.
Adds aligned metadata appending, deduplication, locking, and truncation.
Expands synchronous, asynchronous, and handshake tests.

File summaries

File	Description
`pymongo/driver_info.py`	Validates metadata delimiters.
`pymongo/pool_options.py`	Implements aligned, thread-safe metadata updates.
`test/asynchronous/test_client.py`	Tests async metadata alignment and truncation.
`test/asynchronous/test_client_metadata.py`	Adds async handshake prose tests.
`test/test_client.py`	Adds generated synchronous coverage.
`test/test_client_metadata.py`	Adds generated synchronous prose tests.
`test/mockupdb/test_handshake.py`	Updates expected handshake metadata.

Review details

Suppressed comments (1)

test/asynchronous/test_client_metadata.py:286

This does not test a wrapper matching the driver's own identity because the appended version is None. Append PyMongo's base name and version together and expect both entries, otherwise the required whole-identity case remains uncovered.

            ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"),

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Add 'Gap in middle (name)' and 'All names absent' cases and drop the non-None name assertion so empty name segments are verified to stay index-aligned.

codecov · 2026-09-15T12:30:29Z

Codecov Report

❌ Patch coverage is 89.28571% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pymongo/pool_options.py	88.88%	3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

Number index correspondence as prose test 10 and delimiter rejection as prose test 11, and mirror the specifications PR mongodb#1975 case table (order and content, resolving <driver-name>/<driver-version> at runtime).

Prefix each client metadata prose test method with its prose test number and full specification title (Test 1, 2, 9, 10, 11) and order them by prose test number.

Exercise the DriverInfo delimiter ValueError for every field, the platform-recreation path, and add truncation/bounded-retention coverage so the new pool_options and driver_info lines are covered without mockupdb.

Copilot

🟡 Changes recommended

Truncation can discard an entire wrapper name instead of retaining a size-compliant truncated value.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 7/7 changed files
Comments generated: 1
Review effort level: Balanced

When the trailing version entry is empty, shrink the oversized wrapper name instead of dropping the whole name/version pair, so a driver with a large name and no version keeps a truncated name rather than collapsing to the base entry.

Copilot

🟡 Changes recommended

Driver truncation incorrectly treats byte overflow as a character count, discarding valid multibyte metadata.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

pymongo/pool_options.py:257

overflow measures encoded bytes, whereas this slice removes characters. An oversized multibyte name such as an emoji-only wrapper is reduced to an empty segment (PyMongo|) even though many complete characters fit under 512 bytes, contradicting the intended retention of a truncated wrapper name. Apply the limit to UTF-8 bytes without splitting a code point.

            elif len(n_parts) > 1 and n_parts[-1]:
                n_parts[-1] = n_parts[-1][:-overflow]
                driver["name"] = "|".join(n_parts)

Files reviewed: 7/7 changed files
Comments generated: 1
Review effort level: Balanced

The overflow is a BSON byte count, so trim wrapper version, name, and platform by UTF-8 bytes and decode a valid prefix instead of slicing Unicode code points.

Copilot

🔵 Needs a closer look

Deduplication incorrectly treats equivalent empty-string and unset fields as distinct metadata.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

pymongo/pool_options.py:406

Empty strings are defined as unset when comparing driver metadata, but namedtuple membership distinguishes "" from None. For example, after appending DriverInfo("library", None, "platform"), appending DriverInfo("library", "", "platform") currently adds a duplicate name/version segment instead of being a no-op. Normalize fields before the deduplication comparison.

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

Normalize DriverInfo fields before the dedup comparison so None and '' are treated as the same unset value, matching the spec and avoiding duplicate name/version segments.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

🟢 Approval recommended

The implementation preserves metadata alignment, bounds truncation correctly, and includes comprehensive mirrored coverage.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

NoahStapp · 2026-09-15T19:18:09Z

 _IS_SYNC = False


+def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str:


This doesn't have to be synchronized, there's no async or sync-specific code. We can put this in test/utils_shared.py instead.

NoahStapp · 2026-09-15T19:24:28Z

+        client.append_metadata(DriverInfo("library", "", "Library Platform"))
+        metadata = client.options.pool_options.metadata
+        self.assertEqual(metadata["driver"]["name"], names)
+        self.assertEqual(metadata["driver"]["version"], vers)


Can we split this up into several smaller testcases each testing a subset of the current body?

NoahStapp · 2026-09-15T19:53:24Z

+            # Only track drivers whose appended name/version pair survived
+            # truncation (i.e. the name gained a segment), so __appended_drivers
+            # stays bounded and the dedup membership check stays fast.
+            if metadata["driver"]["name"].count("|") > name_delims:


This only checks if the name survived, not the version.

NoahStapp · 2026-09-15T19:56:20Z

        self.__credentials = credentials
        self.__metadata = copy.deepcopy(_METADATA)
+        self.__appended_drivers: list[DriverInfo] = []
+        self.__metadata_lock = _create_lock()


This uses a sync lock even on the async API, which will block the loop.

Good call, I think we don't need a lock in async at all, since the only condition that would cause a race is an async client being used in two threads, which we don't support.

NoahStapp · 2026-09-15T19:57:40Z

+                name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True)
+                await asyncio.sleep(0.005)
+
+                assert name0 is not None


These should be unittest-style asserts for consistency with our test suite.

NoahStapp · 2026-09-15T19:59:26Z

+                        driver_name=driver_name, driver_version=driver_version
+                    ),
+                )
+                await client.close()


These clients will get leaked if a subtest fails, we need a try-catch-finally block here.

I went with adding cleanup instead to avoid the indent

NoahStapp · 2026-09-15T20:01:33Z

+                await asyncio.sleep(0.005)
+                # Appending metadata containing the delimiter raises.
+                with self.assertRaises(ValueError):
+                    DriverInfo(name, version, platform)


This is inconsistent with the comment above: it doesn't append metadata, but constructs a DriverInfo.

NoahStapp · 2026-09-15T20:03:03Z

            metadata["driver"]["name"] = "{}|{}".format(
-                metadata["driver"]["name"],
-                driver.name,
+                metadata["driver"]["name"], driver.name or ""


Since the _normalize_driver(driver) call above already normalized the name, version, and platform fields, we don't need to do or "" afterwards.

Co-authored-by: Noah Stapp <noah@noahstapp.com>

Scope the metadata lock to the synchronous client and track appended name/version pairs by both fields. Split test_metadata into focused tests, close test clients in a finally block, and document the whole-DriverInfo dedup and reserved delimiter in the changelog.

Store a nullcontext for the asynchronous client so _update_metadata uses one with block for both clients.

Register client.close via addAsyncCleanup instead of a try/finally so clients are still closed when a subtest fails.

blink1073 added 2 commits September 14, 2026 19:37

PYTHON-6040 Preserve 1:1 index correspondence in client metadata

ac6ed46

PYTHON-6040 Update tests for index-aligned client metadata

5f2e74c

blink1073 closed this Sep 15, 2026

blink1073 added 7 commits September 14, 2026 20:14

PYTHON-6040 Address PR review feedback

b3d18d1

Trim the _truncate_metadata comments and number/label the handshake prose tests (backpressure no. 9, delimiter no. 10, index no. 11).

PYTHON-6040 Address Copilot review feedback

b035cfb

Reapply the 512-byte handshake limit after append_metadata, guard the check/update/record sequence with a lock for thread-safe clients, and document the reserved '|' delimiter on DriverInfo.

PYTHON-6040 Use fork-aware lock for metadata updates

61ba3a2

Use _create_lock() so the metadata lock is registered with pymongo.lock and reset after a fork, avoiding a deadlock in the child process.

PYTHON-6040 Fix truncation order and platform recreation

7e863c9

Trim wrapper version content before dropping name/version segments so driver identity is preserved, and recreate the platform field when a platform append follows truncation that removed it.

PYTHON-6040 Reverse prose test case and trim truncation comment

d45d987

Revert the 'Equal versions do not collapse' prose test case to the specification and shorten the truncation comment.

PYTHON-6040 Bound appended-driver tracking

0194f47

Only record drivers that remain representable in the truncated metadata, so __appended_drivers cannot grow without bound and the dedup membership check stays fast. Add a regression test.

PYTHON-6040 Bound dedup tracking for platform-only appends

b652b78

Use the name delimiter count before/after the update to decide whether an appended pair survived truncation, instead of a name/version branch that always recorded platform-only (empty name/version) drivers.

blink1073 reopened this Sep 15, 2026

blink1073 requested a balanced review from Copilot September 15, 2026 11:53

Copilot started reviewing on behalf of blink1073 September 15, 2026 11:53 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Comment thread test/asynchronous/test_client_metadata.py Outdated

Comment thread test/asynchronous/test_client_metadata.py Outdated

PYTHON-6040 Cover empty-name entries in index correspondence

a016133

Add 'Gap in middle (name)' and 'All names absent' cases and drop the non-None name assertion so empty name segments are verified to stay index-aligned.

blink1073 added 3 commits September 15, 2026 07:45

PYTHON-6040 Align index-correspondence prose test to specifications

b50cb0f

Number index correspondence as prose test 10 and delimiter rejection as prose test 11, and mirror the specifications PR mongodb#1975 case table (order and content, resolving <driver-name>/<driver-version> at runtime).

PYTHON-6040 Rename prose tests with their specification titles

62e83d4

Prefix each client metadata prose test method with its prose test number and full specification title (Test 1, 2, 9, 10, 11) and order them by prose test number.

PYTHON-6040 Extend metadata unit test coverage

7ff1773

Exercise the DriverInfo delimiter ValueError for every field, the platform-recreation path, and add truncation/bounded-retention coverage so the new pool_options and driver_info lines are covered without mockupdb.

blink1073 requested a balanced review from Copilot September 15, 2026 16:11

Copilot started reviewing on behalf of blink1073 September 15, 2026 16:14 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Comment thread pymongo/pool_options.py

blink1073 requested a balanced review from Copilot September 15, 2026 16:38

Copilot started reviewing on behalf of blink1073 September 15, 2026 16:39 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Comment thread pymongo/pool_options.py

PYTHON-6040 Truncate metadata by UTF-8 bytes

6ef9ab0

The overflow is a BSON byte count, so trim wrapper version, name, and platform by UTF-8 bytes and decode a valid prefix instead of slicing Unicode code points.

blink1073 requested a balanced review from Copilot September 15, 2026 17:27

Copilot started reviewing on behalf of blink1073 September 15, 2026 17:28 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

PYTHON-6040 Dedup metadata across None and empty strings

e3ead2a

Normalize DriverInfo fields before the dedup comparison so None and '' are treated as the same unset value, matching the spec and avoiding duplicate name/version segments.

blink1073 requested a balanced review from Copilot September 15, 2026 17:57

Copilot AI reviewed Sep 15, 2026

blink1073 requested a balanced review from Copilot September 15, 2026 18:04

Copilot started reviewing on behalf of blink1073 September 15, 2026 18:05 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

blink1073 marked this pull request as ready for review September 15, 2026 18:21

blink1073 requested a review from a team as a code owner September 15, 2026 18:21

blink1073 requested review from NoahStapp and sleepyStick and removed request for sleepyStick September 15, 2026 18:21

NoahStapp requested changes Sep 15, 2026

View reviewed changes

blink1073 and others added 5 commits September 15, 2026 20:18

Update pymongo/pool_options.py

917bc0a

Co-authored-by: Noah Stapp <noah@noahstapp.com>

Update test/asynchronous/test_client_metadata.py

66ce419

Co-authored-by: Noah Stapp <noah@noahstapp.com>

PYTHON-6040 Use a null context for the async metadata lock

dbc559d

Store a nullcontext for the asynchronous client so _update_metadata uses one with block for both clients.

PYTHON-6040 Close test clients with unittest cleanup

7ecbb94

Register client.close via addAsyncCleanup instead of a try/finally so clients are still closed when a subtest fails.

blink1073 requested a review from NoahStapp September 16, 2026 02:07

		_IS_SYNC = False


		def _driver_version(base_version: str, name: str, last_version: str \| None = None) -> str:

Uh oh!

Conversation

blink1073 commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes in this PR

Test Plan

Checklist

Checklist for Author

Checklist for Reviewer

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Sep 15, 2026 •

edited

Loading

codecov Bot commented Sep 15, 2026 •

edited

Loading