Skip to content

Commit

Permalink
Remove cumulative metric type param deprecation warning (#356)
Browse files Browse the repository at this point in the history
### Description

This PR removes a deprecation warning so that it can be moved into
dbt-core as a behavior change.

### 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)

---------

Co-authored-by: Courtney Holcomb <[email protected]>
  • Loading branch information
DevonFulcher and courtneyholcomb authored Oct 15, 2024
1 parent b5feed6 commit 4e1dd8c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 23 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20241015-110006.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Remove cumulative metric type param deprecation warning
time: 2024-10-15T11:00:06.108655-05:00
custom:
Author: DevonFulcher
Issue: None
19 changes: 1 addition & 18 deletions dbt_semantic_interfaces/validations/metrics.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import traceback
from typing import Dict, Generic, List, Optional, Sequence, Set, Tuple
from typing import Dict, Generic, List, Optional, Sequence, Tuple

from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets
from dbt_semantic_interfaces.errors import ParsingException
Expand Down Expand Up @@ -48,7 +48,6 @@ class CumulativeMetricRule(SemanticManifestValidationRule[SemanticManifestT], Ge
def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D
issues: List[ValidationIssue] = []

metrics_using_old_params: Set[str] = set()
for metric in semantic_manifest.metrics or []:
if metric.type != MetricType.CUMULATIVE:
continue
Expand All @@ -60,9 +59,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati

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:
metrics_using_old_params.add(metric.name)

# Warn that window or grain_to_date is mismatched across params.
cumulative_type_params_field_value = (
Expand Down Expand Up @@ -115,19 +111,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
extra_detail="".join(traceback.format_tb(e.__traceback__)),
)
)
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

Expand Down
10 changes: 5 additions & 5 deletions tests/validations/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ def check_error_in_issues(error_substrings: List[str], issues: Tuple[ValidationI
for expected_str in error_substrings:
if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in 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 issues])}"
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 issues])}"
)


def test_metric_no_time_dim_dim_only_source() -> None: # noqa:D
Expand Down Expand Up @@ -742,13 +744,11 @@ def test_cumulative_metrics() -> None: # noqa: D
)

build_issues = validation_results.all_issues
assert len(build_issues) == 4
assert len(build_issues) == 3
expected_substrings = [
"Both window and grain_to_date set for cumulative metric. Please set one or the other.",
"Got differing values for `window`",
"Got differing values for `grain_to_date`",
"Cumulative fields `type_params.window` and `type_params.grain_to_date` have been moved",
str(sorted({"what_a_metric", "dis_bad", "woooooo", "metric1", "metric2"})),
]
check_error_in_issues(error_substrings=expected_substrings, issues=build_issues)

Expand Down

0 comments on commit 4e1dd8c

Please sign in to comment.