From d41075e3501acad2e29a519d8ae32fb4e9999588 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Thu, 20 Jun 2024 12:37:40 -0700 Subject: [PATCH] Adjust application of `cumulative_type_params` to smooth out release process (#298) ### Description 2 adjustments related to `cumulative_type_params`: - Add `SetCumulativeTypeParamsRule` to the set of transformations that actually get applied in prod & test. - Disable related validations temporarily. This will allow us to add support for cumulative_type_params in MetricFlow before these validations roll out to customers. Also upgrades to a new production patch version so that we these changes can be used in `metricflow-semantics` appropriately. ### Checklist - [x] I have read [the contributing guide](https://github.com/dbt-labs/dbt-semantic-interfaces/blob/main/CONTRIBUTING.md) and understand what's expected of me - [x] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements) - [x] This PR includes tests, or tests are not required/relevant for this PR - [x] I have run `changie new` to [create a changelog entry](https://github.com/dbt-labs/dbt-semantic-interfaces/blob/main/CONTRIBUTING.md#adding-a-changelog-entry) --- .../transformations/pydantic_rule_set.py | 4 ++ .../validations/metrics.py | 7 +++- pyproject.toml | 2 +- tests/validations/test_metrics.py | 37 ++++++++++++------- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/dbt_semantic_interfaces/transformations/pydantic_rule_set.py b/dbt_semantic_interfaces/transformations/pydantic_rule_set.py index bf4ff96e..7c2f4568 100644 --- a/dbt_semantic_interfaces/transformations/pydantic_rule_set.py +++ b/dbt_semantic_interfaces/transformations/pydantic_rule_set.py @@ -17,6 +17,9 @@ from dbt_semantic_interfaces.transformations.convert_median import ( ConvertMedianToPercentileRule, ) +from dbt_semantic_interfaces.transformations.cumulative_type_params import ( + SetCumulativeTypeParamsRule, +) from dbt_semantic_interfaces.transformations.names import LowerCaseNamesRule from dbt_semantic_interfaces.transformations.proxy_measure import CreateProxyMeasureRule from dbt_semantic_interfaces.transformations.rule_set import ( @@ -50,6 +53,7 @@ def secondary_rules(self) -> Sequence[SemanticManifestTransformRule[PydanticSema ConvertCountToSumRule(), ConvertMedianToPercentileRule(), AddInputMetricMeasuresRule(), + SetCumulativeTypeParamsRule(), ) @property diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index 42fa1ac3..ac206bdd 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/dbt_semantic_interfaces/validations/metrics.py @@ -24,6 +24,9 @@ validate_safely, ) +# Temp: undo once cumulative_type_params are supported in MF +CUMULATIVE_TYPE_PARAMS_SUPPORTED = False + class CumulativeMetricRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): """Checks that cumulative sum metrics are configured properly.""" @@ -42,7 +45,7 @@ def _validate_cumulative_sum_metric_params(metric: Metric) -> List[ValidationIss for field in ("window", "grain_to_date"): type_params_field_value = getattr(metric.type_params, field) # Warn that the old type_params structure has been deprecated. - if type_params_field_value: + if type_params_field_value and CUMULATIVE_TYPE_PARAMS_SUPPORTED: issues.append( ValidationWarning( context=metric_context, @@ -62,7 +65,7 @@ def _validate_cumulative_sum_metric_params(metric: Metric) -> List[ValidationIss type_params_field_value and cumulative_type_params_field_value and cumulative_type_params_field_value != type_params_field_value - ): + ) and CUMULATIVE_TYPE_PARAMS_SUPPORTED: issues.append( ValidationError( context=metric_context, diff --git a/pyproject.toml b/pyproject.toml index bf0591e9..736e009d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "dbt-semantic-interfaces" -version = "0.6.1.dev0" +version = "0.6.1" description = 'The shared semantic layer definitions that dbt-core and MetricFlow use' readme = "README.md" requires-python = ">=3.8" diff --git a/tests/validations/test_metrics.py b/tests/validations/test_metrics.py index 4e021e41..db10f2a6 100644 --- a/tests/validations/test_metrics.py +++ b/tests/validations/test_metrics.py @@ -43,6 +43,7 @@ TimeGranularity, ) from dbt_semantic_interfaces.validations.metrics import ( + CUMULATIVE_TYPE_PARAMS_SUPPORTED, ConversionMetricRule, CumulativeMetricRule, DerivedMetricRule, @@ -690,17 +691,25 @@ def test_cumulative_metrics() -> None: # noqa: D ) build_issues = validation_results.all_issues - for issue in build_issues: - print(issue.message) - assert len(build_issues) == 8 - 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." - 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): - missing_error_strings.add(expected_str) - assert len(missing_error_strings) == 0, "Failed to match one or more expected issues: " - f"{missing_error_strings} in {set([x.as_readable_str() for x in build_issues])}" + if CUMULATIVE_TYPE_PARAMS_SUPPORTED: + assert len(build_issues) == 8 + 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." + 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): + missing_error_strings.add(expected_str) + assert len(missing_error_strings) == 0, "Failed to match one or more expected issues: " + f"{missing_error_strings} in {set([x.as_readable_str() for x in build_issues])}" + else: + assert len(build_issues) == 1 + expected_substr1 = "Both window and grain_to_date set for cumulative metric. Please set one or the other." + missing_error_strings = set() + for expected_str in [expected_substr1]: + if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in build_issues): + missing_error_strings.add(expected_str) + assert len(missing_error_strings) == 0, "Failed to match one or more expected issues: " + f"{missing_error_strings} in {set([x.as_readable_str() for x in build_issues])}"