PYTHON-5143 Support auto encryption in unified tests #2488
Conversation
This reverts commit f0881db.
There was a problem hiding this comment.
LGTM and looks helpful for mongodb/django-mongodb-backend#329
test/asynchronous/unified_format.py
Outdated
| @@ -488,6 +515,7 @@ def tearDownClass(cls) -> None: | |||
| async def asyncSetUp(self): | |||
| # super call creates internal client cls.client | |||
| await super().asyncSetUp() | |||
|
|
|||
| @@ -118,10 +119,22 @@ | |||
| ("kmip", KMIP_CREDS), | |||
| ("kmip:name1", KMIP_CREDS), | |||
| ]: | |||
| # Use the temp aws creds for autoEncryptOpts. | |||
There was a problem hiding this comment.
Can you explain why we do this specifically for AWS?
There was a problem hiding this comment.
For clientEncryptionOpts, the tests are using the non-temp credentials, and for autoEncryptOpts they're using the temp credentials. Ideally they'd be the same, but other teams have already implemented.
There was a problem hiding this comment.
For every provider? Or only for AWS tests?
There was a problem hiding this comment.
AWS is the only provider that has temp credentials as an option
test/asynchronous/helpers.py
Outdated
| "sessionToken": os.environ.get("CSFLE_AWS_TEMP_SESSION_TOKEN", ""), | ||
| } | ||
|
|
||
| ALL_KMS_PROVIDERS = dict( |
There was a problem hiding this comment.
Do these need to be here where they'll be duplicated by synchro? Or can they go in a shared file in test?
There was a problem hiding this comment.
Are you suggesting we move the rest of the constants as well?
There was a problem hiding this comment.
Any constant (or function, value, anything really) that doesn't change based on async/sync should live outside of the synchro'd directories, yeah.
There was a problem hiding this comment.
Okay, I'll refactor a bit
There was a problem hiding this comment.
Updated, let's see how the tests do
|
Okay, this is ready for review again |
Uh oh!
There was an error while loading. Please reload this page.