From aab434ce5453bd52275d5e472bb02bd6404294ea Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Sun, 14 Jul 2024 20:14:47 -0700 Subject: [PATCH 1/2] Only give one validation warning for deprecated cumulative fields --- .../validations/metrics.py | 46 +++++++++---------- tests/validations/test_metrics.py | 12 ++--- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index b391f9c7..be9e57c0 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/dbt_semantic_interfaces/validations/metrics.py @@ -1,5 +1,5 @@ import traceback -from typing import Dict, Generic, List, Optional, Sequence +from typing import Dict, Generic, List, Optional, Sequence, Set from dbt_semantic_interfaces.errors import ParsingException from dbt_semantic_interfaces.implementations.metric import ( @@ -42,11 +42,15 @@ class CumulativeMetricRule(SemanticManifestValidationRule[SemanticManifestT], Ge """Checks that cumulative sum metrics are configured properly.""" @staticmethod - @validate_safely(whats_being_done="checking that the params of metric are valid if it is a cumulative sum metric") - def _validate_cumulative_sum_metric_params(metric: Metric) -> List[ValidationIssue]: + @validate_safely(whats_being_done="running model validation ensuring cumulative sum metrics are valid") + def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D issues: List[ValidationIssue] = [] - if metric.type == MetricType.CUMULATIVE: + metrics_using_old_params: Set[str] = set() + for metric in semantic_manifest.metrics or []: + if metric.type != MetricType.CUMULATIVE: + continue + metric_context = MetricContext( file_context=FileContext.from_metadata(metadata=metric.metadata), metric=MetricModelReference(metric_name=metric.name), @@ -56,15 +60,8 @@ def _validate_cumulative_sum_metric_params(metric: Metric) -> List[ValidationIss type_params_field_value = getattr(metric.type_params, field) # Warn that the old type_params structure has been deprecated. if type_params_field_value: - issues.append( - ValidationWarning( - context=metric_context, - message=( - f"Cumulative `type_params.{field}` field has been moved and will soon be deprecated. " - f"Please nest that value under `type_params.cumulative_type_params.{field}`." - ), - ) - ) + metrics_using_old_params.add(metric.name) + # Warn that window or grain_to_date is mismatched across params. cumulative_type_params_field_value = ( getattr(metric.type_params.cumulative_type_params, field) @@ -116,16 +113,19 @@ def _validate_cumulative_sum_metric_params(metric: Metric) -> List[ValidationIss extra_detail="".join(traceback.format_tb(e.__traceback__)), ) ) - - return issues - - @staticmethod - @validate_safely(whats_being_done="running model validation ensuring cumulative sum metrics are valid") - def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D - issues: List[ValidationIssue] = [] - - for metric in semantic_manifest.metrics or []: - issues += CumulativeMetricRule._validate_cumulative_sum_metric_params(metric=metric) + if metrics_using_old_params: + issues.append( + ValidationWarning( + context=metric_context, + message=( + "Cumulative fields `type_params.window` and `type_params.grain_to_date` have been moved and " + "will soon be deprecated. Please nest those values under " + "`type_params.cumulative_type_params.window` and " + "`type_params.cumulative_type_params.grain_to_date`. Metrics using old fields: " + f"{sorted(metrics_using_old_params)}" + ), + ) + ) return issues diff --git a/tests/validations/test_metrics.py b/tests/validations/test_metrics.py index 7a9856e0..e3c30331 100644 --- a/tests/validations/test_metrics.py +++ b/tests/validations/test_metrics.py @@ -602,7 +602,7 @@ def test_cumulative_metrics() -> None: # noqa: D ), ], metrics=[ - # Metrics with old type params structure - should 2 get warnings + # Metrics with old type params structure - should get warning metric_with_guaranteed_meta( name="metric1", type=MetricType.CUMULATIVE, @@ -640,7 +640,7 @@ def test_cumulative_metrics() -> None: # noqa: D cumulative_type_params=PydanticCumulativeTypeParams(grain_to_date=TimeGranularity.MONTH), ), ), - # Metric with both window & grain across both type_params - should get 2 warnings + # Metric with both window & grain across both type_params - should get warning metric_with_guaranteed_meta( name="woooooo", type=MetricType.CUMULATIVE, @@ -653,7 +653,7 @@ def test_cumulative_metrics() -> None: # noqa: D ), ), ), - # Metrics with duplicated window or grain_to_date - should 4 get warnings + # Metrics with duplicated window or grain_to_date - should get warning metric_with_guaranteed_meta( name="what_a_metric", type=MetricType.CUMULATIVE, @@ -691,12 +691,12 @@ def test_cumulative_metrics() -> None: # noqa: D ) build_issues = validation_results.all_issues - assert len(build_issues) == 8 + assert len(build_issues) == 4 expected_substr1 = "Both window and grain_to_date set for cumulative metric. Please set one or the other." expected_substr2 = "Got differing values for `window`" expected_substr3 = "Got differing values for `grain_to_date`" - expected_substr4 = "Cumulative `type_params.window` field has been moved and will soon be deprecated." - expected_substr5 = "Cumulative `type_params.grain_to_date` field has been moved and will soon be deprecated." + expected_substr4 = "Cumulative fields `type_params.window` and `type_params.grain_to_date` have been moved" + expected_substr5 = str(sorted({"what_a_metric", "dis_bad", "woooooo", "metric1", "metric2"})) missing_error_strings = set() for expected_str in [expected_substr1, expected_substr2, expected_substr3, expected_substr4, expected_substr5]: if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in build_issues): From 8570a751f6d6ffc41bc8d48c965966dc24c682f1 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Sun, 14 Jul 2024 20:15:15 -0700 Subject: [PATCH 2/2] Bump version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 44a10553..d94278ca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "dbt-semantic-interfaces" -version = "0.6.7" +version = "0.6.8" description = 'The shared semantic layer definitions that dbt-core and MetricFlow use' readme = "README.md" requires-python = ">=3.8"