Skip to content

Commit

Permalink
Adjust application of cumulative_type_params to smooth out release …
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
courtneyholcomb authored Jun 20, 2024
1 parent 34b80b4 commit d41075e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 17 deletions.
4 changes: 4 additions & 0 deletions dbt_semantic_interfaces/transformations/pydantic_rule_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -50,6 +53,7 @@ def secondary_rules(self) -> Sequence[SemanticManifestTransformRule[PydanticSema
ConvertCountToSumRule(),
ConvertMedianToPercentileRule(),
AddInputMetricMeasuresRule(),
SetCumulativeTypeParamsRule(),
)

@property
Expand Down
7 changes: 5 additions & 2 deletions dbt_semantic_interfaces/validations/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
37 changes: 23 additions & 14 deletions tests/validations/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
TimeGranularity,
)
from dbt_semantic_interfaces.validations.metrics import (
CUMULATIVE_TYPE_PARAMS_SUPPORTED,
ConversionMetricRule,
CumulativeMetricRule,
DerivedMetricRule,
Expand Down Expand Up @@ -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])}"

0 comments on commit d41075e

Please sign in to comment.