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

Handle Errors and Tables in Query #20658

Merged
merged 12 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from 8 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
6 changes: 6 additions & 0 deletions sdk/monitor/azure-monitor-query/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@

### Features Added

- Added `QueryPartialErrorException` and `LogsQueryError` to handle errors.
- Added `partial_error` and `is_error` attributes to `LogsQueryResult`.
- Added an option `allow_partial_errors` that defaults to False, which can be set to not throw if there are any partial errors.

### Breaking Changes

- `LogsQueryResult` now iterates over the tables directly as a convinience.
rakshith91 marked this conversation as resolved.
Show resolved Hide resolved

### Bugs Fixed

### Other Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
from ._logs_query_client import LogsQueryClient
from ._metrics_query_client import MetricsQueryClient

from ._exceptions import (
LogsQueryError,
QueryPartialErrorException
)

from ._models import (
MetricAggregationType,
LogsQueryResult,
Expand All @@ -30,6 +35,8 @@
"MetricAggregationType",
"LogsQueryClient",
"LogsQueryResult",
"LogsQueryError",
"QueryPartialErrorException",
"LogsTable",
"LogsBatchQuery",
"MetricsQueryClient",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#
# -------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
# --------------------------------------------------------------------------
from azure.core.exceptions import HttpResponseError

class LogsQueryError(object):
Copy link
Member

Choose a reason for hiding this comment

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

Is it an AzureError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this need not be an azure error.

"""The code and message for an error.

All required parameters must be populated in order to send to Azure.

:ivar code: A machine readable error code.
:vartype code: str
:ivar message: A human readable error message.
:vartype message: str
:ivar details: error details.
:vartype details: list[~monitor_query_client.models.ErrorDetail]
:ivar innererror: Inner error details if they exist.
:vartype innererror: ~azure.monitor.query.LogsQueryError
:ivar additional_properties: Additional properties that can be provided on the error info
object.
:vartype additional_properties: object
:ivar bool is_error: Boolean check for error item when iterating over list of
results. Always True for an instance of a LogsQueryError.
"""
def __init__(
self,
**kwargs
):
self.code = kwargs.get('code', None)
self.message = kwargs.get('message', None)
self.details = kwargs.get('details', None)
self.innererror = kwargs.get('innererror', None)
self.additional_properties = kwargs.get('additional_properties', None)
self.is_error = True

@classmethod
def _from_generated(cls, generated):
if not generated:
return None
details = None
if generated.details is not None:
details = [d.serialize() for d in generated.details]
return cls(
code=generated.code,
message=generated.message,
innererror=cls._from_generated(generated.innererror) if generated.innererror else None,
additional_properties=generated.additional_properties,
details=details,
is_error=True
Copy link
Member

Choose a reason for hiding this comment

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

Do we need it? (is_error=True)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes - we return Union[LogsQueryResult, LogsQueryError] for the batch api - the workflow would be

if result.is_error:
   # handle logsquery erroe
elif not result.is_error:
  # hanfle log query result

Copy link
Member

Choose a reason for hiding this comment

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

If I read correctly, we don't honor "is_error" in ctor.

    def __init__(
        self,
        **kwargs
    ):
        self.code = kwargs.get('code', None)
        self.message = kwargs.get('message', None)
        self.details = kwargs.get('details', None)
        self.innererror = kwargs.get('innererror', None)
        self.additional_properties = kwargs.get('additional_properties', None)
        self.is_error = True

Copy link
Contributor Author

@rakshith91 rakshith91 Sep 13, 2021

Choose a reason for hiding this comment

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

that is correct - this is always True for a LogsQueryError and always False for a LogsQueryResult - it's very similar to what we have in text analytics against DocumentError

Update: removed from the ctor

)

class QueryPartialErrorException(HttpResponseError):
xiangyan99 marked this conversation as resolved.
Show resolved Hide resolved
"""There is a partial failure in query operation. This is thrown for a single query operation
when allow_partial_errors is set to False.

:ivar code: A machine readable error code.
:vartype code: str
:ivar message: A human readable error message.
:vartype message: str
:ivar details: error details.
:vartype details: list[~monitor_query_client.models.ErrorDetail]
:ivar innererror: Inner error details if they exist.
:vartype innererror: ~azure.monitor.query.LogsQueryError
:ivar additional_properties: Additional properties that can be provided on the error info
object.
:vartype additional_properties: object
:ivar bool is_error: Boolean check for error item when iterating over list of
results. Always True for an instance of a LogsQueryError.
"""

def __init__(self, **kwargs):
error = kwargs.pop('error', None)
if error:
self.code = error.code
self.message = error.message
self.details = [d.serialize() for d in error.details] if error.details else None
self.innererror = LogsQueryError._from_generated(error.innererror) if error.innererror else None
self.additional_properties = error.additional_properties
self.is_error = True
super(QueryPartialErrorException, self).__init__(message=self.message)
39 changes: 33 additions & 6 deletions sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,21 @@ def get_metrics_authentication_policy(

raise TypeError("Unsupported credential")

def process_error(exception):
raise_error = HttpResponseError
raise raise_error(message=exception.message, response=exception.response)

def order_results(request_order, mapping, obj):
def order_results(request_order, mapping, obj, err, allow_partial_errors=False):
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to put allow_partial_errors into kwargs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

normally i would say yes - but here, i don't see the kwargs expanding anytime soon - plus this is an internal method, so we can change it if we think it's necessary

ordered = [mapping[id] for id in request_order]
return [obj._from_generated(rsp) for rsp in ordered] # pylint: disable=protected-access
results = []
for item in ordered:
if not item.body.error:
results.append(obj._from_generated(item.body)) # pylint: disable=protected-access
else:
error = item.body.error
if allow_partial_errors and error.code == 'PartialError':
res = obj._from_generated(item.body) # pylint: disable=protected-access
res.partial_error = err._from_generated(error) # pylint: disable=protected-access
results.append(res)
else:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we raise if allow_partial_errors = False and there is error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no - in case of batch, we return the error object instead of the result object

of course, we raise when something fatal happens (like auth error for instance)

results.append(err._from_generated(error)) # pylint: disable=protected-access
return results

def construct_iso8601(timespan=None):
if not timespan:
Expand Down Expand Up @@ -90,3 +98,22 @@ def native_col_type(col_type, value):

def process_row(col_types, row):
return [native_col_type(col_types[ind], val) for ind, val in enumerate(row)]

def process_error(error):
Copy link
Member

Choose a reason for hiding this comment

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

Here, error is an HttpResponseError, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes that is correct

Copy link
Member

Choose a reason for hiding this comment

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

Could you help me understand what is this method for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, made some small changes to this - missed a commit earlier
this is to convert the model into the logsqueryerror intead of generated internal model.

if not error:
return None
raise HttpResponseError(message=error.message, response=error.response)

def process_prefer(server_timeout, include_statistics, include_visualization):
prefer = ""
if server_timeout:
prefer += "wait=" + str(server_timeout)
if include_statistics:
if len(prefer) > 0:
prefer += ","
prefer += "include-statistics=true"
if include_visualization:
if len(prefer) > 0:
prefer += ","
prefer += "include-render=true"
return prefer
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
from ._generated._monitor_query_client import MonitorQueryClient

from ._generated.models import BatchRequest, QueryBody as LogsQueryBody
from ._helpers import get_authentication_policy, process_error, construct_iso8601, order_results
from ._helpers import get_authentication_policy, construct_iso8601, order_results, process_error, process_prefer
from ._models import LogsBatchQuery, LogsQueryResult
from ._exceptions import LogsQueryError, QueryPartialErrorException

if TYPE_CHECKING:
from azure.core.credentials import TokenCredential
Expand Down Expand Up @@ -76,6 +77,8 @@ def query(self, workspace_id, query, **kwargs):
:keyword additional_workspaces: A list of workspaces that are included in the query.
These can be qualified workspace names, workspace Ids, or Azure resource Ids.
:paramtype additional_workspaces: list[str]
:keyword allow_partial_errors: Defaults to False. If set to true, partial errors are not thrown.
:paramtype allow_partial_errors: bool
:return: LogsQueryResult, or the result of cls(response)
:rtype: ~azure.monitor.query.LogsQueryResult
:raises: ~azure.core.exceptions.HttpResponseError
Expand All @@ -89,6 +92,7 @@ def query(self, workspace_id, query, **kwargs):
:dedent: 0
:caption: Get a response for a single Log Query
"""
allow_partial_errors = kwargs.pop('allow_partial_errors', False)
if 'timespan' not in kwargs:
raise TypeError("query() missing 1 required keyword-only argument: 'timespan'")
timespan = construct_iso8601(kwargs.pop('timespan'))
Expand All @@ -108,6 +112,7 @@ def query(self, workspace_id, query, **kwargs):
if len(prefer) > 0:
prefer += ","
prefer += "include-render=true"
prefer = process_prefer(server_timeout, include_statistics, include_visualization)

body = LogsQueryBody(
query=query,
Expand All @@ -117,14 +122,23 @@ def query(self, workspace_id, query, **kwargs):
)

try:
return LogsQueryResult._from_generated(self._query_op.execute( # pylint: disable=protected-access
generated_response = self._query_op.execute( # pylint: disable=protected-access
workspace_id=workspace_id,
body=body,
prefer=prefer,
**kwargs
))
except HttpResponseError as e:
process_error(e)
)
except HttpResponseError as err:
process_error(err)
response = LogsQueryResult._from_generated(generated_response) # pylint: disable=protected-access
if not generated_response.error:
return response
if not allow_partial_errors:
raise QueryPartialErrorException(error=generated_response.error)
response.partial_error = LogsQueryError._from_generated( # pylint: disable=protected-access
generated_response.error
)
return response

@distributed_trace
def query_batch(self, queries, **kwargs):
Expand All @@ -136,6 +150,9 @@ def query_batch(self, queries, **kwargs):

:param queries: The list of Kusto queries to execute.
:type queries: list[dict] or list[~azure.monitor.query.LogsBatchQuery]
:keyword bool allow_partial_errors: If set to True, a `LogsQueryResult` object is returned
when a partial error occurs. The error can be accessed using the `partial_error`
attribute in the object.
:return: List of LogsQueryResult, or the result of cls(response)
:rtype: list[~azure.monitor.query.LogsQueryResult]
:raises: ~azure.core.exceptions.HttpResponseError
Expand All @@ -149,6 +166,7 @@ def query_batch(self, queries, **kwargs):
:dedent: 0
:caption: Get a response for multiple Log Queries.
"""
allow_partial_errors = kwargs.pop('allow_partial_errors', False)
try:
queries = [LogsBatchQuery(**q) for q in queries]
except (KeyError, TypeError):
Expand All @@ -161,7 +179,12 @@ def query_batch(self, queries, **kwargs):
batch = BatchRequest(requests=queries)
generated = self._query_op.batch(batch, **kwargs)
mapping = {item.id: item for item in generated.responses}
return order_results(request_order, mapping, LogsQueryResult)
return order_results(
request_order,
mapping,
LogsQueryResult,
LogsQueryError,
allow_partial_errors)

def close(self):
# type: () -> None
Expand Down
16 changes: 11 additions & 5 deletions sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,21 +165,27 @@ class LogsQueryResult(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
:ivar partial_error: Any error info. This is none except in the case where `allow_partial_errors`
is explicitly set to True.
:vartype partial_error: ~azure.core.exceptions.HttpResponseError
:ivar bool is_error: Boolean check for error item when iterating over list of
results. Always False for an instance of a LogsQueryResult.
"""
def __init__(
self,
**kwargs
):
self.tables = kwargs.get('tables', None)
self.error = kwargs.get('error', None)
self.partial_error = None
self.statistics = kwargs.get('statistics', None)
self.visualization = kwargs.get('visualization', None)
self.is_error = False

def __iter__(self):
return iter(self.tables)

@classmethod
def _from_generated(cls, generated):

if not generated:
return cls()
tables = None
Expand All @@ -195,7 +201,7 @@ def _from_generated(cls, generated):
tables=tables,
statistics=generated.statistics,
visualization=generated.render,
error=generated.error
is_error=False
)


Expand Down
Loading