From 3baf371735a6f1925b42cb73f58fd50d7df0445e Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Wed, 23 Oct 2024 17:26:44 -0700 Subject: [PATCH 1/7] Validate time spines in saved query where filters Validations for time spines in WHERE filters in Saved Queries. This mimics the where filter time spine validation for metrics and applies it to saved queries. This also bumps the version. (I assume this is necessary?) --- .../validations/saved_query.py | 41 +++++++++++++++++- tests/validations/test_saved_query.py | 43 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/dbt_semantic_interfaces/validations/saved_query.py b/dbt_semantic_interfaces/validations/saved_query.py index 7d3716bb..fe23c68d 100644 --- a/dbt_semantic_interfaces/validations/saved_query.py +++ b/dbt_semantic_interfaces/validations/saved_query.py @@ -23,6 +23,7 @@ ) from dbt_semantic_interfaces.protocols import SemanticManifestT from dbt_semantic_interfaces.protocols.saved_query import SavedQuery +from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity from dbt_semantic_interfaces.validations.validator_helpers import ( FileContext, SavedQueryContext, @@ -30,6 +31,7 @@ SemanticManifestValidationRule, ValidationError, ValidationIssue, + ValidationWarning, generate_exception_issue, validate_safely, ) @@ -114,7 +116,7 @@ def _check_metrics(valid_metric_names: Set[str], saved_query: SavedQuery) -> Seq @staticmethod @validate_safely("Validate the where field in a saved query.") - def _check_where(saved_query: SavedQuery) -> Sequence[ValidationIssue]: + def _check_where(saved_query: SavedQuery, custom_granularity_names: list[str]) -> Sequence[ValidationIssue]: issues: List[ValidationIssue] = [] if saved_query.query_params.where is None: return issues @@ -136,9 +138,39 @@ def _check_where(saved_query: SavedQuery) -> Sequence[ValidationIssue]: }, ) ) + else: + issues += SavedQueryRule._check_where_timespine(saved_query, custom_granularity_names) return issues + def _check_where_timespine( + saved_query: SavedQuery, custom_granularity_names: list[str] + ) -> Sequence[ValidationIssue]: + issues: List[ValidationIssue] = [] + + valid_granularity_names = [ + standard_granularity.name for standard_granularity in TimeGranularity + ] + custom_granularity_names + for where_filter in saved_query.query_params.where.where_filters: + for time_dim_call_parameter_set in where_filter.call_parameter_sets.time_dimension_call_parameter_sets: + if not time_dim_call_parameter_set.time_granularity_name: + continue + if time_dim_call_parameter_set.time_granularity_name not in valid_granularity_names: + issues.append( + ValidationWarning( + context=SavedQueryContext( + file_context=FileContext.from_metadata(metadata=saved_query.metadata), + element_type=SavedQueryElementType.WHERE, + element_value=where_filter.where_sql_template, + ), + # message=f"Filter for metric `{context.metric.metric_name}` is not valid. " + message=f"Filter for saved query `{saved_query.name}` is not valid. " + f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. " + f"Valid granularity options: {valid_granularity_names}", + ) + ) + return issues + @staticmethod def _parse_query_item( saved_query: SavedQuery, @@ -280,6 +312,11 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati for entity in semantic_model.entities: valid_group_by_element_names.add(entity.name) + custom_granularity_names = [ + granularity.name + for time_spine in semantic_manifest.project_configuration.time_spines + for granularity in time_spine.custom_granularities + ] for saved_query in semantic_manifest.saved_queries: issues += SavedQueryRule._check_metrics( valid_metric_names=valid_metric_names, @@ -289,7 +326,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati valid_group_by_element_names=valid_group_by_element_names, saved_query=saved_query, ) - issues += SavedQueryRule._check_where(saved_query) + issues += SavedQueryRule._check_where(saved_query, custom_granularity_names) issues += SavedQueryRule._check_order_by(saved_query) issues += SavedQueryRule._check_limit(saved_query) return issues diff --git a/tests/validations/test_saved_query.py b/tests/validations/test_saved_query.py index e07288e5..56f50dea 100644 --- a/tests/validations/test_saved_query.py +++ b/tests/validations/test_saved_query.py @@ -38,6 +38,21 @@ def check_only_one_error_with_message( # noqa: D } and found_match +def check_only_one_warning_with_message( # noqa: D + results: SemanticManifestValidationResults, target_message: str +) -> None: + assert len(results.warnings) == 1 + assert len(results.errors) == 0 + assert len(results.future_errors) == 0 + + found_match = results.warnings[0].message.find(target_message) != -1 + # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. + assert { + "expected": target_message, + "actual": results.warnings[0].message, + } and found_match + + def test_invalid_metric_in_saved_query( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: @@ -87,6 +102,34 @@ def test_invalid_where_in_saved_query( # noqa: D ) +def test_where_filter_validations_invalid_granularity( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) + check_only_one_warning_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "is not a valid granularity name", + ) + + def test_invalid_group_by_element_in_saved_query( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: From fe81c4212a7f93d3e131f1ff570db3d05accb599 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Wed, 23 Oct 2024 17:43:51 -0700 Subject: [PATCH 2/7] fix lint --- dbt_semantic_interfaces/validations/saved_query.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dbt_semantic_interfaces/validations/saved_query.py b/dbt_semantic_interfaces/validations/saved_query.py index fe23c68d..e062b39f 100644 --- a/dbt_semantic_interfaces/validations/saved_query.py +++ b/dbt_semantic_interfaces/validations/saved_query.py @@ -146,12 +146,17 @@ def _check_where(saved_query: SavedQuery, custom_granularity_names: list[str]) - def _check_where_timespine( saved_query: SavedQuery, custom_granularity_names: list[str] ) -> Sequence[ValidationIssue]: + where_param = saved_query.query_params.where + if where_param is None: + return [] + issues: List[ValidationIssue] = [] valid_granularity_names = [ standard_granularity.name for standard_granularity in TimeGranularity ] + custom_granularity_names - for where_filter in saved_query.query_params.where.where_filters: + + for where_filter in where_param.where_filters: for time_dim_call_parameter_set in where_filter.call_parameter_sets.time_dimension_call_parameter_sets: if not time_dim_call_parameter_set.time_granularity_name: continue From c5fca352d7a46239624f935ebde1d9a156dd4260 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Wed, 23 Oct 2024 18:04:57 -0700 Subject: [PATCH 3/7] Added changie file --- .changes/unreleased/Under the Hood-20241023-180425.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Under the Hood-20241023-180425.yaml diff --git a/.changes/unreleased/Under the Hood-20241023-180425.yaml b/.changes/unreleased/Under the Hood-20241023-180425.yaml new file mode 100644 index 00000000..33ebed54 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20241023-180425.yaml @@ -0,0 +1,6 @@ +kind: Under the Hood +body: Added validation warnings for invalid time spines in where filters of saved queries. +time: 2024-10-23T18:04:25.235887-07:00 +custom: + Author: theyostalservice + Issue: "360" From ad329c5e862c934d4259a417827e7a536f73daf8 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Sun, 27 Oct 2024 17:44:57 -0700 Subject: [PATCH 4/7] Centralize Where filter validation + comments This is not *exactly* what was described in Courtney's comments, but I think there was a little roughness in that plan once I began implementing (but I'm happy to change course if reviewers disagree!). This commit promotes the `WhereFiltersAreParseable` to be `WhereFiltersAreParseableRule` (a free-standing 'rule' in a separate file). It was weird to be passing things in from other classes but somehow centralizing the manifest, so instead, I just moved ALL of the relevant checks here. This moves the tests for where filters to a new file specifically for this rule (again, I'm open to the idea that this would be better just divided amongst the existing tests, but they share so much conceptually that it seems nice to group them together and to have a test that is pointed 1:1 at a rule where possible). Finally, I also moved some of the test support functions (`check_only_one_error_with_message`, for example) to `dbt_semantic_interfaces.test_utils` because they seem useful and reusable. --- .../Under the Hood-20241023-180425.yaml | 2 +- dbt_semantic_interfaces/test_utils.py | 39 +++ .../validations/metrics.py | 175 +--------- .../validations/saved_query.py | 70 ---- .../semantic_manifest_validator.py | 6 +- .../validations/where_filters.py | 268 +++++++++++++++ tests/validations/test_metrics.py | 142 -------- tests/validations/test_saved_query.py | 111 +------ .../test_where_filters_are_parseable.py | 309 ++++++++++++++++++ 9 files changed, 623 insertions(+), 499 deletions(-) create mode 100644 dbt_semantic_interfaces/validations/where_filters.py create mode 100644 tests/validations/test_where_filters_are_parseable.py diff --git a/.changes/unreleased/Under the Hood-20241023-180425.yaml b/.changes/unreleased/Under the Hood-20241023-180425.yaml index 33ebed54..25b933b8 100644 --- a/.changes/unreleased/Under the Hood-20241023-180425.yaml +++ b/.changes/unreleased/Under the Hood-20241023-180425.yaml @@ -1,5 +1,5 @@ kind: Under the Hood -body: Added validation warnings for invalid time spines in where filters of saved queries. +body: Added validation warnings for invalid granularity names in where filters of saved queries. time: 2024-10-23T18:04:25.235887-07:00 custom: Author: theyostalservice diff --git a/dbt_semantic_interfaces/test_utils.py b/dbt_semantic_interfaces/test_utils.py index ced0cd21..88cdd625 100644 --- a/dbt_semantic_interfaces/test_utils.py +++ b/dbt_semantic_interfaces/test_utils.py @@ -25,6 +25,9 @@ ) from dbt_semantic_interfaces.parsing.objects import YamlConfigFile from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity +from dbt_semantic_interfaces.validations.validator_helpers import ( + SemanticManifestValidationResults, +) logger = logging.getLogger(__name__) @@ -169,3 +172,39 @@ def semantic_model_with_guaranteed_meta( dimensions=dimensions, metadata=metadata, ) + + +def check_only_one_error_with_message( # noqa: D + results: SemanticManifestValidationResults, target_message: str +) -> None: + assert len(results.warnings) == 0 + assert len(results.errors) == 1 + assert len(results.future_errors) == 0 + + found_match = results.errors[0].message.find(target_message) != -1 + # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. + assert { + "expected": target_message, + "actual": results.errors[0].message, + } and found_match + + +def check_only_one_warning_with_message( # noqa: D + results: SemanticManifestValidationResults, target_message: str +) -> None: + assert len(results.errors) == 0 + assert len(results.warnings) == 1 + assert len(results.future_errors) == 0 + + found_match = results.warnings[0].message.find(target_message) != -1 + # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. + assert { + "expected": target_message, + "actual": results.warnings[0].message, + } and found_match + + +def check_no_errors_or_warnings(results: SemanticManifestValidationResults) -> None: # noqa: D + assert len(results.errors) == 0 + assert len(results.warnings) == 0 + assert len(results.future_errors) == 0 diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index f0eb0dff..da9153a8 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/dbt_semantic_interfaces/validations/metrics.py @@ -1,7 +1,6 @@ import traceback -from typing import Dict, Generic, List, Optional, Sequence, Tuple +from typing import Dict, Generic, List, Optional, Sequence -from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets from dbt_semantic_interfaces.errors import ParsingException from dbt_semantic_interfaces.implementations.metric import ( PydanticMetric, @@ -35,7 +34,6 @@ ValidationError, ValidationIssue, ValidationWarning, - generate_exception_issue, validate_safely, ) @@ -244,177 +242,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati return issues -class WhereFiltersAreParseable(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): - """Validates that all Metric WhereFilters are parseable.""" - - @staticmethod - def _validate_time_granularity_names( - context: MetricContext, - filter_expression_parameter_sets: Sequence[Tuple[str, FilterCallParameterSets]], - custom_granularity_names: List[str], - ) -> Sequence[ValidationIssue]: - issues: List[ValidationIssue] = [] - - valid_granularity_names = [ - standard_granularity.value for standard_granularity in TimeGranularity - ] + custom_granularity_names - for _, parameter_set in filter_expression_parameter_sets: - for time_dim_call_parameter_set in parameter_set.time_dimension_call_parameter_sets: - if not time_dim_call_parameter_set.time_granularity_name: - continue - if time_dim_call_parameter_set.time_granularity_name.lower() not in valid_granularity_names: - issues.append( - ValidationWarning( - context=context, - message=f"Filter for metric `{context.metric.metric_name}` is not valid. " - f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. " - f"Valid granularity options: {valid_granularity_names}", - ) - ) - return issues - - @staticmethod - @validate_safely( - whats_being_done="running model validation ensuring a metric's filter properties are configured properly" - ) - def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Sequence[ValidationIssue]: # noqa: D - issues: List[ValidationIssue] = [] - context = MetricContext( - file_context=FileContext.from_metadata(metadata=metric.metadata), - metric=MetricModelReference(metric_name=metric.name), - ) - - if metric.filter is not None: - try: - metric.filter.filter_expression_parameter_sets - except Exception as e: - issues.append( - generate_exception_issue( - what_was_being_done=f"trying to parse filter of metric `{metric.name}`", - e=e, - context=context, - extras={ - "traceback": "".join(traceback.format_tb(e.__traceback__)), - }, - ) - ) - else: - issues += WhereFiltersAreParseable._validate_time_granularity_names( - context=context, - filter_expression_parameter_sets=metric.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, - ) - - if metric.type_params: - measure = metric.type_params.measure - if measure is not None and measure.filter is not None: - try: - measure.filter.filter_expression_parameter_sets - except Exception as e: - issues.append( - generate_exception_issue( - what_was_being_done=f"trying to parse filter of measure input `{measure.name}` " - f"on metric `{metric.name}`", - e=e, - context=context, - extras={ - "traceback": "".join(traceback.format_tb(e.__traceback__)), - }, - ) - ) - else: - issues += WhereFiltersAreParseable._validate_time_granularity_names( - context=context, - filter_expression_parameter_sets=measure.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, - ) - - numerator = metric.type_params.numerator - if numerator is not None and numerator.filter is not None: - try: - numerator.filter.filter_expression_parameter_sets - except Exception as e: - issues.append( - generate_exception_issue( - what_was_being_done=f"trying to parse the numerator filter on metric `{metric.name}`", - e=e, - context=context, - extras={ - "traceback": "".join(traceback.format_tb(e.__traceback__)), - }, - ) - ) - else: - issues += WhereFiltersAreParseable._validate_time_granularity_names( - context=context, - filter_expression_parameter_sets=numerator.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, - ) - - denominator = metric.type_params.denominator - if denominator is not None and denominator.filter is not None: - try: - denominator.filter.filter_expression_parameter_sets - except Exception as e: - issues.append( - generate_exception_issue( - what_was_being_done=f"trying to parse the denominator filter on metric `{metric.name}`", - e=e, - context=context, - extras={ - "traceback": "".join(traceback.format_tb(e.__traceback__)), - }, - ) - ) - else: - issues += WhereFiltersAreParseable._validate_time_granularity_names( - context=context, - filter_expression_parameter_sets=denominator.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, - ) - - for input_metric in metric.type_params.metrics or []: - if input_metric.filter is not None: - try: - input_metric.filter.filter_expression_parameter_sets - except Exception as e: - issues.append( - generate_exception_issue( - what_was_being_done=f"trying to parse filter for input metric `{input_metric.name}` " - f"on metric `{metric.name}`", - e=e, - context=context, - extras={ - "traceback": "".join(traceback.format_tb(e.__traceback__)), - }, - ) - ) - else: - issues += WhereFiltersAreParseable._validate_time_granularity_names( - context=context, - filter_expression_parameter_sets=input_metric.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, - ) - - # TODO: Are saved query filters being validated? Task: SL-2932 - return issues - - @staticmethod - @validate_safely(whats_being_done="running manifest validation ensuring all metric where filters are parseable") - def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D - issues: List[ValidationIssue] = [] - custom_granularity_names = [ - granularity.name - for time_spine in semantic_manifest.project_configuration.time_spines - for granularity in time_spine.custom_granularities - ] - for metric in semantic_manifest.metrics or []: - issues += WhereFiltersAreParseable._validate_metric( - metric=metric, custom_granularity_names=custom_granularity_names - ) - return issues - - class ConversionMetricRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): """Checks that conversion metrics are configured properly.""" diff --git a/dbt_semantic_interfaces/validations/saved_query.py b/dbt_semantic_interfaces/validations/saved_query.py index e062b39f..c805aa8f 100644 --- a/dbt_semantic_interfaces/validations/saved_query.py +++ b/dbt_semantic_interfaces/validations/saved_query.py @@ -23,7 +23,6 @@ ) from dbt_semantic_interfaces.protocols import SemanticManifestT from dbt_semantic_interfaces.protocols.saved_query import SavedQuery -from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity from dbt_semantic_interfaces.validations.validator_helpers import ( FileContext, SavedQueryContext, @@ -31,7 +30,6 @@ SemanticManifestValidationRule, ValidationError, ValidationIssue, - ValidationWarning, generate_exception_issue, validate_safely, ) @@ -114,68 +112,6 @@ def _check_metrics(valid_metric_names: Set[str], saved_query: SavedQuery) -> Seq ) return issues - @staticmethod - @validate_safely("Validate the where field in a saved query.") - def _check_where(saved_query: SavedQuery, custom_granularity_names: list[str]) -> Sequence[ValidationIssue]: - issues: List[ValidationIssue] = [] - if saved_query.query_params.where is None: - return issues - for where_filter in saved_query.query_params.where.where_filters: - try: - where_filter.call_parameter_sets - except Exception as e: - issues.append( - generate_exception_issue( - what_was_being_done=f"trying to parse a filter in saved query `{saved_query.name}`", - e=e, - context=SavedQueryContext( - file_context=FileContext.from_metadata(metadata=saved_query.metadata), - element_type=SavedQueryElementType.WHERE, - element_value=where_filter.where_sql_template, - ), - extras={ - "traceback": "".join(traceback.format_tb(e.__traceback__)), - }, - ) - ) - else: - issues += SavedQueryRule._check_where_timespine(saved_query, custom_granularity_names) - - return issues - - def _check_where_timespine( - saved_query: SavedQuery, custom_granularity_names: list[str] - ) -> Sequence[ValidationIssue]: - where_param = saved_query.query_params.where - if where_param is None: - return [] - - issues: List[ValidationIssue] = [] - - valid_granularity_names = [ - standard_granularity.name for standard_granularity in TimeGranularity - ] + custom_granularity_names - - for where_filter in where_param.where_filters: - for time_dim_call_parameter_set in where_filter.call_parameter_sets.time_dimension_call_parameter_sets: - if not time_dim_call_parameter_set.time_granularity_name: - continue - if time_dim_call_parameter_set.time_granularity_name not in valid_granularity_names: - issues.append( - ValidationWarning( - context=SavedQueryContext( - file_context=FileContext.from_metadata(metadata=saved_query.metadata), - element_type=SavedQueryElementType.WHERE, - element_value=where_filter.where_sql_template, - ), - # message=f"Filter for metric `{context.metric.metric_name}` is not valid. " - message=f"Filter for saved query `{saved_query.name}` is not valid. " - f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. " - f"Valid granularity options: {valid_granularity_names}", - ) - ) - return issues - @staticmethod def _parse_query_item( saved_query: SavedQuery, @@ -317,11 +253,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati for entity in semantic_model.entities: valid_group_by_element_names.add(entity.name) - custom_granularity_names = [ - granularity.name - for time_spine in semantic_manifest.project_configuration.time_spines - for granularity in time_spine.custom_granularities - ] for saved_query in semantic_manifest.saved_queries: issues += SavedQueryRule._check_metrics( valid_metric_names=valid_metric_names, @@ -331,7 +262,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati valid_group_by_element_names=valid_group_by_element_names, saved_query=saved_query, ) - issues += SavedQueryRule._check_where(saved_query, custom_granularity_names) issues += SavedQueryRule._check_order_by(saved_query) issues += SavedQueryRule._check_limit(saved_query) return issues diff --git a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py index 26f81452..cfad51ed 100644 --- a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py +++ b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py @@ -27,7 +27,6 @@ ConversionMetricRule, CumulativeMetricRule, DerivedMetricRule, - WhereFiltersAreParseable, ) from dbt_semantic_interfaces.validations.non_empty import NonEmptyRule from dbt_semantic_interfaces.validations.primary_entity import PrimaryEntityRule @@ -47,6 +46,9 @@ SemanticManifestValidationResults, SemanticManifestValidationRule, ) +from dbt_semantic_interfaces.validations.where_filters import ( + WhereFiltersAreParseableRule, +) logger = logging.getLogger(__name__) @@ -86,7 +88,7 @@ class SemanticManifestValidator(Generic[SemanticManifestT]): SemanticModelDefaultsRule[SemanticManifestT](), PrimaryEntityRule[SemanticManifestT](), PrimaryEntityDimensionPairs[SemanticManifestT](), - WhereFiltersAreParseable[SemanticManifestT](), + WhereFiltersAreParseableRule[SemanticManifestT](), SavedQueryRule[SemanticManifestT](), MetricLabelsRule[SemanticManifestT](), SemanticModelLabelsRule[SemanticManifestT](), diff --git a/dbt_semantic_interfaces/validations/where_filters.py b/dbt_semantic_interfaces/validations/where_filters.py new file mode 100644 index 00000000..333be457 --- /dev/null +++ b/dbt_semantic_interfaces/validations/where_filters.py @@ -0,0 +1,268 @@ +import traceback +from typing import Generic, List, Sequence, Tuple + +from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets +from dbt_semantic_interfaces.protocols import Metric, SemanticManifestT +from dbt_semantic_interfaces.protocols.saved_query import SavedQuery +from dbt_semantic_interfaces.references import MetricModelReference +from dbt_semantic_interfaces.type_enums import TimeGranularity +from dbt_semantic_interfaces.validations.validator_helpers import ( + FileContext, + MetricContext, + SavedQueryContext, + SavedQueryElementType, + SemanticManifestValidationRule, + ValidationContext, + ValidationIssue, + ValidationWarning, + generate_exception_issue, + validate_safely, +) + + +class WhereFiltersAreParseableRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): + """Validates that all Metric WhereFilters are parseable.""" + + @staticmethod + def _validate_time_granularity_names_impl( + location_label_for_errors: str, + filter_call_parameter_sets_with_context: Sequence[Tuple[ValidationContext, FilterCallParameterSets]], + custom_granularity_names: List[str], + ) -> Sequence[ValidationIssue]: + issues: List[ValidationIssue] = [] + + valid_granularity_names = [ + standard_granularity.value for standard_granularity in TimeGranularity + ] + custom_granularity_names + + for context, parameter_set in filter_call_parameter_sets_with_context: + for time_dim_call_parameter_set in parameter_set.time_dimension_call_parameter_sets: + if not time_dim_call_parameter_set.time_granularity_name: + continue + if time_dim_call_parameter_set.time_granularity_name.lower() not in valid_granularity_names: + issues.append( + ValidationWarning( + context=context, + message=f"Filter for `{location_label_for_errors}` is not valid. " + f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. " + f"Valid granularity options: {valid_granularity_names}", + ) + ) + return issues + + @staticmethod + def _validate_time_granularity_names_for_saved_query( + saved_query: SavedQuery, custom_granularity_names: List[str] + ) -> Sequence[ValidationIssue]: + where_param = saved_query.query_params.where + if where_param is None: + return [] + + return WhereFiltersAreParseableRule._validate_time_granularity_names_impl( + location_label_for_errors="saved query `{saved_query.name}`", + filter_call_parameter_sets_with_context=[ + ( + SavedQueryContext( + file_context=FileContext.from_metadata(metadata=saved_query.metadata), + element_type=SavedQueryElementType.WHERE, + element_value=where_filter.where_sql_template, + ), + where_filter.call_parameter_sets, + ) + for where_filter in where_param.where_filters + ], + custom_granularity_names=custom_granularity_names, + ) + + @staticmethod + def _validate_time_granularity_names_for_metric( + context: MetricContext, + filter_expression_parameter_sets: Sequence[Tuple[str, FilterCallParameterSets]], + custom_granularity_names: List[str], + ) -> Sequence[ValidationIssue]: + return WhereFiltersAreParseableRule._validate_time_granularity_names_impl( + location_label_for_errors="metric `{context.metric.metric_name}`", + filter_call_parameter_sets_with_context=[ + ( + context, + param_set[1], + ) + for param_set in filter_expression_parameter_sets + ], + custom_granularity_names=custom_granularity_names, + ) + + @staticmethod + @validate_safely("validating the where field in a saved query.") + def _validate_saved_query( + saved_query: SavedQuery, custom_granularity_names: List[str] + ) -> Sequence[ValidationIssue]: + issues: List[ValidationIssue] = [] + if saved_query.query_params.where is None: + return issues + for where_filter in saved_query.query_params.where.where_filters: + try: + where_filter.call_parameter_sets + except Exception as e: + issues.append( + generate_exception_issue( + what_was_being_done=f"trying to parse a filter in saved query `{saved_query.name}`", + e=e, + context=SavedQueryContext( + file_context=FileContext.from_metadata(metadata=saved_query.metadata), + element_type=SavedQueryElementType.WHERE, + element_value=where_filter.where_sql_template, + ), + extras={ + "traceback": "".join(traceback.format_tb(e.__traceback__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_saved_query( + saved_query, custom_granularity_names + ) + + return issues + + @staticmethod + @validate_safely( + whats_being_done="running model validation ensuring a metric's filter properties are configured properly" + ) + def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Sequence[ValidationIssue]: # noqa: D + issues: List[ValidationIssue] = [] + context = MetricContext( + file_context=FileContext.from_metadata(metadata=metric.metadata), + metric=MetricModelReference(metric_name=metric.name), + ) + + if metric.filter is not None: + try: + metric.filter.filter_expression_parameter_sets + except Exception as e: + issues.append( + generate_exception_issue( + what_was_being_done=f"trying to parse filter of metric `{metric.name}`", + e=e, + context=context, + extras={ + "traceback": "".join(traceback.format_tb(e.__traceback__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + context=context, + filter_expression_parameter_sets=metric.filter.filter_expression_parameter_sets, + custom_granularity_names=custom_granularity_names, + ) + + if metric.type_params: + measure = metric.type_params.measure + if measure is not None and measure.filter is not None: + try: + measure.filter.filter_expression_parameter_sets + except Exception as e: + issues.append( + generate_exception_issue( + what_was_being_done=f"trying to parse filter of measure input `{measure.name}` " + f"on metric `{metric.name}`", + e=e, + context=context, + extras={ + "traceback": "".join(traceback.format_tb(e.__traceback__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + context=context, + filter_expression_parameter_sets=measure.filter.filter_expression_parameter_sets, + custom_granularity_names=custom_granularity_names, + ) + + numerator = metric.type_params.numerator + if numerator is not None and numerator.filter is not None: + try: + numerator.filter.filter_expression_parameter_sets + except Exception as e: + issues.append( + generate_exception_issue( + what_was_being_done=f"trying to parse the numerator filter on metric `{metric.name}`", + e=e, + context=context, + extras={ + "traceback": "".join(traceback.format_tb(e.__traceback__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + context=context, + filter_expression_parameter_sets=numerator.filter.filter_expression_parameter_sets, + custom_granularity_names=custom_granularity_names, + ) + + denominator = metric.type_params.denominator + if denominator is not None and denominator.filter is not None: + try: + denominator.filter.filter_expression_parameter_sets + except Exception as e: + issues.append( + generate_exception_issue( + what_was_being_done=f"trying to parse the denominator filter on metric `{metric.name}`", + e=e, + context=context, + extras={ + "traceback": "".join(traceback.format_tb(e.__traceback__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + context=context, + filter_expression_parameter_sets=denominator.filter.filter_expression_parameter_sets, + custom_granularity_names=custom_granularity_names, + ) + + for input_metric in metric.type_params.metrics or []: + if input_metric.filter is not None: + try: + input_metric.filter.filter_expression_parameter_sets + except Exception as e: + issues.append( + generate_exception_issue( + what_was_being_done=f"trying to parse filter for input metric `{input_metric.name}` " + f"on metric `{metric.name}`", + e=e, + context=context, + extras={ + "traceback": "".join(traceback.format_tb(e.__traceback__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + context=context, + filter_expression_parameter_sets=input_metric.filter.filter_expression_parameter_sets, + custom_granularity_names=custom_granularity_names, + ) + return issues + + @staticmethod + @validate_safely(whats_being_done="running manifest validation ensuring all metric where filters are parseable") + def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D + issues: List[ValidationIssue] = [] + custom_granularity_names = [ + granularity.name + for time_spine in semantic_manifest.project_configuration.time_spines + for granularity in time_spine.custom_granularities + ] + for metric in semantic_manifest.metrics or []: + issues += WhereFiltersAreParseableRule._validate_metric( + metric=metric, custom_granularity_names=custom_granularity_names + ) + for saved_query in semantic_manifest.saved_queries: + issues += WhereFiltersAreParseableRule._validate_saved_query(saved_query, custom_granularity_names) + + return issues diff --git a/tests/validations/test_metrics.py b/tests/validations/test_metrics.py index efafeef0..c96d23b3 100644 --- a/tests/validations/test_metrics.py +++ b/tests/validations/test_metrics.py @@ -1,4 +1,3 @@ -from copy import deepcopy from typing import List, Tuple import pytest @@ -31,7 +30,6 @@ TimeDimensionReference, ) from dbt_semantic_interfaces.test_utils import ( - find_metric_with, metric_with_guaranteed_meta, semantic_model_with_guaranteed_meta, ) @@ -48,7 +46,6 @@ CumulativeMetricRule, DerivedMetricRule, MetricTimeGranularityRule, - WhereFiltersAreParseable, ) from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( SemanticManifestValidator, @@ -345,145 +342,6 @@ def test_derived_metric() -> None: # noqa: D check_error_in_issues(error_substrings=expected_substrings, issues=build_issues) -def test_where_filter_validations_happy( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - results = validator.validate_semantic_manifest(simple_semantic_manifest__with_primary_transforms) - assert not results.has_blocking_issues - - -def test_where_filter_validations_bad_base_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with(manifest, lambda metric: metric.filter is not None) - assert metric.filter is not None - assert len(metric.filter.where_filters) > 0 - metric.filter.where_filters[0].where_sql_template = "{{ dimension('too', 'many', 'variables', 'to', 'handle') }}" - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises(SemanticManifestValidationException, match=f"trying to parse filter of metric `{metric.name}`"): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_measure_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, lambda metric: metric.type_params is not None and metric.type_params.measure is not None - ) - assert metric.type_params.measure is not None - metric.type_params.measure.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, - match=f"trying to parse filter of measure input `{metric.type_params.measure.name}` on metric `{metric.name}`", - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_numerator_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, lambda metric: metric.type_params is not None and metric.type_params.numerator is not None - ) - assert metric.type_params.numerator is not None - metric.type_params.numerator.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, match=f"trying to parse the numerator filter on metric `{metric.name}`" - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_denominator_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, lambda metric: metric.type_params is not None and metric.type_params.denominator is not None - ) - assert metric.type_params.denominator is not None - metric.type_params.denominator.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, match=f"trying to parse the denominator filter on metric `{metric.name}`" - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_input_metric_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, - lambda metric: metric.type_params is not None - and metric.type_params.metrics is not None - and len(metric.type_params.metrics) > 0, - ) - assert metric.type_params.metrics is not None - input_metric = metric.type_params.metrics[0] - input_metric.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, - match=f"trying to parse filter for input metric `{input_metric.name}` on metric `{metric.name}`", - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_invalid_granularity( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, - lambda metric: metric.type_params is not None - and metric.type_params.metrics is not None - and len(metric.type_params.metrics) > 0, - ) - assert metric.type_params.metrics is not None - input_metric = metric.type_params.metrics[0] - input_metric.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'month') }}"), - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'MONTH') }}"), - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - issues = validator.validate_semantic_manifest(manifest) - assert not issues.has_blocking_issues - assert len(issues.warnings) == 1 - assert "`cool` is not a valid granularity name" in issues.warnings[0].message - - def test_conversion_metrics() -> None: # noqa: D base_measure_name = "base_measure" conversion_measure_name = "conversion_measure" diff --git a/tests/validations/test_saved_query.py b/tests/validations/test_saved_query.py index 56f50dea..566407f9 100644 --- a/tests/validations/test_saved_query.py +++ b/tests/validations/test_saved_query.py @@ -12,47 +12,15 @@ from dbt_semantic_interfaces.implementations.semantic_manifest import ( PydanticSemanticManifest, ) +from dbt_semantic_interfaces.test_utils import check_only_one_error_with_message from dbt_semantic_interfaces.validations.saved_query import SavedQueryRule from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( SemanticManifestValidator, ) -from dbt_semantic_interfaces.validations.validator_helpers import ( - SemanticManifestValidationResults, -) logger = logging.getLogger(__name__) -def check_only_one_error_with_message( # noqa: D - results: SemanticManifestValidationResults, target_message: str -) -> None: - assert len(results.warnings) == 0 - assert len(results.errors) == 1 - assert len(results.future_errors) == 0 - - found_match = results.errors[0].message.find(target_message) != -1 - # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. - assert { - "expected": target_message, - "actual": results.errors[0].message, - } and found_match - - -def check_only_one_warning_with_message( # noqa: D - results: SemanticManifestValidationResults, target_message: str -) -> None: - assert len(results.warnings) == 1 - assert len(results.errors) == 0 - assert len(results.future_errors) == 0 - - found_match = results.warnings[0].message.find(target_message) != -1 - # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. - assert { - "expected": target_message, - "actual": results.warnings[0].message, - } and found_match - - def test_invalid_metric_in_saved_query( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: @@ -77,59 +45,6 @@ def test_invalid_metric_in_saved_query( # noqa: D ) -def test_invalid_where_in_saved_query( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) - manifest.saved_queries = [ - PydanticSavedQuery( - name="Example Saved Query", - description="Example description.", - query_params=PydanticSavedQueryQueryParams( - metrics=["bookings"], - group_by=["Dimension('booking__is_instant')"], - where=PydanticWhereFilterIntersection( - where_filters=[PydanticWhereFilter(where_sql_template="{{ invalid_jinja }}")], - ), - ), - ), - ] - - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) - check_only_one_error_with_message( - manifest_validator.validate_semantic_manifest(manifest), - "trying to parse a filter in saved query", - ) - - -def test_where_filter_validations_invalid_granularity( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) - - manifest.saved_queries = [ - PydanticSavedQuery( - name="Example Saved Query", - description="Example description.", - query_params=PydanticSavedQueryQueryParams( - metrics=["bookings"], - group_by=["Dimension('booking__is_instant')"], - where=PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), - ] - ), - ), - ), - ] - - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) - check_only_one_warning_with_message( - manifest_validator.validate_semantic_manifest(manifest), - "is not a valid granularity name", - ) - - def test_invalid_group_by_element_in_saved_query( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: @@ -180,30 +95,6 @@ def test_invalid_group_by_format_in_saved_query( # noqa: D ) -def test_metric_filter_error( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) - manifest.saved_queries = [ - PydanticSavedQuery( - name="Example Saved Query", - description="Example description.", - query_params=PydanticSavedQueryQueryParams( - metrics=["listings"], - where=PydanticWhereFilterIntersection( - where_filters=[PydanticWhereFilter(where_sql_template="{{ Metric('bookings') }} > 2")], - ), - ), - ), - ] - - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) - check_only_one_error_with_message( - manifest_validator.validate_semantic_manifest(manifest), - "An error occurred while trying to parse a filter in saved query", - ) - - def test_metric_filter_success( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: diff --git a/tests/validations/test_where_filters_are_parseable.py b/tests/validations/test_where_filters_are_parseable.py new file mode 100644 index 00000000..15b75986 --- /dev/null +++ b/tests/validations/test_where_filters_are_parseable.py @@ -0,0 +1,309 @@ +import copy +import logging + +import pytest + +from dbt_semantic_interfaces.implementations.filters.where_filter import ( + PydanticWhereFilter, + PydanticWhereFilterIntersection, +) +from dbt_semantic_interfaces.implementations.saved_query import ( + PydanticSavedQuery, + PydanticSavedQueryQueryParams, +) +from dbt_semantic_interfaces.implementations.semantic_manifest import ( + PydanticSemanticManifest, +) +from dbt_semantic_interfaces.test_utils import ( + check_no_errors_or_warnings, + check_only_one_error_with_message, + check_only_one_warning_with_message, + find_metric_with, +) +from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( + SemanticManifestValidator, +) +from dbt_semantic_interfaces.validations.validator_helpers import ( + SemanticManifestValidationException, +) +from dbt_semantic_interfaces.validations.where_filters import ( + WhereFiltersAreParseableRule, +) + +logger = logging.getLogger(__name__) + + +# ------------------------------------------------------------------------------ +# Metric validations +# ------------------------------------------------------------------------------ + + +def test_metric_where_filter_validations_happy( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + results = validator.validate_semantic_manifest(simple_semantic_manifest__with_primary_transforms) + assert not results.has_blocking_issues + + +def test_where_filter_validations_bad_base_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with(manifest, lambda metric: metric.filter is not None) + assert metric.filter is not None + assert len(metric.filter.where_filters) > 0 + metric.filter.where_filters[0].where_sql_template = "{{ dimension('too', 'many', 'variables', 'to', 'handle') }}" + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises(SemanticManifestValidationException, match=f"trying to parse filter of metric `{metric.name}`"): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_measure_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, lambda metric: metric.type_params is not None and metric.type_params.measure is not None + ) + assert metric.type_params.measure is not None + metric.type_params.measure.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, + match=f"trying to parse filter of measure input `{metric.type_params.measure.name}` on metric `{metric.name}`", + ): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_numerator_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, lambda metric: metric.type_params is not None and metric.type_params.numerator is not None + ) + assert metric.type_params.numerator is not None + metric.type_params.numerator.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, match=f"trying to parse the numerator filter on metric `{metric.name}`" + ): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_denominator_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, lambda metric: metric.type_params is not None and metric.type_params.denominator is not None + ) + assert metric.type_params.denominator is not None + metric.type_params.denominator.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, match=f"trying to parse the denominator filter on metric `{metric.name}`" + ): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_input_metric_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, + lambda metric: metric.type_params is not None + and metric.type_params.metrics is not None + and len(metric.type_params.metrics) > 0, + ) + assert metric.type_params.metrics is not None + input_metric = metric.type_params.metrics[0] + input_metric.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, + match=f"trying to parse filter for input metric `{input_metric.name}` on metric `{metric.name}`", + ): + validator.checked_validations(manifest) + + +def test_metric_where_filter_validations_invalid_granularity( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, + lambda metric: metric.type_params is not None + and metric.type_params.metrics is not None + and len(metric.type_params.metrics) > 0, + ) + assert metric.type_params.metrics is not None + input_metric = metric.type_params.metrics[0] + input_metric.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'month') }}"), + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'MONTH') }}"), + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + issues = validator.validate_semantic_manifest(manifest) + assert not issues.has_blocking_issues + assert len(issues.warnings) == 1 + assert "`cool` is not a valid granularity name" in issues.warnings[0].message + + +# ------------------------------------------------------------------------------ +# Saved Query validations +# ------------------------------------------------------------------------------ + + +def test_saved_query_with_happy_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'hour') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_no_errors_or_warnings(manifest_validator.validate_semantic_manifest(manifest)) + + +def test_saved_query_validates_granularity_name_despite_case( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'DAY') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_no_errors_or_warnings(manifest_validator.validate_semantic_manifest(manifest)) + + +def test_invalid_where_in_saved_query( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[PydanticWhereFilter(where_sql_template="{{ invalid_jinja }}")], + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_only_one_error_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "trying to parse a filter in saved query", + ) + + +def test_saved_query_where_filter_validations_invalid_granularity( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_only_one_warning_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "is not a valid granularity name", + ) + + +def test_metric_filter_error( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["listings"], + where=PydanticWhereFilterIntersection( + where_filters=[PydanticWhereFilter(where_sql_template="{{ Metric('bookings') }} > 2")], + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_only_one_error_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "An error occurred while trying to parse a filter in saved query", + ) From b33ac7e26e8782aaa5940e91e41c72a44bd6f405 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Wed, 30 Oct 2024 18:54:09 -0700 Subject: [PATCH 5/7] Addresses all comments. --- dbt_semantic_interfaces/test_utils.py | 70 ++++++--- .../semantic_manifest_validator.py | 6 +- .../validations/where_filters.py | 144 +++++++++--------- .../test_where_filters_are_parseable.py | 28 ++-- 4 files changed, 138 insertions(+), 110 deletions(-) diff --git a/dbt_semantic_interfaces/test_utils.py b/dbt_semantic_interfaces/test_utils.py index 88cdd625..fc9a01b2 100644 --- a/dbt_semantic_interfaces/test_utils.py +++ b/dbt_semantic_interfaces/test_utils.py @@ -27,6 +27,7 @@ from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity from dbt_semantic_interfaces.validations.validator_helpers import ( SemanticManifestValidationResults, + ValidationIssue, ) logger = logging.getLogger(__name__) @@ -174,37 +175,62 @@ def semantic_model_with_guaranteed_meta( ) -def check_only_one_error_with_message( # noqa: D - results: SemanticManifestValidationResults, target_message: str +def _assert_expected_validation_message( # noqa: D + issues: Sequence[ValidationIssue], + message_fragment: str, ) -> None: - assert len(results.warnings) == 0 - assert len(results.errors) == 1 - assert len(results.future_errors) == 0 - - found_match = results.errors[0].message.find(target_message) != -1 + found_match = any([issue.message.find(message_fragment) != -1 for issue in issues]) # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. assert { - "expected": target_message, - "actual": results.errors[0].message, + "expected": message_fragment, + "actual_messages": [issue.message for issue in issues], } and found_match -def check_only_one_warning_with_message( # noqa: D +def check_expected_issues( # noqa: D + results: SemanticManifestValidationResults, + num_expected_errors: int = 0, + num_expected_warnings: int = 0, + expected_error_msgs: Sequence[str] = [], + expected_warning_msgs: Sequence[str] = [], +) -> None: + """Validates the number, type, and content of ValidationIssues. + + Currently assumes zero future_errors as there are no future_errors + implemented, but this function can be expanded to cover those if needed. + """ + assert len(results.warnings) == num_expected_warnings + assert len(results.errors) == num_expected_errors + assert len(results.future_errors) == 0, "validation function expects zero future_errors to be implemented." + + for expected_error_msg in expected_error_msgs: + _assert_expected_validation_message(issues=results.errors, message_fragment=expected_error_msg) + for expected_warning_msg in expected_warning_msgs: + _assert_expected_validation_message(issues=results.warnings, message_fragment=expected_warning_msg) + + +def check_only_one_error_with_message( # noqa: D results: SemanticManifestValidationResults, target_message: str ) -> None: - assert len(results.errors) == 0 - assert len(results.warnings) == 1 - assert len(results.future_errors) == 0 + check_expected_issues( + results=results, + num_expected_errors=1, + expected_error_msgs=[target_message], + ) - found_match = results.warnings[0].message.find(target_message) != -1 - # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. - assert { - "expected": target_message, - "actual": results.warnings[0].message, - } and found_match + +def check_only_one_warning_with_message( # noqa: D + results: SemanticManifestValidationResults, target_message: str +) -> None: + check_expected_issues( + results=results, + num_expected_warnings=1, + expected_warning_msgs=[target_message], + ) def check_no_errors_or_warnings(results: SemanticManifestValidationResults) -> None: # noqa: D - assert len(results.errors) == 0 - assert len(results.warnings) == 0 - assert len(results.future_errors) == 0 + # no num arguments required since all defaults are zero + check_expected_issues( + results=results, + ) diff --git a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py index cfad51ed..44de62cf 100644 --- a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py +++ b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py @@ -46,9 +46,7 @@ SemanticManifestValidationResults, SemanticManifestValidationRule, ) -from dbt_semantic_interfaces.validations.where_filters import ( - WhereFiltersAreParseableRule, -) +from dbt_semantic_interfaces.validations.where_filters import WhereFiltersAreParseable logger = logging.getLogger(__name__) @@ -88,7 +86,7 @@ class SemanticManifestValidator(Generic[SemanticManifestT]): SemanticModelDefaultsRule[SemanticManifestT](), PrimaryEntityRule[SemanticManifestT](), PrimaryEntityDimensionPairs[SemanticManifestT](), - WhereFiltersAreParseableRule[SemanticManifestT](), + WhereFiltersAreParseable[SemanticManifestT](), SavedQueryRule[SemanticManifestT](), MetricLabelsRule[SemanticManifestT](), SemanticModelLabelsRule[SemanticManifestT](), diff --git a/dbt_semantic_interfaces/validations/where_filters.py b/dbt_semantic_interfaces/validations/where_filters.py index 333be457..01ba5364 100644 --- a/dbt_semantic_interfaces/validations/where_filters.py +++ b/dbt_semantic_interfaces/validations/where_filters.py @@ -1,4 +1,5 @@ import traceback +from enum import StrEnum, auto from typing import Generic, List, Sequence, Tuple from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets @@ -20,83 +21,84 @@ ) -class WhereFiltersAreParseableRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): - """Validates that all Metric WhereFilters are parseable.""" +class SemanticManifestNodeType(StrEnum): + """Types of objects to validate (used for validation messages).""" + + SAVED_QUERY = auto() + METRIC = auto() + + +class WhereFiltersAreParseable(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): + """Validates that all WhereFilters are parseable.""" @staticmethod - def _validate_time_granularity_names_impl( - location_label_for_errors: str, - filter_call_parameter_sets_with_context: Sequence[Tuple[ValidationContext, FilterCallParameterSets]], - custom_granularity_names: List[str], + def _validate_time_granularity_names( + element_name: str, + object_type: SemanticManifestNodeType, + context: ValidationContext, + filter_call_param_sets: FilterCallParameterSets, + valid_granularity_names: List[str], ) -> Sequence[ValidationIssue]: issues: List[ValidationIssue] = [] - valid_granularity_names = [ - standard_granularity.value for standard_granularity in TimeGranularity - ] + custom_granularity_names - - for context, parameter_set in filter_call_parameter_sets_with_context: - for time_dim_call_parameter_set in parameter_set.time_dimension_call_parameter_sets: - if not time_dim_call_parameter_set.time_granularity_name: - continue - if time_dim_call_parameter_set.time_granularity_name.lower() not in valid_granularity_names: - issues.append( - ValidationWarning( - context=context, - message=f"Filter for `{location_label_for_errors}` is not valid. " - f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. " - f"Valid granularity options: {valid_granularity_names}", - ) + for time_dim_call_parameter_set in filter_call_param_sets.time_dimension_call_parameter_sets: + if not time_dim_call_parameter_set.time_granularity_name: + continue + if time_dim_call_parameter_set.time_granularity_name.lower() not in valid_granularity_names: + issues.append( + ValidationWarning( + context=context, + message=f"Filter for {object_type} `{element_name}` is not valid. " + f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. " + f"Valid granularity options: {valid_granularity_names}", ) + ) return issues @staticmethod def _validate_time_granularity_names_for_saved_query( - saved_query: SavedQuery, custom_granularity_names: List[str] + saved_query: SavedQuery, valid_granularity_names: List[str] ) -> Sequence[ValidationIssue]: where_param = saved_query.query_params.where if where_param is None: return [] - return WhereFiltersAreParseableRule._validate_time_granularity_names_impl( - location_label_for_errors="saved query `{saved_query.name}`", - filter_call_parameter_sets_with_context=[ - ( - SavedQueryContext( - file_context=FileContext.from_metadata(metadata=saved_query.metadata), - element_type=SavedQueryElementType.WHERE, - element_value=where_filter.where_sql_template, - ), - where_filter.call_parameter_sets, - ) - for where_filter in where_param.where_filters - ], - custom_granularity_names=custom_granularity_names, - ) + issues: List[ValidationIssue] = [] + for where_filter in where_param.where_filters: + issues += WhereFiltersAreParseable._validate_time_granularity_names( + element_name=saved_query.name, + object_type=SemanticManifestNodeType.SAVED_QUERY, + context=SavedQueryContext( + file_context=FileContext.from_metadata(metadata=saved_query.metadata), + element_type=SavedQueryElementType.WHERE, + element_value=where_filter.where_sql_template, + ), + filter_call_param_sets=where_filter.call_parameter_sets, + valid_granularity_names=valid_granularity_names, + ) + + return issues @staticmethod def _validate_time_granularity_names_for_metric( context: MetricContext, filter_expression_parameter_sets: Sequence[Tuple[str, FilterCallParameterSets]], - custom_granularity_names: List[str], + valid_granularity_names: List[str], ) -> Sequence[ValidationIssue]: - return WhereFiltersAreParseableRule._validate_time_granularity_names_impl( - location_label_for_errors="metric `{context.metric.metric_name}`", - filter_call_parameter_sets_with_context=[ - ( - context, - param_set[1], - ) - for param_set in filter_expression_parameter_sets - ], - custom_granularity_names=custom_granularity_names, - ) + issues: List[ValidationIssue] = [] + for _, param_set in filter_expression_parameter_sets: + issues += WhereFiltersAreParseable._validate_time_granularity_names( + element_name=context.metric.metric_name, + object_type=SemanticManifestNodeType.METRIC, + context=context, + filter_call_param_sets=param_set, + valid_granularity_names=valid_granularity_names, + ) + return issues @staticmethod @validate_safely("validating the where field in a saved query.") - def _validate_saved_query( - saved_query: SavedQuery, custom_granularity_names: List[str] - ) -> Sequence[ValidationIssue]: + def _validate_saved_query(saved_query: SavedQuery, valid_granularity_names: List[str]) -> Sequence[ValidationIssue]: issues: List[ValidationIssue] = [] if saved_query.query_params.where is None: return issues @@ -119,8 +121,8 @@ def _validate_saved_query( ) ) else: - issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_saved_query( - saved_query, custom_granularity_names + issues += WhereFiltersAreParseable._validate_time_granularity_names_for_saved_query( + saved_query, valid_granularity_names ) return issues @@ -129,7 +131,7 @@ def _validate_saved_query( @validate_safely( whats_being_done="running model validation ensuring a metric's filter properties are configured properly" ) - def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Sequence[ValidationIssue]: # noqa: D + def _validate_metric(metric: Metric, valid_granularity_names: List[str]) -> Sequence[ValidationIssue]: # noqa: D issues: List[ValidationIssue] = [] context = MetricContext( file_context=FileContext.from_metadata(metadata=metric.metadata), @@ -151,10 +153,10 @@ def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Seq ) ) else: - issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + issues += WhereFiltersAreParseable._validate_time_granularity_names_for_metric( context=context, filter_expression_parameter_sets=metric.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, + valid_granularity_names=valid_granularity_names, ) if metric.type_params: @@ -175,10 +177,10 @@ def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Seq ) ) else: - issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + issues += WhereFiltersAreParseable._validate_time_granularity_names_for_metric( context=context, filter_expression_parameter_sets=measure.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, + valid_granularity_names=valid_granularity_names, ) numerator = metric.type_params.numerator @@ -197,10 +199,10 @@ def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Seq ) ) else: - issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + issues += WhereFiltersAreParseable._validate_time_granularity_names_for_metric( context=context, filter_expression_parameter_sets=numerator.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, + valid_granularity_names=valid_granularity_names, ) denominator = metric.type_params.denominator @@ -219,10 +221,10 @@ def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Seq ) ) else: - issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + issues += WhereFiltersAreParseable._validate_time_granularity_names_for_metric( context=context, filter_expression_parameter_sets=denominator.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, + valid_granularity_names=valid_granularity_names, ) for input_metric in metric.type_params.metrics or []: @@ -242,10 +244,10 @@ def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Seq ) ) else: - issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + issues += WhereFiltersAreParseable._validate_time_granularity_names_for_metric( context=context, filter_expression_parameter_sets=input_metric.filter.filter_expression_parameter_sets, - custom_granularity_names=custom_granularity_names, + valid_granularity_names=valid_granularity_names, ) return issues @@ -258,11 +260,15 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati for time_spine in semantic_manifest.project_configuration.time_spines for granularity in time_spine.custom_granularities ] + valid_granularity_names = [ + standard_granularity.value for standard_granularity in TimeGranularity + ] + custom_granularity_names + for metric in semantic_manifest.metrics or []: - issues += WhereFiltersAreParseableRule._validate_metric( - metric=metric, custom_granularity_names=custom_granularity_names + issues += WhereFiltersAreParseable._validate_metric( + metric=metric, valid_granularity_names=valid_granularity_names ) for saved_query in semantic_manifest.saved_queries: - issues += WhereFiltersAreParseableRule._validate_saved_query(saved_query, custom_granularity_names) + issues += WhereFiltersAreParseable._validate_saved_query(saved_query, valid_granularity_names) return issues diff --git a/tests/validations/test_where_filters_are_parseable.py b/tests/validations/test_where_filters_are_parseable.py index 15b75986..ae948a99 100644 --- a/tests/validations/test_where_filters_are_parseable.py +++ b/tests/validations/test_where_filters_are_parseable.py @@ -26,9 +26,7 @@ from dbt_semantic_interfaces.validations.validator_helpers import ( SemanticManifestValidationException, ) -from dbt_semantic_interfaces.validations.where_filters import ( - WhereFiltersAreParseableRule, -) +from dbt_semantic_interfaces.validations.where_filters import WhereFiltersAreParseable logger = logging.getLogger(__name__) @@ -41,7 +39,7 @@ def test_metric_where_filter_validations_happy( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) results = validator.validate_semantic_manifest(simple_semantic_manifest__with_primary_transforms) assert not results.has_blocking_issues @@ -55,7 +53,7 @@ def test_where_filter_validations_bad_base_filter( # noqa: D assert metric.filter is not None assert len(metric.filter.where_filters) > 0 metric.filter.where_filters[0].where_sql_template = "{{ dimension('too', 'many', 'variables', 'to', 'handle') }}" - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) with pytest.raises(SemanticManifestValidationException, match=f"trying to parse filter of metric `{metric.name}`"): validator.checked_validations(manifest) @@ -74,7 +72,7 @@ def test_where_filter_validations_bad_measure_filter( # noqa: D PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") ] ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) with pytest.raises( SemanticManifestValidationException, match=f"trying to parse filter of measure input `{metric.type_params.measure.name}` on metric `{metric.name}`", @@ -96,7 +94,7 @@ def test_where_filter_validations_bad_numerator_filter( # noqa: D PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") ] ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) with pytest.raises( SemanticManifestValidationException, match=f"trying to parse the numerator filter on metric `{metric.name}`" ): @@ -117,7 +115,7 @@ def test_where_filter_validations_bad_denominator_filter( # noqa: D PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") ] ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) with pytest.raises( SemanticManifestValidationException, match=f"trying to parse the denominator filter on metric `{metric.name}`" ): @@ -142,7 +140,7 @@ def test_where_filter_validations_bad_input_metric_filter( # noqa: D PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") ] ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) with pytest.raises( SemanticManifestValidationException, match=f"trying to parse filter for input metric `{input_metric.name}` on metric `{metric.name}`", @@ -170,7 +168,7 @@ def test_metric_where_filter_validations_invalid_granularity( # noqa: D PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'MONTH') }}"), ] ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) issues = validator.validate_semantic_manifest(manifest) assert not issues.has_blocking_issues assert len(issues.warnings) == 1 @@ -203,7 +201,7 @@ def test_saved_query_with_happy_filter( # noqa: D ), ] - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) check_no_errors_or_warnings(manifest_validator.validate_semantic_manifest(manifest)) @@ -228,7 +226,7 @@ def test_saved_query_validates_granularity_name_despite_case( # noqa: D ), ] - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) check_no_errors_or_warnings(manifest_validator.validate_semantic_manifest(manifest)) @@ -250,7 +248,7 @@ def test_invalid_where_in_saved_query( # noqa: D ), ] - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) check_only_one_error_with_message( manifest_validator.validate_semantic_manifest(manifest), "trying to parse a filter in saved query", @@ -278,7 +276,7 @@ def test_saved_query_where_filter_validations_invalid_granularity( # noqa: D ), ] - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) check_only_one_warning_with_message( manifest_validator.validate_semantic_manifest(manifest), "is not a valid granularity name", @@ -302,7 +300,7 @@ def test_metric_filter_error( # noqa: D ), ] - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) check_only_one_error_with_message( manifest_validator.validate_semantic_manifest(manifest), "An error occurred while trying to parse a filter in saved query", From 80f352fcaa8cfabb99c6b65decc329fddebe3ed1 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Fri, 1 Nov 2024 14:22:16 -0700 Subject: [PATCH 6/7] Import WhereFiltersAreParseable to metrics.py to avoid making breaking change. --- dbt_semantic_interfaces/validations/metrics.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index da9153a8..80278a51 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/dbt_semantic_interfaces/validations/metrics.py @@ -37,6 +37,11 @@ validate_safely, ) +# Avoids breaking change from moving this class out of this file. +from dbt_semantic_interfaces.validations.where_filters import ( + WhereFiltersAreParseable, # noQa +) + class CumulativeMetricRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): """Checks that cumulative metrics are configured properly.""" From 40bd63c5f02c609bb07bdd73fd920a911166dd72 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Fri, 1 Nov 2024 14:27:58 -0700 Subject: [PATCH 7/7] Update SemanticManifestNodeType for compatibility with older python versions --- dbt_semantic_interfaces/validations/where_filters.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dbt_semantic_interfaces/validations/where_filters.py b/dbt_semantic_interfaces/validations/where_filters.py index 01ba5364..57ebaa30 100644 --- a/dbt_semantic_interfaces/validations/where_filters.py +++ b/dbt_semantic_interfaces/validations/where_filters.py @@ -1,5 +1,5 @@ import traceback -from enum import StrEnum, auto +from enum import Enum from typing import Generic, List, Sequence, Tuple from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets @@ -21,11 +21,11 @@ ) -class SemanticManifestNodeType(StrEnum): +class SemanticManifestNodeType(Enum): """Types of objects to validate (used for validation messages).""" - SAVED_QUERY = auto() - METRIC = auto() + SAVED_QUERY = "saved query" + METRIC = "metric" class WhereFiltersAreParseable(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]):