Skip to content

Commit

Permalink
[BUGFIX] - Prevent duplicate Expectations in Validation Results when …
Browse files Browse the repository at this point in the history
…Exceptions are triggered (#9456)
  • Loading branch information
josectobar committed Feb 24, 2024
1 parent 1293ac0 commit 10b6362
Show file tree
Hide file tree
Showing 12 changed files with 297 additions and 185 deletions.
3 changes: 3 additions & 0 deletions great_expectations/core/expectation_validation_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,9 @@ def to_json_dict(self):
)
myself["statistics"] = convert_to_json_serializable(myself["statistics"])
myself["meta"] = convert_to_json_serializable(myself["meta"])
myself["results"] = [
convert_to_json_serializable(result) for result in myself["results"]
]
myself = expectationSuiteValidationResultSchema.dump(myself)
return myself

Expand Down
41 changes: 32 additions & 9 deletions great_expectations/expectations/expectation.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,15 @@ def _diagnostic_status_icon_renderer(
runtime_configuration: Optional[dict] = None,
) -> RenderedStringTemplateContent:
assert result, "Must provide a result object."
if result.exception_info["raised_exception"]:
raised_exception: bool = False
if "raised_exception" in result.exception_info:
raised_exception = result.exception_info["raised_exception"]
else:
for k, v in result.exception_info.items():
# TODO JT: This accounts for a dictionary of type {"metric_id": ExceptionInfo} path defined in
# validator._resolve_suite_level_graph_and_process_metric_evaluation_errors
raised_exception = v["raised_exception"]
if raised_exception:
return RenderedStringTemplateContent(
**{ # type: ignore[arg-type]
"content_block_type": "string_template",
Expand Down Expand Up @@ -717,8 +725,27 @@ def _diagnostic_unexpected_statement_renderer(
assert result, "Must provide a result object."
success: Optional[bool] = result.success
result_dict: dict = result.result

if result.exception_info["raised_exception"]:
exception: dict = {
"raised_exception": False,
"exception_message": "",
"exception_traceback": "",
}
if "raised_exception" in result.exception_info:
exception["raised_exception"] = result.exception_info["raised_exception"]
exception["exception_message"] = result.exception_info["exception_message"]
exception["exception_traceback"] = result.exception_info[
"exception_traceback"
]
else:
for k, v in result.exception_info.items():
# TODO JT: This accounts for a dictionary of type {"metric_id": ExceptionInfo} path defined in
# validator._resolve_suite_level_graph_and_process_metric_evaluation_errors
exception["raised_exception"] = v["raised_exception"]
exception["exception_message"] = v["exception_message"]
exception["exception_traceback"] = v["exception_traceback"]
break

if exception["raised_exception"]:
exception_message_template_str = (
"\n\n$expectation_type raised an exception:\n$exception_message"
)
Expand All @@ -735,9 +762,7 @@ def _diagnostic_unexpected_statement_renderer(
"template": exception_message_template_str,
"params": {
"expectation_type": expectation_type,
"exception_message": result.exception_info[
"exception_message"
],
"exception_message": exception["exception_message"],
},
"tag": "strong",
"styling": {
Expand All @@ -761,9 +786,7 @@ def _diagnostic_unexpected_statement_renderer(
**{ # type: ignore[arg-type]
"content_block_type": "string_template",
"string_template": {
"template": result.exception_info[
"exception_traceback"
],
"template": exception["exception_traceback"],
"tag": "code",
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
TableMetric,
)
from great_expectations.rule_based_profiler.domain_builder import ColumnDomainBuilder
from great_expectations.validator.exception_info import ExceptionInfo
from great_expectations.validator.metric_configuration import MetricConfiguration

if TYPE_CHECKING:
Expand Down Expand Up @@ -409,15 +410,13 @@ def _get_metric_from_computed_metrics(
value = computed_metrics[metric_lookup_key]
elif metric_lookup_key in aborted_metrics:
exception = aborted_metrics[metric_lookup_key]
# Note: Only retrieving the first exception if there are multiple exceptions.
exception_info = exception["exception_info"]
assert isinstance(exception_info, set) # Type narrowing for mypy
first_exception = list(exception_info)[0]
exception_type = "Unknown" # Note: we currently only capture the message and traceback, not the type
exception_message = first_exception.exception_message
metric_exception = MetricException(
type=exception_type, message=exception_message
)
if isinstance(exception_info, ExceptionInfo):
exception_message = exception_info.exception_message
metric_exception = MetricException(
type=exception_type, message=exception_message
)
else:
metric_exception = MetricException(
type="Not found",
Expand Down
30 changes: 21 additions & 9 deletions great_expectations/self_check/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2710,15 +2710,27 @@ def check_json_test_result( # noqa: C901, PLR0912, PLR0915
)

elif key == "traceback_substring":
assert result["exception_info"][
"raised_exception"
], f"{result['exception_info']['raised_exception']}"
assert value in result["exception_info"]["exception_traceback"], (
"expected to find "
+ value
+ " in "
+ result["exception_info"]["exception_traceback"]
)
if "raised_exception" not in result["exception_info"]:
# TODO JT: This accounts for a dictionary of type {"metric_id": ExceptionInfo} path defined in
# validator._resolve_suite_level_graph_and_process_metric_evaluation_errors
for k, v in result["exception_info"].items():
assert v["raised_exception"], f"{v['raised_exception']}"
assert value in v["exception_traceback"], (
"expected to find "
+ value
+ " in "
+ value["exception_traceback"]
)
else:
assert result["exception_info"][
"raised_exception"
], f"{result['exception_info']['raised_exception']}"
assert value in result["exception_info"]["exception_traceback"], (
"expected to find "
+ value
+ " in "
+ result["exception_info"]["exception_traceback"]
)

elif key == "expected_partition":
assert np.allclose(
Expand Down
33 changes: 8 additions & 25 deletions great_expectations/validator/metrics_calculator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import logging
from collections.abc import Hashable
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple, Union
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union

from great_expectations.core._docs_decorators import public_api
from great_expectations.validator.computed_metric import MetricValue
Expand All @@ -24,7 +24,7 @@
_MetricsDict: TypeAlias = Dict[_MetricKey, MetricValue]
_AbortedMetricsInfoDict: TypeAlias = Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
Dict[str, Union[MetricConfiguration, ExceptionInfo, int]],
]


Expand Down Expand Up @@ -218,13 +218,7 @@ def resolve_validation_graph_and_handle_aborted_metrics_info(
runtime_configuration: Optional[dict] = None,
min_graph_edges_pbar_enable: int = 0,
# Set to low number (e.g., 3) to suppress progress bar for small graphs.
) -> Tuple[
_MetricsDict,
Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
],
]:
) -> Tuple[_MetricsDict, _AbortedMetricsInfoDict]:
"""
Args:
graph: "ValidationGraph" object, containing "metric_edge" structures with "MetricConfiguration" objects.
Expand All @@ -233,12 +227,10 @@ def resolve_validation_graph_and_handle_aborted_metrics_info(
Returns:
Dictionary with requested metrics resolved, with unique metric ID as key and computed metric as value.
Dictionary with aborted metrics information, with metric ID as key.
"""
resolved_metrics: _MetricsDict
aborted_metrics_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
]
aborted_metrics_info: _AbortedMetricsInfoDict
(
resolved_metrics,
aborted_metrics_info,
Expand All @@ -264,13 +256,7 @@ def resolve_validation_graph(
runtime_configuration: Optional[dict] = None,
min_graph_edges_pbar_enable: int = 0,
# Set to low number (e.g., 3) to suppress progress bar for small graphs.
) -> Tuple[
_MetricsDict,
Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
],
]:
) -> Tuple[_MetricsDict, _AbortedMetricsInfoDict]:
"""
Calls "ValidationGraph.resolve()" method with supplied arguments.
Expand All @@ -281,13 +267,10 @@ def resolve_validation_graph(
Returns:
Dictionary with requested metrics resolved, with unique metric ID as key and computed metric as value.
Aborted metrics information, with metric ID as key.
Dictionary with aborted metrics information, with metric ID as key.
"""
resolved_metrics: _MetricsDict
aborted_metrics_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
]
aborted_metrics_info: _AbortedMetricsInfoDict
resolved_metrics, aborted_metrics_info = graph.resolve(
runtime_configuration=runtime_configuration,
min_graph_edges_pbar_enable=min_graph_edges_pbar_enable,
Expand Down
67 changes: 26 additions & 41 deletions great_expectations/validator/validation_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
Set,
Tuple,
Union,
cast,
)

from tqdm.auto import tqdm
Expand All @@ -30,7 +29,10 @@
from great_expectations.execution_engine import ExecutionEngine
from great_expectations.expectations.metrics.metric_provider import MetricProvider
from great_expectations.validator.computed_metric import MetricValue
from great_expectations.validator.metrics_calculator import _MetricKey
from great_expectations.validator.metrics_calculator import (
_AbortedMetricsInfoDict,
_MetricKey,
)

__all__ = [
"ExpectationValidationGraph",
Expand Down Expand Up @@ -193,20 +195,11 @@ def resolve(
min_graph_edges_pbar_enable: int = 0,
# Set to low number (e.g., 3) to suppress progress bar for small graphs.
show_progress_bars: bool = True,
) -> Tuple[
Dict[_MetricKey, MetricValue],
Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
],
]:
) -> Tuple[Dict[_MetricKey, MetricValue], _AbortedMetricsInfoDict,]:
resolved_metrics: Dict[_MetricKey, MetricValue] = {}

# updates graph with aborted metrics
aborted_metrics_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
] = self._resolve(
aborted_metrics_info: _AbortedMetricsInfoDict = self._resolve(
metrics=resolved_metrics,
runtime_configuration=runtime_configuration,
min_graph_edges_pbar_enable=min_graph_edges_pbar_enable,
Expand All @@ -221,10 +214,7 @@ def _resolve( # noqa: C901, PLR0912, PLR0915
runtime_configuration: Optional[dict] = None,
min_graph_edges_pbar_enable: int = 0, # Set to low number (e.g., 3) to suppress progress bar for small graphs.
show_progress_bars: bool = True,
) -> Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
]:
) -> Dict[_MetricKey, Dict[str, Union[MetricConfiguration, ExceptionInfo, int]],]:
if metrics is None:
metrics = {}

Expand All @@ -238,12 +228,9 @@ def _resolve( # noqa: C901, PLR0912, PLR0915

failed_metric_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
] = {}
aborted_metrics_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
Dict[str, Union[MetricConfiguration, ExceptionInfo, int]],
] = {}
aborted_metrics_info: _AbortedMetricsInfoDict = {}

ready_metrics: Set[MetricConfiguration]
needed_metrics: Set[MetricConfiguration]
Expand Down Expand Up @@ -301,16 +288,19 @@ def _resolve( # noqa: C901, PLR0912, PLR0915
for failed_metric in err.failed_metrics:
if failed_metric.id in failed_metric_info:
failed_metric_info[failed_metric.id]["num_failures"] += 1 # type: ignore[operator] # Incorrect flagging of 'Unsupported operand types for <= ("int" and "MetricConfiguration") and for >= ("Set[ExceptionInfo]" and "int")' in deep "Union" structure.
failed_metric_info[failed_metric.id]["exception_info"].add(exception_info) # type: ignore[union-attr] # Incorrect flagging of 'Item "MetricConfiguration" of "Union[MetricConfiguration, Set[ExceptionInfo], int]" has no attribute "add" and Item "int" of "Union[MetricConfiguration, Set[ExceptionInfo], int]" has no attribute "add"' in deep "Union" structure.
failed_metric_info[failed_metric.id][
"exception_info"
] = exception_info
else:
failed_metric_info[failed_metric.id] = {}
failed_metric_info[failed_metric.id][
"metric_configuration"
] = failed_metric
failed_metric_info[failed_metric.id]["num_failures"] = 1
failed_metric_info[failed_metric.id]["exception_info"] = {
exception_info
}
failed_metric_info[failed_metric.id][
"exception_info"
] = exception_info

else:
raise err
except Exception as e:
Expand Down Expand Up @@ -411,31 +401,26 @@ def get_exception_info(
self,
metric_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
Dict[str, Union[MetricConfiguration, ExceptionInfo, int]],
],
) -> Set[ExceptionInfo]:
) -> Dict[str, Union[MetricConfiguration, ExceptionInfo, int]]:
metric_info = self._filter_metric_info_in_graph(metric_info=metric_info)
metric_exception_info: Set[ExceptionInfo] = set()
metric_exception_info: Dict[
str, Union[MetricConfiguration, ExceptionInfo, int]
] = {}
metric_id: _MetricKey
metric_info_item: Union[MetricConfiguration, Set[ExceptionInfo], int]
for metric_id, metric_info_item in metric_info.items(): # type: ignore[assignment] # Incorrect flagging of 'Incompatible types in assignment (expression has type "Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]]", variable has type "Union[MetricConfiguration, Set[ExceptionInfo], int]")' in deep "Union" structure.
# noinspection PyUnresolvedReferences
metric_exception_info.update(
cast(Set[ExceptionInfo], metric_info_item["exception_info"]) # type: ignore[index] # Incorrect flagging of 'Value of type "Union[MetricConfiguration, Set[ExceptionInfo], int]" is not indexable' in deep "Union" structure.
)
metric_info_item: Dict[str, Union[MetricConfiguration, ExceptionInfo, int]]
for metric_id, metric_info_item in metric_info.items():
metric_exception_info[str(metric_id)] = metric_info_item["exception_info"]

return metric_exception_info

def _filter_metric_info_in_graph(
self,
metric_info: Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
_MetricKey, Dict[str, Union[MetricConfiguration, ExceptionInfo, int]]
],
) -> Dict[
_MetricKey,
Dict[str, Union[MetricConfiguration, Set[ExceptionInfo], int]],
]:
) -> Dict[_MetricKey, Dict[str, Union[MetricConfiguration, ExceptionInfo, int]]]:
graph_metric_ids: List[_MetricKey] = []
edge: MetricEdge
vertex: MetricConfiguration
Expand Down

0 comments on commit 10b6362

Please sign in to comment.