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

Get rid of LogsBatchQueryResult #20418

Merged
merged 5 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions sdk/monitor/azure-monitor-query/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down
7 changes: 3 additions & 4 deletions sdk/monitor/azure-monitor-query/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
rakshith91 marked this conversation as resolved.
Show resolved Hide resolved
|---statistics
|---visualization
|---error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from ._models import (
MetricAggregationType,
LogsBatchQueryResult,
LogsQueryResult,
LogsTable,
MetricsResult,
Expand All @@ -30,7 +29,6 @@
__all__ = [
"MetricAggregationType",
"LogsQueryClient",
"LogsBatchQueryResult",
"LogsQueryResult",
"LogsTable",
"LogsBatchQuery",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Does this work correctly for LogsBatchQueryResult?

I see we have

visualization=generated.render, for LogsQueryResult while we have visualization=generated.body.render, for LogsBatchQueryResult.

Copy link
Member

Choose a reason for hiding this comment

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

Never mind. I see you updated _from_generated below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We got rid of LogsBAtchQueryResult - yes ; i have added tests to verify it's working


def construct_iso8601(timespan=None):
if not timespan:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -128,16 +128,16 @@ 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.

The response is returned in the same order as that of the requests sent.

: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:
Expand All @@ -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
Expand Down
52 changes: 2 additions & 50 deletions sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -115,16 +115,16 @@ 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.

The response is returned in the same order as that of the requests sent.

: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:
Expand All @@ -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__()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -85,14 +85,22 @@ 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
),
]
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
Expand Down
13 changes: 11 additions & 2 deletions sdk/monitor/azure-monitor-query/tests/test_logs_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -92,14 +92,23 @@ 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
),
]
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():
Expand Down