Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behavior change cumulative type param #10909

Merged
merged 4 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20241031-094609.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Behavior change for cumulative metric type param
time: 2024-10-31T09:46:09.757879-05:00
custom:
Author: DevonFulcher
Issue: "10960"
21 changes: 18 additions & 3 deletions core/dbt/contracts/graph/semantic_manifest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import List, Optional
from typing import List, Optional, Set

from dbt import deprecations
from dbt.constants import (
Expand Down Expand Up @@ -58,8 +58,20 @@ def validate(self) -> bool:
return True

semantic_manifest = self._get_pydantic_semantic_manifest()
validator = SemanticManifestValidator[PydanticSemanticManifest]()
validation_results = validator.validate_semantic_manifest(semantic_manifest)

if get_flags().require_nested_cumulative_type_params is False:
metrics_using_old_params: Set[str] = set()
for metric in semantic_manifest.metrics or []:
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)
if metrics_using_old_params:
deprecations.warn(
"mf-cumulative-type-params-deprecation",
)

time_spines = semantic_manifest.project_configuration.time_spines
legacy_time_spines = (
semantic_manifest.project_configuration.time_spine_table_configurations
Expand All @@ -77,6 +89,9 @@ def validate(self) -> bool:
"mf-timespine-without-yaml-configuration",
)

validator = SemanticManifestValidator[PydanticSemanticManifest]()
validation_results = validator.validate_semantic_manifest(semantic_manifest)

for warning in validation_results.warnings:
fire_event(SemanticValidationFailure(msg=warning.message))

Expand Down
2 changes: 2 additions & 0 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ class ProjectFlags(ExtensibleDbtClassMixin):
state_modified_compare_more_unrendered_values: bool = False
state_modified_compare_vars: bool = False
require_yaml_configuration_for_mf_time_spines: bool = False
require_nested_cumulative_type_params: bool = False

@property
def project_only_flags(self) -> Dict[str, Any]:
Expand All @@ -356,6 +357,7 @@ def project_only_flags(self) -> Dict[str, Any]:
"state_modified_compare_more_unrendered_values": self.state_modified_compare_more_unrendered_values,
"state_modified_compare_vars": self.state_modified_compare_vars,
"require_yaml_configuration_for_mf_time_spines": self.require_yaml_configuration_for_mf_time_spines,
"require_nested_cumulative_type_params": self.require_nested_cumulative_type_params,
}


Expand Down
6 changes: 6 additions & 0 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ class MFTimespineWithoutYamlConfigurationDeprecation(DBTDeprecation):
_event = "MFTimespineWithoutYamlConfigurationDeprecation"


class MFCumulativeTypeParamsDeprecation(DBTDeprecation):
_name = "mf-cumulative-type-params-deprecation"
_event = "MFCumulativeTypeParamsDeprecation"


def renamed_env_var(old_name: str, new_name: str):
class EnvironmentVariableRenamed(DBTDeprecation):
_name = f"environment-variable-renamed:{old_name}"
Expand Down Expand Up @@ -172,6 +177,7 @@ def show_callback():
ResourceNamesWithSpacesDeprecation(),
SourceFreshnessProjectHooksNotRun(),
MFTimespineWithoutYamlConfigurationDeprecation(),
MFCumulativeTypeParamsDeprecation(),
]

deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list}
Expand Down
8 changes: 8 additions & 0 deletions core/dbt/events/core_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,14 @@ message MFTimespineWithoutYamlConfigurationDeprecationMsg {
MFTimespineWithoutYamlConfigurationDeprecation data = 2;
}

// D019
message MFCumulativeTypeParamsDeprecation {}

message MFCumulativeTypeParamsDeprecationMsg {
CoreEventInfo info = 1;
MFCumulativeTypeParamsDeprecation data = 2;
}

// I065
message DeprecatedModel {
string model_name = 1;
Expand Down
1,182 changes: 593 additions & 589 deletions core/dbt/events/core_types_pb2.py

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,16 @@ def message(self) -> str:
return line_wrap_message(warning_tag(description))


class MFCumulativeTypeParamsDeprecation(WarnLevel):
def code(self) -> str:
return "D019"

def message(self) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be easy to add the names of the outdated metrics in this message? Otherwise it might be hard for users to find.
But if that doesn't work with the behavior change objects easily, it probably isn't worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, I don't think that is a straightforward change, given that it would change the interface. I think that could be an enhancement in the future.

description = "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`. See documentation on behavior changes: https://docs.getdbt.com/reference/global-configs/behavior-changes."

return line_wrap_message(warning_tag(description))


# =======================================================
# I - Project parsing
# =======================================================
Expand Down
62 changes: 61 additions & 1 deletion tests/unit/contracts/graph/test_semantic_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,16 @@
import pytest

from core.dbt.contracts.graph.manifest import Manifest
from core.dbt.contracts.graph.nodes import ModelNode
from core.dbt.contracts.graph.nodes import Metric, ModelNode
from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.metric import (
CumulativeTypeParams,
MetricTimeWindow,
MetricTypeParams,
)
from dbt.contracts.graph.semantic_manifest import SemanticManifest
from dbt_semantic_interfaces.type_enums import TimeGranularity
from dbt_semantic_interfaces.type_enums.metric_type import MetricType


# Overwrite the default nods to construct the manifest
Expand Down Expand Up @@ -46,3 +54,55 @@ def test_require_yaml_configuration_for_mf_time_spines(
sm_manifest = SemanticManifest(manifest)
assert sm_manifest.validate()
assert patched_deprecations.warn.call_count == 1

@pytest.mark.parametrize(
"metric_type_params, num_warns",
[
(MetricTypeParams(grain_to_date=TimeGranularity.MONTH), 1),
(
MetricTypeParams(
window=MetricTimeWindow(count=1, granularity=TimeGranularity.MONTH)
),
1,
),
(
MetricTypeParams(
cumulative_type_params=CumulativeTypeParams(
grain_to_date=TimeGranularity.MONTH,
)
),
0,
),
(
MetricTypeParams(
cumulative_type_params=CumulativeTypeParams(
window=MetricTimeWindow(count=1, granularity=TimeGranularity.MONTH),
)
),
0,
),
],
)
def test_deprecate_cumulative_type_params(
self, manifest: Manifest, metric_type_params: MetricTypeParams, num_warns: int
):
with patch("dbt.contracts.graph.semantic_manifest.get_flags") as patched_get_flags, patch(
"dbt.contracts.graph.semantic_manifest.deprecations"
) as patched_deprecations:
patched_get_flags.return_value.require_nested_cumulative_type_params = False
manifest.metrics["metric.test.my_metric"] = Metric(
name="my_metric",
type=MetricType.CUMULATIVE,
type_params=metric_type_params,
resource_type=NodeType.Metric,
package_name="test",
path="models/test/my_metric.yml",
original_file_path="models/test/my_metric.yml",
unique_id="metric.test.my_metric",
fqn=["test", "my_metric"],
description="My metric",
label="My Metric",
)
sm_manifest = SemanticManifest(manifest)
assert sm_manifest.validate()
assert patched_deprecations.warn.call_count == num_warns
1 change: 1 addition & 0 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def test_event_codes(self):
),
core_types.SourceFreshnessProjectHooksNotRun(),
core_types.MFTimespineWithoutYamlConfigurationDeprecation(),
core_types.MFCumulativeTypeParamsDeprecation(),
# E - DB Adapter ======================
adapter_types.AdapterEventDebug(),
adapter_types.AdapterEventInfo(),
Expand Down
Loading