Skip to content

PYTHON-6154 Extract KMS connect helpers into dedicated modules - #3110

Merged
blink1073 merged 4 commits into
mongodb:mainfrom
blink1073:PYTHON-6154
Oct 9, 2026
Merged

blink1073 merged 4 commits into
mongodb:mainfrom
blink1073:PYTHON-6154

Conversation

@blink1073

@blink1073 blink1073 commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

PYTHON-6154

Summary

Moves the KMS connect plumbing out of the encryption modules into dedicated private modules and avoids duplication of tests using a facade pattern in a single test module. This work prepares for the CSFLE HTTP proxy KMS connect helpers (PYTHON-6147).

Motivation

The encryption modules mix encryption orchestration with socket-level connect code. The dedicated modules let the proxy work extend the connect path without touching the encryption modules. The tests also did not need a full client, and can be deduped to run against sync and async versions in the same module.

Changes

  • Extracted the shared KMS connect types and helpers into pymongo/_kms_connect.py.
  • Moved the per-API connect logic into dedicated async and sync modules, with the public re-exports unchanged.
  • Replaced the mirrored KMS connect tests with one file parametrized over both APIs.

Testing

  • just lint and just typing pass.
  • pytest test/test_kms_connect.py -m "" passes all 33 tests.

Move the KMS connect plumbing out of the encryption modules into
dedicated private modules, prep for the CSFLE HTTP proxy KMS connect
helpers (PYTHON-6147):

- pymongo/_kms_connect.py holds KMSConnectContext, the callback type
  aliases, and _close_rejected_kms_socket, shared by both APIs.
- pymongo/{asynchronous,synchronous}/_kms_connect.py hold _connect_kms
  and _KMS_CONNECT_TIMEOUT; the synchronous module is generated by
  synchro.
- encryption_options.py re-exports the public names unchanged.
- test/asynchronous/test_kms_connect.py is expanded to cover the new
  helpers and test/test_kms_connect.py is its synchro-generated mirror.
Deliver the PR's stated goals:

- Rename pymongo/_kms_connect.py to pymongo/_kms_connect_shared.py, so
  the shared module reads naturally next to the per-flavor
  pymongo/{asynchronous,synchronous}/_kms_connect.py modules.
- Genericize the shared module docstring: it enumerated the module's
  exact contents, which would go stale as helpers move in.
- Drop the PYTHON-6147 feature tests (HTTP proxy helpers and the prose
  class) that leaked into this refactor and broke test collection: they
  import HTTPProxyKMSConnect/AsyncHTTPProxyKMSConnect, which are added
  by PYTHON-6147, not by this PR.
- Replace the synchro-mirrored test pair with a single hand-written
  test/test_kms_connect.py, written once and parameterized over both
  APIs through a Flavor facade (the 18 pre-existing tests, 32 variants,
  byte-identical bodies). The async-side file is gone, so synchro no
  longer mirrors it.
Deliver the PR's stated goals:

- Rename pymongo/_kms_connect.py to pymongo/_kms_connect_shared.py, so
  the shared module reads naturally next to the per-API
  pymongo/{asynchronous,synchronous}/_kms_connect.py modules.
- Genericize the shared module docstring: it enumerated the module's
  exact contents, which would go stale as helpers move in.
- Drop the PYTHON-6147 feature tests (HTTP proxy helpers and the prose
  class) that leaked into this refactor and broke test collection: they
  import HTTPProxyKMSConnect/AsyncHTTPProxyKMSConnect, which are added
  by PYTHON-6147, not by this PR.
- Replace the synchro-mirrored test pair with a single hand-written
  test/test_kms_connect.py, written once and parameterized over both
  APIs through a Facade class (the 18 pre-existing tests, 32 variants,
  byte-identical bodies). The async-side file is gone, so synchro no
  longer mirrors it.
- Tighten the docstrings and comments across the touched modules and
  the test file.
Deliver the PR's stated goals:

- Rename pymongo/_kms_connect.py to pymongo/_kms_connect_shared.py, so
  the shared module reads naturally next to the per-API
  pymongo/{asynchronous,synchronous}/_kms_connect.py modules.
- Genericize the shared module docstring: it enumerated the module's
  exact contents, which would go stale as helpers move in.
- Drop the PYTHON-6147 feature tests (HTTP proxy helpers and the prose
  class) that leaked into this refactor and broke test collection: they
  import HTTPProxyKMSConnect/AsyncHTTPProxyKMSConnect, which are added
  by PYTHON-6147, not by this PR.
- Replace the synchro-mirrored test pair with a single hand-written
  test/test_kms_connect.py, written once and parameterized over both
  APIs through a Facade class (the 18 pre-existing tests, 32 variants,
  byte-identical bodies). The async-side file is gone, so synchro no
  longer mirrors it.
- Tighten the docstrings and comments across the touched modules and
  the test file.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The extraction preserves existing behavior and provides consolidated coverage for both APIs.

0 open findings

What changed in this PR

Extracts KMS connection plumbing from encryption orchestration into dedicated shared, asynchronous, and synchronous modules.

Changes:

  • Centralizes shared KMS callback types and validation helpers.
  • Moves API-specific connection logic into dedicated modules.
  • Consolidates synchronous and asynchronous tests into one parametrized suite.
File Description
pymongo/​_kms_connect_shared.py Adds shared KMS types and cleanup logic.
pymongo/​asynchronous/​_kms_connect.py Adds asynchronous KMS connection handling.
pymongo/​synchronous/​_kms_connect.py Adds generated synchronous connection handling.
pymongo/​asynchronous/​encryption.py Delegates async KMS connections to the new module.
pymongo/​synchronous/​encryption.py Delegates sync KMS connections to the new module.
pymongo/​encryption_options.py Re-exports the relocated callback types.
test/​test_kms_connect.py Tests both APIs through parametrization.
test/​asynchronous/​test_kms_connect.py Removes the superseded mirrored test suite.

🧠 Review effort: Balanced


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

@blink1073
blink1073 marked this pull request as ready for review October 8, 2026 22:26
@blink1073
blink1073 requested a review from a team as a code owner October 8, 2026 22:26
@blink1073
blink1073 requested review from NoahStapp and sleepyStick and removed request for NoahStapp October 8, 2026 22:26
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.23490% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pymongo/synchronous/_kms_connect.py 69.35% 17 Missing and 2 partials ⚠️
pymongo/asynchronous/_kms_connect.py 95.16% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@blink1073
blink1073 merged commit 56d8e6a into mongodb:main Oct 9, 2026
92 of 94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants