From b4a33f275f6e8e1824ba8f00802eb5f814a61450 Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 25 Aug 2021 14:15:05 -0700 Subject: [PATCH 1/5] Get rid of LogsBatchQueryResult --- sdk/monitor/azure-monitor-query/CHANGELOG.md | 4 +- sdk/monitor/azure-monitor-query/README.md | 7 ++- .../azure/monitor/query/__init__.py | 2 - .../azure/monitor/query/_helpers.py | 5 +- .../azure/monitor/query/_logs_query_client.py | 15 +++--- .../azure/monitor/query/_models.py | 52 +------------------ .../query/aio/_logs_query_client_async.py | 15 +++--- .../samples/sample_batch_query.py | 2 +- .../tests/async/test_logs_client_async.py | 12 ++++- .../tests/test_logs_client.py | 13 ++++- 10 files changed, 43 insertions(+), 84 deletions(-) diff --git a/sdk/monitor/azure-monitor-query/CHANGELOG.md b/sdk/monitor/azure-monitor-query/CHANGELOG.md index 5805d4278e0b..cae3e02459f1 100644 --- a/sdk/monitor/azure-monitor-query/CHANGELOG.md +++ b/sdk/monitor/azure-monitor-query/CHANGELOG.md @@ -15,7 +15,7 @@ - Rename `batch_query` to `query_batch`. - Rename `LogsBatchQueryRequest` to `LogsBatchQuery`. - `include_render` is now renamed to `include_visualization` in the query API. -- `LogsQueryResult` and `LogsBatchQueryResult` now return `visualization` instead of `render`. +- `LogsQueryResult` now returns `visualization` instead of `render`. - `start_time`, `duration` and `end_time` are now replaced with a single param called `timespan` - `resourceregion` is renamed to `resource_region` in the MetricResult type. - `top` is renamed to `max_results` in the metric's `query` API. @@ -30,11 +30,11 @@ - `AggregationType` is renamed to `MetricAggregationType`. - Removed `LogsBatchResultError` type. - `LogsQueryResultTable` is named to `LogsTable` -- `LogsQueryResultColumn` is renamed to `LogsTableColumn` - `LogsTableColumn` is now removed. Column labels are strings instead. - `start_time` in `list_metric_namespaces` API is now a datetime. - The order of params in `LogsBatchQuery` is changed. Also, `headers` is no longer accepted. - `timespan` is now a required keyword-only argument in logs APIs. +- batch api now returns a list of `LogsQueryResult` objects. ### Bugs Fixed diff --git a/sdk/monitor/azure-monitor-query/README.md b/sdk/monitor/azure-monitor-query/README.md index 8d3d209f4c24..16d2176f910a 100644 --- a/sdk/monitor/azure-monitor-query/README.md +++ b/sdk/monitor/azure-monitor-query/README.md @@ -215,14 +215,13 @@ for rsp in response: #### Handling the response for Logs Query -The `query` API returns the `LogsQueryResult` while the `batch_query` API returns the `LogsBatchQueryResult`. +The `query` API returns the `LogsQueryResult` while the `batch_query` API returns list of `LogsQueryResult`. Here is a heirarchy of the response: ``` -LogsQueryResult / LogsBatchQueryResult -|---id (this exists in `LogsBatchQueryResult` object only) -|---status (this exists in `LogsBatchQueryResult` object only) +LogsQueryResult +|---status |---statistics |---visualization |---error diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/__init__.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/__init__.py index d201fbff31bb..9043ee10d44c 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/__init__.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/__init__.py @@ -9,7 +9,6 @@ from ._models import ( MetricAggregationType, - LogsBatchQueryResult, LogsQueryResult, LogsTable, MetricsResult, @@ -30,7 +29,6 @@ __all__ = [ "MetricAggregationType", "LogsQueryClient", - "LogsBatchQueryResult", "LogsQueryResult", "LogsTable", "LogsBatchQuery", diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py index cda750cd845e..a64e712f7c63 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py @@ -45,10 +45,9 @@ def process_error(exception): raise_error = HttpResponseError raise raise_error(message=exception.message, response=exception.response) -def order_results(request_order, responses): - mapping = {item.id: item for item in responses} +def order_results(request_order, mapping, type): ordered = [mapping[id] for id in request_order] - return ordered + return [type._from_generated(rsp) for rsp in ordered] # pylint: disable=protected-access def construct_iso8601(timespan=None): if not timespan: diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/_logs_query_client.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/_logs_query_client.py index eaac63c874bc..d2d04abb7d77 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/_logs_query_client.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/_logs_query_client.py @@ -13,7 +13,7 @@ from ._generated.models import BatchRequest, QueryBody as LogsQueryBody from ._helpers import get_authentication_policy, process_error, construct_iso8601, order_results -from ._models import LogsQueryResult, LogsBatchQuery, LogsBatchQueryResult +from ._models import LogsBatchQuery, LogsQueryResult if TYPE_CHECKING: from azure.core.credentials import TokenCredential @@ -128,7 +128,7 @@ def query(self, workspace_id, query, **kwargs): @distributed_trace def query_batch(self, queries, **kwargs): - # type: (Union[Sequence[Dict], Sequence[LogsBatchQuery]], Any) -> List[LogsBatchQueryResult] + # type: (Union[Sequence[Dict], Sequence[LogsBatchQuery]], Any) -> List[LogsQueryResult] """Execute a list of analytics queries. Each request can be either a LogQueryRequest object or an equivalent serialized model. @@ -136,8 +136,8 @@ def query_batch(self, queries, **kwargs): :param queries: The list of queries that should be processed :type queries: list[dict] or list[~azure.monitor.query.LogsBatchQuery] - :return: List of LogsBatchQueryResult, or the result of cls(response) - :rtype: list[~azure.monitor.query.LogsBatchQueryResult] + :return: List of LogsQueryResult, or the result of cls(response) + :rtype: list[~azure.monitor.query.LogsQueryResult] :raises: ~azure.core.exceptions.HttpResponseError .. admonition:: Example: @@ -160,11 +160,8 @@ def query_batch(self, queries, **kwargs): request_order = [req['id'] for req in queries] batch = BatchRequest(requests=queries) generated = self._query_op.batch(batch, **kwargs) - return order_results( - request_order, - [ - LogsBatchQueryResult._from_generated(rsp) for rsp in generated.responses # pylint: disable=protected-access - ]) + mapping = {item.id: item for item in generated.responses} + return order_results(request_order, mapping, LogsQueryResult) def close(self): # type: () -> None diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py index 18cfa1245435..842381fe4176 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py @@ -47,46 +47,6 @@ def _from_generated(cls, generated): ) -class LogsQueryResult(object): - """Contains the tables, columns & rows resulting from a query. - - :ivar tables: The list of tables, columns and rows. - :vartype tables: list[~azure.monitor.query.LogsTable] - :ivar statistics: This will include a statistics property in the response that describes various - performance statistics such as query execution time and resource usage. - :vartype statistics: object - :ivar visualization: This will include a visualization property in the response that specifies the type of - visualization selected by the query and any properties for that visualization. - :vartype visualization: object - :ivar error: Any error info. - :vartype error: ~azure.core.exceptions.HttpResponseError - """ - def __init__(self, **kwargs): - # type: (Any) -> None - self.tables = kwargs.get("tables", None) - self.statistics = kwargs.get("statistics", None) - self.visualization = kwargs.get("visualization", None) - self.error = kwargs.get("error", None) - - @classmethod - def _from_generated(cls, generated): - if not generated: - return cls() - tables = None - if generated.tables is not None: - tables = [ - LogsTable._from_generated( # pylint: disable=protected-access - table - ) for table in generated.tables - ] - return cls( - tables=tables, - statistics=generated.statistics, - visualization=generated.render, - error=generated.error - ) - - class MetricsResult(object): """The response to a metrics query. @@ -193,13 +153,9 @@ def _to_generated(self): workspace=self.workspace ) -class LogsBatchQueryResult(object): - """The LogsBatchQueryResult. +class LogsQueryResult(object): + """The LogsQueryResult. - :ivar id: the request id of the request that was sent. - :vartype id: str - :ivar status: status code of the response. - :vartype status: int :ivar tables: The list of tables, columns and rows. :vartype tables: list[~azure.monitor.query.LogsTable] :ivar statistics: This will include a statistics property in the response that describes various @@ -215,8 +171,6 @@ def __init__( self, **kwargs ): - self.id = kwargs.get('id', None) - self.status = kwargs.get('status', None) self.tables = kwargs.get('tables', None) self.error = kwargs.get('error', None) self.statistics = kwargs.get('statistics', None) @@ -234,8 +188,6 @@ def _from_generated(cls, generated): ) for table in generated.body.tables ] return cls( - id=generated.id, - status=generated.status, tables=tables, statistics=generated.body.statistics, visualization=generated.body.render, diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/aio/_logs_query_client_async.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/aio/_logs_query_client_async.py index 5039f62402f6..19994423c947 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/aio/_logs_query_client_async.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/aio/_logs_query_client_async.py @@ -14,7 +14,7 @@ from .._generated.models import BatchRequest, QueryBody as LogsQueryBody from .._helpers import process_error, construct_iso8601, order_results -from .._models import LogsQueryResult, LogsBatchQuery, LogsBatchQueryResult +from .._models import LogsQueryResult, LogsBatchQuery from ._helpers_asyc import get_authentication_policy if TYPE_CHECKING: @@ -115,7 +115,7 @@ async def query_batch( self, queries: Union[Sequence[Dict], Sequence[LogsBatchQuery]], **kwargs: Any - ) -> List[LogsBatchQueryResult]: + ) -> List[LogsQueryResult]: """Execute a list of analytics queries. Each request can be either a LogQueryRequest object or an equivalent serialized model. @@ -123,8 +123,8 @@ async def query_batch( :param queries: The list of queries that should be processed :type queries: list[dict] or list[~azure.monitor.query.LogsBatchQuery] - :return: list of LogsBatchQueryResult objects, or the result of cls(response) - :rtype: list[~azure.monitor.query.LogsBatchQueryResult] + :return: list of LogsQueryResult objects, or the result of cls(response) + :rtype: list[~azure.monitor.query.LogsQueryResult] :raises: ~azure.core.exceptions.HttpResponseError """ try: @@ -138,11 +138,8 @@ async def query_batch( request_order = [req['id'] for req in queries] batch = BatchRequest(requests=queries) generated = await self._query_op.batch(batch, **kwargs) - return order_results( - request_order, - [ - LogsBatchQueryResult._from_generated(rsp) for rsp in generated.responses # pylint: disable=protected-access - ]) + mapping = {item.id: item for item in generated.responses} + return order_results(request_order, mapping, LogsQueryResult) async def __aenter__(self) -> "LogsQueryClient": await self._client.__aenter__() diff --git a/sdk/monitor/azure-monitor-query/samples/sample_batch_query.py b/sdk/monitor/azure-monitor-query/samples/sample_batch_query.py index 3705d94cf494..c0838643dce4 100644 --- a/sdk/monitor/azure-monitor-query/samples/sample_batch_query.py +++ b/sdk/monitor/azure-monitor-query/samples/sample_batch_query.py @@ -37,7 +37,7 @@ for response in responses: try: table = response.tables[0] - df = pd.DataFrame(table.rows, columns=[col.name for col in table.columns]) + df = pd.DataFrame(table.rows, columns=table.columns) print(df) print("\n\n-------------------------\n\n") except TypeError: diff --git a/sdk/monitor/azure-monitor-query/tests/async/test_logs_client_async.py b/sdk/monitor/azure-monitor-query/tests/async/test_logs_client_async.py index fb49883cff6d..bf7ade3b9ad2 100644 --- a/sdk/monitor/azure-monitor-query/tests/async/test_logs_client_async.py +++ b/sdk/monitor/azure-monitor-query/tests/async/test_logs_client_async.py @@ -69,7 +69,7 @@ async def test_logs_query_batch_raises_on_no_timespan(): @pytest.mark.live_test_only @pytest.mark.asyncio -async def test_logs_query_batch(): +async def test_logs_query_batch_default(): client = LogsQueryClient(_credential()) requests = [ @@ -85,7 +85,7 @@ async def test_logs_query_batch(): workspace_id= os.environ['LOG_WORKSPACE_ID'] ), LogsBatchQuery( - query= "AppRequests | take 2", + query= "Wrong query | take 2", workspace_id= os.environ['LOG_WORKSPACE_ID'], timespan=None ), @@ -93,6 +93,14 @@ async def test_logs_query_batch(): response = await client.query_batch(requests) assert len(response) == 3 + r0 = response[0] + assert r0.tables[0].columns == ['count_'] + r1 = response[1] + assert r1.tables[0].columns[0] == 'TimeGenerated' + assert r1.tables[0].columns[1] == '_ResourceId' + assert r1.tables[0].columns[2] == 'avgRequestDuration' + r2 = response[2] + assert r2.error is not None @pytest.mark.skip('https://github.com/Azure/azure-sdk-for-python/issues/19382') @pytest.mark.live_test_only diff --git a/sdk/monitor/azure-monitor-query/tests/test_logs_client.py b/sdk/monitor/azure-monitor-query/tests/test_logs_client.py index dba61f5471d1..248f44796cbe 100644 --- a/sdk/monitor/azure-monitor-query/tests/test_logs_client.py +++ b/sdk/monitor/azure-monitor-query/tests/test_logs_client.py @@ -76,7 +76,7 @@ def test_logs_server_timeout(): assert 'Gateway timeout' in e.value.message @pytest.mark.live_test_only -def test_logs_query_batch(): +def test_logs_query_batch_default(): client = LogsQueryClient(_credential()) requests = [ @@ -92,7 +92,7 @@ def test_logs_query_batch(): workspace_id= os.environ['LOG_WORKSPACE_ID'] ), LogsBatchQuery( - query= "AppRequests | take 2", + query= "Wrong query | take 2", workspace_id= os.environ['LOG_WORKSPACE_ID'], timespan=None ), @@ -100,6 +100,15 @@ def test_logs_query_batch(): response = client.query_batch(requests) assert len(response) == 3 + + r0 = response[0] + assert r0.tables[0].columns == ['count_'] + r1 = response[1] + assert r1.tables[0].columns[0] == 'TimeGenerated' + assert r1.tables[0].columns[1] == '_ResourceId' + assert r1.tables[0].columns[2] == 'avgRequestDuration' + r2 = response[2] + assert r2.error is not None @pytest.mark.live_test_only def test_logs_single_query_with_statistics(): From 7dfca19fe88946c3a4c85e48b25b10009c8f64aa Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 25 Aug 2021 14:17:33 -0700 Subject: [PATCH 2/5] Update sdk/monitor/azure-monitor-query/README.md --- sdk/monitor/azure-monitor-query/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/monitor/azure-monitor-query/README.md b/sdk/monitor/azure-monitor-query/README.md index 16d2176f910a..16ed4a04e214 100644 --- a/sdk/monitor/azure-monitor-query/README.md +++ b/sdk/monitor/azure-monitor-query/README.md @@ -221,7 +221,6 @@ Here is a heirarchy of the response: ``` LogsQueryResult -|---status |---statistics |---visualization |---error From 69393bde3d77fb0737bfae10e0079fbd09704a82 Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 25 Aug 2021 14:33:20 -0700 Subject: [PATCH 3/5] oops --- .../azure/monitor/query/_models.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py index 842381fe4176..3d4ec17cfc78 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py @@ -178,20 +178,23 @@ def __init__( @classmethod def _from_generated(cls, generated): + if not generated: return cls() tables = None - if generated.body.tables is not None: + if hasattr(generated, 'body'): + generated = generated.body + if generated.tables is not None: tables = [ LogsTable._from_generated( # pylint: disable=protected-access table - ) for table in generated.body.tables + ) for table in generated.tables ] return cls( tables=tables, - statistics=generated.body.statistics, - visualization=generated.body.render, - error=generated.body.error + statistics=generated.statistics, + visualization=generated.render, + error=generated.error ) From 58f09125dba24658654e3b9f893f77492d2c8426 Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 25 Aug 2021 15:13:10 -0700 Subject: [PATCH 4/5] comments --- .../azure-monitor-query/azure/monitor/query/_helpers.py | 4 ++-- .../azure-monitor-query/azure/monitor/query/_models.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py index a64e712f7c63..8b3194f3dc14 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py @@ -45,9 +45,9 @@ def process_error(exception): raise_error = HttpResponseError raise raise_error(message=exception.message, response=exception.response) -def order_results(request_order, mapping, type): +def order_results(request_order, mapping, obj): ordered = [mapping[id] for id in request_order] - return [type._from_generated(rsp) for rsp in ordered] # pylint: disable=protected-access + return [obj._from_generated(rsp) for rsp in ordered] # pylint: disable=protected-access def construct_iso8601(timespan=None): if not timespan: diff --git a/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py b/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py index 3d4ec17cfc78..84e0a05f4cd1 100644 --- a/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py +++ b/sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py @@ -12,6 +12,7 @@ from ._helpers import construct_iso8601, process_row from ._generated.models import ( BatchQueryRequest as InternalLogQueryRequest, + BatchQueryResponse ) @@ -182,7 +183,7 @@ def _from_generated(cls, generated): if not generated: return cls() tables = None - if hasattr(generated, 'body'): + if isinstance(generated, BatchQueryResponse): generated = generated.body if generated.tables is not None: tables = [ From 542fa041735fb3ba75a0a14d253a69f706265b0f Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 25 Aug 2021 15:38:12 -0700 Subject: [PATCH 5/5] row --- sdk/monitor/azure-monitor-query/tests/test_logs_response.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/monitor/azure-monitor-query/tests/test_logs_response.py b/sdk/monitor/azure-monitor-query/tests/test_logs_response.py index 3c69e01caa69..f3632faef97f 100644 --- a/sdk/monitor/azure-monitor-query/tests/test_logs_response.py +++ b/sdk/monitor/azure-monitor-query/tests/test_logs_response.py @@ -27,5 +27,3 @@ def test_query_response_types(): assert isinstance(result.tables[0].rows[0][1], six.string_types) # _ResourceId generated is a string assert isinstance(result.tables[0].rows[0][2], bool) # Success generated is a bool assert isinstance(result.tables[0].rows[0][3], int) # ItemCount generated is a int - assert isinstance(result.tables[0].rows[0][4], float) # DurationMs generated is a real -