Skip to content
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

Merged
merged 3 commits into from
Sep 30, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions sdk/monitor/azure-monitor-query/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Release History

## 1.0.0b5 (Unreleased)
## 1.0.0b5 (2021-10-05)

### Features Added

Expand All @@ -9,6 +9,7 @@
- Added `LogsQueryStatus` Enum to describe the status of a result.
- Added a new `LogsTableRow` type that represents a single row in a table.
- Items in `metrics` list in `MetricsResult` can now be accessed by metric names.
- Added `audience` keyword to support providing credential scope when creating clients.

### Breaking Changes

Expand All @@ -19,10 +20,6 @@
- `query_batch` API now returns a union of `LogsQueryPartialResult`, `LogsQueryError` and `LogsQueryResult`.
- `metric_namespace` is renamed to `namespace` and is a keyword-only argument in `list_metric_definitions` API.

### Bugs Fixed

### Other Changes

## 1.0.0b4 (2021-09-09)

### Features Added
Expand Down
4 changes: 4 additions & 0 deletions sdk/monitor/azure-monitor-query/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ The Azure Monitor Query client library is used to execute read-only queries agai
- [Samples][samples]
- [Change log][changelog]

## _Disclaimer_
Copy link
Contributor

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)

Copy link
Contributor Author

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


_Azure SDK Python packages support for Python 2.7 is ending 01 January 2022. For more information and questions, please refer to https://github.com/Azure/azure-sdk-for-python/issues/20691_

## Getting started

### Prerequisites
Expand Down
16 changes: 11 additions & 5 deletions sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think get_metrics_authentication_policy and get_authentication_policy could get merged into one method since the only difference is the default audience.

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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this kinda a bug fix? (something to be mentioned in changelog?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,22 @@ class MetricsQueryClient(object):
:type credential: ~azure.core.credentials.TokenCredential
:keyword endpoint: The endpoint to connect to. Defaults to 'https://management.azure.com'.
: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
audience = kwargs.pop("audience", None)
endpoint = kwargs.pop("endpoint", "https://management.azure.com")
if not endpoint.startswith("https://") and not endpoint.startswith("http://"):
endpoint = "https://" + endpoint
self._endpoint = endpoint

rakshith91 marked this conversation as resolved.
Show resolved Hide resolved
self._client = MonitorQueryClient(
credential=credential,
base_url=endpoint,
authentication_policy=get_metrics_authentication_policy(credential),
base_url=self._endpoint,
authentication_policy=get_metrics_authentication_policy(credential, audience),
**kwargs
)
self._metrics_op = self._client.metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,35 @@

def get_authentication_policy(
credential: "AsyncTokenCredential",
audience: str = None
) -> AsyncBearerTokenCredentialPolicy:
"""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 AsyncBearerTokenCredentialPolicy(
credential, "https://api.loganalytics.io/.default"
credential, scope
)

raise TypeError("Unsupported credential")


def get_metrics_authentication_policy(
credential: "AsyncTokenCredential",
audience: str = None
) -> AsyncBearerTokenCredentialPolicy:
"""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 AsyncBearerTokenCredentialPolicy(
credential, "https://management.azure.com/.default"
credential, scope
)

raise TypeError("Unsupported credential")
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,19 @@ class LogsQueryClient(object):
:type credential: ~azure.core.credentials_async.AsyncTokenCredential
:keyword endpoint: The endpoint to connect to. Defaults to 'https://api.loganalytics.io/v1'.
:paramtype endpoint: str
:keyword audience: URL to use for credential authentication with AAD.
:paramtype audience: str
"""

def __init__(self, credential: "AsyncTokenCredential", **kwargs: 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
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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,20 @@ class MetricsQueryClient(object):
:type credential: ~azure.core.credentials.TokenCredential
:keyword endpoint: The endpoint to connect to. Defaults to 'https://management.azure.com'.
:paramtype endpoint: str
:keyword audience: URL to use for credential authentication with AAD.
:paramtype audience: str
"""

def __init__(self, credential: "AsyncTokenCredential", **kwargs: Any) -> None:
audience = kwargs.pop("audience", None)
endpoint = kwargs.pop("endpoint", "https://management.azure.com")
if not endpoint.startswith("https://") and not endpoint.startswith("http://"):
endpoint = "https://" + endpoint
self._endpoint = endpoint
self._client = MonitorQueryClient(
credential=credential,
base_url=endpoint,
authentication_policy=get_metrics_authentication_policy(credential),
base_url=self._endpoint,
authentication_policy=get_metrics_authentication_policy(credential, audience),
**kwargs
)
self._metrics_op = self._client.metrics
Expand Down