-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add credential Scope keyword #20987
Add credential Scope keyword #20987
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,32 +15,38 @@ | |
|
||
|
||
def get_authentication_policy( | ||
credential, # type: TokenCredential | ||
credential, # type: "TokenCredential" | ||
audience=None # type: str | ||
): | ||
# type: (...) -> BearerTokenCredentialPolicy | ||
"""Returns the correct authentication policy""" | ||
|
||
if not audience: | ||
audience = "https://api.loganalytics.io/" | ||
scope = audience.rstrip('/') + "/.default" | ||
if credential is None: | ||
raise ValueError("Parameter 'credential' must not be None.") | ||
if hasattr(credential, "get_token"): | ||
return BearerTokenCredentialPolicy( | ||
credential, "https://api.loganalytics.io/.default" | ||
credential, scope | ||
) | ||
|
||
raise TypeError("Unsupported credential") | ||
|
||
|
||
def get_metrics_authentication_policy( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it doesn't need to happen in this PR, but something we could consider to avoid duplication |
||
credential, # type: TokenCredential | ||
audience=None # type: str | ||
): | ||
# type: (...) -> BearerTokenCredentialPolicy | ||
"""Returns the correct authentication policy""" | ||
|
||
if not audience: | ||
audience = "https://management.azure.com/" | ||
scope = audience.rstrip('/') + "/.default" | ||
if credential is None: | ||
raise ValueError("Parameter 'credential' must not be None.") | ||
if hasattr(credential, "get_token"): | ||
return BearerTokenCredentialPolicy( | ||
credential, "https://management.azure.com/.default" | ||
credential, scope | ||
) | ||
|
||
raise TypeError("Unsupported credential") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,15 +49,20 @@ class LogsQueryClient(object): | |
:type credential: ~azure.core.credentials.TokenCredential | ||
:keyword endpoint: The endpoint to connect to. Defaults to 'https://api.loganalytics.io'. | ||
:paramtype endpoint: str | ||
:keyword audience: URL to use for credential authentication with AAD. | ||
:paramtype audience: str | ||
""" | ||
|
||
def __init__(self, credential, **kwargs): | ||
# type: (TokenCredential, Any) -> None | ||
|
||
self._endpoint = kwargs.pop("endpoint", "https://api.loganalytics.io/v1") | ||
audience = kwargs.pop("audience", None) | ||
endpoint = kwargs.pop("endpoint", "https://api.loganalytics.io/v1") | ||
if not endpoint.startswith("https://") and not endpoint.startswith("http://"): | ||
endpoint = "https://" + endpoint | ||
Comment on lines
+60
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this kinda a bug fix? (something to be mentioned in changelog?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not really - it's more of a convenience |
||
self._endpoint = endpoint | ||
self._client = MonitorQueryClient( | ||
credential=credential, | ||
authentication_policy=get_authentication_policy(credential), | ||
authentication_policy=get_authentication_policy(credential, audience), | ||
base_url=self._endpoint, | ||
**kwargs | ||
) | ||
|
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.
(if you're addressing Laurent's request in one PR, don't miss the 3.10 classifier in setup.py :P)
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.
have that in a different PR :P