-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5444 Update OIDC tests use camelCase options #2436
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
@@ -1119,7 +1119,7 @@ async def test_5_2_azure_with_bad_username(self): | |||
raise unittest.SkipTest("Test is only supported on Azure") | |||
|
|||
opts = parse_uri(self.uri_single)["options"] | |||
token_aud = opts["authmechanismproperties"]["TOKEN_RESOURCE"] | |||
token_aud = opts["authMechanismProperties"]["TOKEN_RESOURCE"] | |||
|
|||
props = dict(TOKEN_RESOURCE=token_aud, ENVIRONMENT="azure") | |||
client = await self.create_client(username="bad", authmechanismproperties=props) |
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.
Missed kwarg here. It's inconsistent with line 1114.
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.
These aren't subject to normalization rules.
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.
Still. It feels like consistency would be helpful. create_client
pops "authmechanismproperties"
from kwargs
, but in the docstring for MongoClient
, the kwarg
is authMechanismProperties
.
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.
That value was set elsewhere, I wanted to make the smallest change possible for this PR
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.
Because testing OIDC is expensive (time and compute-wise)
@@ -1117,7 +1117,7 @@ def test_5_2_azure_with_bad_username(self): | |||
raise unittest.SkipTest("Test is only supported on Azure") | |||
|
|||
opts = parse_uri(self.uri_single)["options"] | |||
token_aud = opts["authmechanismproperties"]["TOKEN_RESOURCE"] | |||
token_aud = opts["authMechanismProperties"]["TOKEN_RESOURCE"] | |||
|
|||
props = dict(TOKEN_RESOURCE=token_aud, ENVIRONMENT="azure") | |||
client = self.create_client(username="bad", authmechanismproperties=props) |
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.
Here again.
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.
LGTM.
Passing build: https://spruce.mongodb.com/version/6877c6284320e70007aa831f/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC