-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-2267: Allow UUID key_id to be passed to ClientEncryption.encrypt #1494
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
Conversation
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.
Thanks, I've scheduled the tests. After this is merged I'll add a test for encrypt_expression since it's a bit more work.
pymongo/encryption.py
Outdated
key_alt_name: Optional[str] = None, | ||
query_type: Optional[str] = None, | ||
contention_factor: Optional[int] = None, | ||
range_opts: Optional[RangeOpts] = None, | ||
is_expression: bool = False, | ||
) -> Any: | ||
self._check_closed() | ||
if isinstance(key_id, uuid.UUID): | ||
key_id = Binary(key_id.bytes, UUID_SUBTYPE) |
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.
Let's use Binary.from_uuid(key_id)
here.
@@ -795,7 +799,7 @@ def encrypt( | |||
self, | |||
value: Any, | |||
algorithm: str, | |||
key_id: Optional[Binary] = None, | |||
key_id: Optional[Union[Binary, uuid.UUID]] = None, |
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.
In the docstring below can you add this?
.. versionchanged:: 4.7
``key_id`` can now be passed in as a :class:`uuid.UUID`.
.. versionchanged:: 4.2
...
And same for encrypt_expression.
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.
Thanks LGTM! Just waiting for the tests to pass before merging.
Modify the ClientEncryption.encrypt API to accept native UUIDs directly. jira task
I validated my changes with the TestExplicitSimple cases. pytest -v -s test/test_encryption.py::TestExplicitSimple
I tried to run all the test_encryption.py tests, but some of the tests require environment credentials, and as far as I understand, I need MongoDB Enterprise for mongocryptd support.