Skip to content

feat(document): add optional sync client via get_mongodb_sync#8

Merged
tanmay-s-lyzr merged 2 commits into
mainfrom
feat/sync_mongo_client
Jun 11, 2026
Merged

feat(document): add optional sync client via get_mongodb_sync#8
tanmay-s-lyzr merged 2 commits into
mainfrom
feat/sync_mongo_client

Conversation

@tanmay-s-lyzr

Copy link
Copy Markdown
Collaborator

Closes #7

Mirror the Motor-based async factories with a synchronous variant for
services that don't run an event loop. get_mongodb_sync returns a raw
pymongo.MongoClient (the sync driver Motor wraps) with provider and
auth routing identical to get_mongodb:

documentdb_sync.py: connect_uri / connect_credentials / connect_tls_cert
cosmos_sync.py: connect_connection_string / connect_account_key
(MongoDB API, keys-only — same constraints as the async path)
No wrappers, consistent with the v0.2.0 document refactor: the caller
uses PyMongo's native API directly and manages client.close(). pymongo
is now explicit in the aws/azure/all extras (already transitive via
motor). Tests mirror the async suite's recording-client style.

@shreyas-lyzr shreyas-lyzr left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Three issues before merge: a security vulnerability in the pinned minimum version (blocking), a missing dev dependency that will break CI (blocking), and a routing edge case worth a docstring warning or test. Full details in inline comments.

Comment thread pyproject.toml Outdated
aws = [
"aioboto3>=13.0.0", # native async S3 / SQS / SNS / Secrets Manager / SES
"motor>=3.3.0", # async MongoDB driver for DocumentDB
"pymongo>=4.5.0", # sync MongoDB driver (optional sync client)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Security — blocking: pymongo>=4.5.0 allows any version up to (but not including) the fix, so an install could land on 4.5.x or 4.6.0–4.6.2, all of which carry CVE-2024-5629 (GHSA-m87m-mmvp-v9qm, CVSS 6.1). The vulnerability is an out-of-bounds read in the bson module: a server-crafted BSON document can cause the parser to read past the buffer. Fixed in 4.6.3. Bump all three occurrences of this lower bound (aws extra line 39, azure extra line 50, all extra line 62):

Suggested change
"pymongo>=4.5.0", # sync MongoDB driver (optional sync client)
"pymongo>=4.6.3", # sync MongoDB driver (optional sync client)

Reference: https://osv.dev/vulnerability/GHSA-m87m-mmvp-v9qm

Comment thread pyproject.toml Outdated
"azure-eventgrid>=4.9.0",
"azure-communication-email>=1.0.0",
"motor>=3.3.0", # Cosmos DB via MongoDB API
"pymongo>=4.5.0", # sync MongoDB driver (optional sync client)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same CVE-2024-5629 issue here — azure extra should also pin pymongo>=4.6.3.

Suggested change
"pymongo>=4.5.0", # sync MongoDB driver (optional sync client)
"pymongo>=4.6.3", # sync MongoDB driver (optional sync client)

Comment thread pyproject.toml Outdated
all = [
"aioboto3>=13.0.0",
"motor>=3.3.0",
"pymongo>=4.5.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same CVE-2024-5629 fix needed in the all extra:

Suggested change
"pymongo>=4.5.0",
"pymongo>=4.6.3",

@shreyas-lyzr

Copy link
Copy Markdown

Two additional notes not easily attached as inline comments:

pymongo missing from dev extra (blocking for CI)

The dev extra (pyproject.toml lines 72–85) does not list pymongo. The new test file imports it directly (from pymongo import MongoClient) and one test (test_documentdb_uri_returns_pymongo_client) constructs a real MongoClient without mocking. CI running uv sync --extra dev && uv run pytest will fail with ModuleNotFoundError: No module named 'pymongo'. Add "pymongo>=4.6.3" to the dev extra.

Routing edge case: uri + tls_cert_key_file together (suggestion)

In both get_mongodb and get_mongodb_sync (cloudrift/document/init.py lines 43 and 83), if a caller passes both uri and tls_cert_key_file, the uri branch fires first. connect_uri accepts **client_kwargs so the key file is forwarded, but it is passed as a generic kwarg rather than via the typed tlsCertificateKeyFile parameter that connect_tls_cert sets — behaviour may differ depending on pymongo's kwarg handling. At minimum the docstring of get_mongodb_sync should document which key takes priority when both are supplied, mirroring whatever the async factory already says (currently neither documents this).

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
12.6% Duplication on New Code (required ≤ 3%)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@tanmay-s-lyzr tanmay-s-lyzr merged commit d11ed61 into main Jun 11, 2026
1 of 2 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.

Expose an optional sync MongoDB client

3 participants