Deps/update pymongo #97
Conversation
WalkthroughUpdated typing annotations and small API adjustments across the codebase, encryption module, tests, and packaging/CI. Specific changes include: added explicit type parameters for class and local variables; switched some imports from Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mongoengine_plus/types/encrypted_string/base.py (1)
24-35: Critical: Update return type annotation.The return type annotation states
Binary, but Line 35 now returnsuuid_data_keydirectly (the raw_idfrom the data key document) instead of wrapping it inBinarywithUUID_SUBTYPE. This is correct for pymongo 4.x, but the type annotation is now inaccurate.Apply this diff to fix the return type:
-def get_data_key_binary(key_namespace: str, key_name: str) -> Binary: +def get_data_key_binary(key_namespace: str, key_name: str):Or, if you prefer explicit typing:
+from typing import Union +from uuid import UUID + -def get_data_key_binary(key_namespace: str, key_name: str) -> Binary: +def get_data_key_binary(key_namespace: str, key_name: str) -> Union[Binary, UUID]:
🧹 Nitpick comments (5)
mongoengine_plus/types/encrypted_string/base.py (1)
55-55: Optional: Redundant type annotation.The type annotation
client_encryption: ClientEncryptionis redundant since the context manager on line 56 already provides strong type information. While not harmful, it adds visual noise.Consider removing the annotation:
- client_encryption: ClientEncryption with ClientEncryption(tests/types/test_encrypted_string.py (2)
92-92: Optional: Redundant assertion.The assertion
assert data_key is not Noneis redundant because Line 93 immediately accessesdata_key['keyAltNames'], which would raise an exception ifdata_keywereNone.Consider removing this assertion:
- assert data_key is not None assert data_key['keyAltNames'] == [key_name]
121-121: Optional: Redundant type annotation.The type annotation
client_encryption: ClientEncryptionis redundant since the context manager on line 122 already provides strong type information.Consider removing the annotation:
- client_encryption: ClientEncryption with ClientEncryption(mongoengine_plus/types/encrypted_string/fields.py (2)
60-60: Optional: Redundant type annotation.The type annotation
client_encryption: ClientEncryptionis redundant since the context manager on line 61 already provides strong type information.Consider removing the annotation:
- client_encryption: ClientEncryption with ClientEncryption(
70-70: Optional: Redundant type annotation.The type annotation
client_encryption: ClientEncryptionis redundant since the context manager on line 71 already provides strong type information.Consider removing the annotation:
- client_encryption: ClientEncryption with ClientEncryption(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
mongoengine_plus/models/base.py(1 hunks)mongoengine_plus/types/encrypted_string/__init__.py(1 hunks)mongoengine_plus/types/encrypted_string/base.py(3 hunks)mongoengine_plus/types/encrypted_string/fields.py(3 hunks)requirements-test.txt(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/types/test_encrypted_string.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
mongoengine_plus/models/base.pymongoengine_plus/types/encrypted_string/__init__.pymongoengine_plus/types/encrypted_string/base.pymongoengine_plus/types/encrypted_string/fields.pytests/types/test_encrypted_string.pysetup.py
🪛 GitHub Actions: test
requirements.txt
[error] 1-1: pip install -qU -r requirements.txt failed: No matching distribution found for dnspython==2.8.0 (requires Python >=3.10); Ignored versions that require a different Python version. Command failed: 'pip install -qU -r requirements.txt'. Makefile exit: 2.
🔇 Additional comments (6)
mongoengine_plus/types/encrypted_string/__init__.py (1)
6-6: LGTM! Correct migration to pymongo 4.x import path.The import path update from
pymongo.encryptiontopymongo.synchronous.encryptionaligns with pymongo 4.x's new synchronous API structure.tests/types/test_encrypted_string.py (1)
9-13: LGTM! Correct migration to pymongo 4.x import paths.The import statements correctly reference
pymongo.synchronous.encryptionfor Algorithm, ClientEncryption, and _EncryptionIO, aligning with pymongo 4.x's synchronous API.mongoengine_plus/types/encrypted_string/fields.py (1)
11-11: LGTM! Improved type annotation for module constant.Explicitly typing
CODEC_OPTIONasCodecOptionsimproves code clarity and provides better IDE support.mongoengine_plus/models/base.py (1)
7-8: No issues found with the behavioral change; the defaults are necessary for the current code to work.Based on my verification:
TestModel uses
_excludedwithout defining it: Intests/models/test_base.py, TestModel defines_hiddenbut not_excluded. Whento_dict()is called (line 48), it accessesself._excluded(base.py line 15) for concatenation. This works only because the default empty list exists.The test is not failing: TestModel's
to_dict()test is not marked as skipped or xfail, indicating the code path is already active and working.All subclasses that override these variables do so explicitly:
- Address (line 18-19): Explicitly defines both
_excludedand_hidden- TestModel (line 32): Explicitly defines
_hiddenNo code relies on AttributeError: There are no
hasattr(),getattr(), or try/except patterns attempting to detect if these variables are undefined.The defaults are necessary for the current
to_dict()implementation to work correctly, particularly for subclasses that don't define all variables.setup.py (1)
30-30: LGTM! Pymongo 4.x version constraint is correctly specified.The version constraint properly allows pymongo 4.x versions while preventing automatic upgrades to 5.x, which is appropriate for this migration.
requirements-test.txt (1)
1-10: All specified package versions are available; verify compatibility by running the test suite.The latest stable versions as of November 2025 are: black 25.11.0, isort 7.0.0, mypy 1.18.2, pytest 9.0.1, and pytest-asyncio 1.3.0. These versions match the pinned ranges in
requirements-test.txtand are publicly available. However, ensure the test suite runs successfully with these major version updates before merging, as they may include breaking changes in linting, type checking, or testing behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 15 15
Lines 254 254
Branches 23 23
=========================================
Hits 254 254
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/types/test_encrypted_string.py (1)
118-118: Type annotation may be redundant.The explicit type annotation
client_encryption: ClientEncryptionbefore the context manager is valid but potentially unnecessary, as type checkers can typically infer the type fromClientEncryption's__enter__method. However, if this improves type checking in your environment or aligns with your typing standards, it's acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/types/test_encrypted_string.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
tests/types/test_encrypted_string.py
🔇 Additional comments (2)
tests/types/test_encrypted_string.py (2)
89-89: Good addition for test clarity.Adding the explicit
assert data_key is not Noneimproves test reliability and helps type checkers understand that subsequent attribute accesses ondata_keyare safe.
9-10: ****The imports in lines 9-10 are correct for pymongo 4.x. According to PyMongo documentation,
AlgorithmandClientEncryptionshould remain imported frompymongo.encryption, while synchronous-specific utilities like_EncryptionIOare correctly imported frompymongo.synchronous.encryption. The split is intentional per pymongo's API design.Likely an incorrect or invalid review comment.
| partialFilterExpression={"keyAltNames": {"$exists": True}}, | ||
| ) | ||
|
|
||
| client_encryption: ClientEncryption |
There was a problem hiding this comment.
Si, el lint ahora es más estricto, marca este error:
error: Need type annotation for "client_encryption" [var-annotated]
Se creo 1 registro con la versión
1.2.1y otro con esta nueva versión, ambos pueden ser leídos tanto con la versión1.2.1como con esta nueva:Summary by CodeRabbit
Dependencies
Improvements
Tests / CI