Skip to content

Commit

Permalink
Behavior change for mf timespine without yaml configuration (#10857)
Browse files Browse the repository at this point in the history
  • Loading branch information
DevonFulcher authored Oct 31, 2024
1 parent 8c6bec4 commit 8a17a0d
Show file tree
Hide file tree
Showing 12 changed files with 673 additions and 596 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20241031-093251.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Behavior change for mf timespine without yaml configuration
time: 2024-10-31T09:32:51.166594-05:00
custom:
Author: DevonFulcher
Issue: "10959"
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,6 @@ venv/

# poetry
poetry.lock

# asdf
.tool-versions
18 changes: 18 additions & 0 deletions core/dbt/contracts/graph/semantic_manifest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from typing import List, Optional

from dbt import deprecations
from dbt.constants import (
LEGACY_TIME_SPINE_GRANULARITY,
LEGACY_TIME_SPINE_MODEL_NAME,
Expand All @@ -9,6 +10,7 @@
from dbt.contracts.graph.nodes import ModelNode
from dbt.events.types import ArtifactWritten, SemanticValidationFailure
from dbt.exceptions import ParsingError
from dbt.flags import get_flags
from dbt_common.clients.system import write_file
from dbt_common.events.base_types import EventLevel
from dbt_common.events.functions import fire_event
Expand Down Expand Up @@ -58,6 +60,22 @@ def validate(self) -> bool:
semantic_manifest = self._get_pydantic_semantic_manifest()
validator = SemanticManifestValidator[PydanticSemanticManifest]()
validation_results = validator.validate_semantic_manifest(semantic_manifest)
time_spines = semantic_manifest.project_configuration.time_spines
legacy_time_spines = (
semantic_manifest.project_configuration.time_spine_table_configurations
)
# If the time spine contains a day grain then it is functionally equivalent to the legacy time spine.
time_spines_contain_day = any(
c for c in time_spines if c.primary_column.time_granularity == TimeGranularity.DAY
)
if (
get_flags().require_yaml_configuration_for_mf_time_spines is False
and legacy_time_spines
and not time_spines_contain_day
):
deprecations.warn(
"mf-timespine-without-yaml-configuration",
)

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 @@ -344,6 +344,7 @@ class ProjectFlags(ExtensibleDbtClassMixin):
skip_nodes_if_on_run_start_fails: bool = False
state_modified_compare_more_unrendered_values: bool = False
state_modified_compare_vars: bool = False
require_yaml_configuration_for_mf_time_spines: bool = False

@property
def project_only_flags(self) -> Dict[str, Any]:
Expand All @@ -354,6 +355,7 @@ def project_only_flags(self) -> Dict[str, Any]:
"skip_nodes_if_on_run_start_fails": self.skip_nodes_if_on_run_start_fails,
"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,
}


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


class MFTimespineWithoutYamlConfigurationDeprecation(DBTDeprecation):
_name = "mf-timespine-without-yaml-configuration"
_event = "MFTimespineWithoutYamlConfigurationDeprecation"


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

deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list}
Expand Down
8 changes: 4 additions & 4 deletions core/dbt/events/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Events Module
The Events module is responsible for communicating internal dbt structures into a consumable interface. Because the "event" classes are based entirely on protobuf definitions, the interface is really clearly defined, whether or not protobufs are used to consume it. We use Betterproto for compiling the protobuf message definitions into Python classes.
The Events module is responsible for communicating internal dbt structures into a consumable interface. Because the "event" classes are based entirely on protobuf definitions, the interface is really clearly defined, whether or not protobufs are used to consume it. We use protoc for compiling the protobuf message definitions into Python classes.

# Using the Events Module
The event module provides types that represent what is happening in dbt in `events.types`. These types are intended to represent an exhaustive list of all things happening within dbt that will need to be logged, streamed, or printed. To fire an event, `common.events.functions::fire_event` is the entry point to the module from everywhere in dbt.
Expand All @@ -8,14 +8,14 @@ The event module provides types that represent what is happening in dbt in `even
When events are processed via `fire_event`, nearly everything is logged. Whether or not the user has enabled the debug flag, all debug messages are still logged to the file. However, some events are particularly time consuming to construct because they return a huge amount of data. Today, the only messages in this category are cache events and are only logged if the `--log-cache-events` flag is on. This is important because these messages should not be created unless they are going to be logged, because they cause a noticable performance degredation. These events use a "fire_event_if" functions.

# Adding a New Event
* Add a new message in types.proto, and a second message with the same name + "Msg". The "Msg" message should have two fields, an "info" field of EventInfo, and a "data" field referring to the message name without "Msg"
* Add a new message in `core_types.proto`, and a second message with the same name + "Msg". The "Msg" message should have two fields, an "info" field of EventInfo, and a "data" field referring to the message name without "Msg"
* run the protoc compiler to update core_types_pb2.py: make core_proto_types
* Add a wrapping class in core/dbt/event/core_types.py with a Level superclass plus code and message methods
* Add the class to tests/unit/test_events.py

We have switched from using betterproto to using google protobuf, because of a lack of support for Struct fields in betterproto.

The google protobuf interface is janky and very much non-Pythonic. The "generated" classes in types_pb2.py do not resemble regular Python classes. They do not have normal constructors; they can only be constructed empty. They can be "filled" by setting fields individually or using a json_format method like ParseDict. We have wrapped the logging events with a class (in types.py) which allows using a constructor -- keywords only, no positional parameters.
The google protobuf interface is janky and very much non-Pythonic. The "generated" classes in types_pb2.py do not resemble regular Python classes. They do not have normal constructors; they can only be constructed empty. They can be "filled" by setting fields individually or using a json_format method like ParseDict. We have wrapped the logging events with a class (in types.py) which allows using a constructor -- keywords only, no positional parameters.

## Required for Every Event

Expand All @@ -37,6 +37,6 @@ class PartialParsingDeletedExposure(DebugLevel):

## Compiling core_types.proto

After adding a new message in `types.proto`, either:
After adding a new message in `core_types.proto`, either:
- In the repository root directory: `make core_proto_types`
- In the `core/dbt/events` directory: `protoc -I=. --python_out=. types.proto`
8 changes: 8 additions & 0 deletions core/dbt/events/core_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,14 @@ message SourceFreshnessProjectHooksNotRunMsg {
SourceFreshnessProjectHooksNotRun data = 2;
}

// D018
message MFTimespineWithoutYamlConfigurationDeprecation {}

message MFTimespineWithoutYamlConfigurationDeprecationMsg {
CoreEventInfo info = 1;
MFTimespineWithoutYamlConfigurationDeprecation 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 @@ -466,6 +466,16 @@ def message(self) -> str:
return line_wrap_message(warning_tag(description))


class MFTimespineWithoutYamlConfigurationDeprecation(WarnLevel):
def code(self) -> str:
return "D018"

def message(self) -> str:
description = "Time spines without YAML configuration are in the process of deprecation. Please add YAML configuration for your 'metricflow_time_spine' model. See documentation on MetricFlow time spines: https://docs.getdbt.com/docs/build/metricflow-time-spine and behavior change documentation: https://docs.getdbt.com/reference/global-configs/behavior-changes."

return line_wrap_message(warning_tag(description))


# =======================================================
# I - Project parsing
# =======================================================
Expand Down
2 changes: 1 addition & 1 deletion core/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
# These are major-version-0 packages also maintained by dbt-labs.
# Accept patches but avoid automatically updating past a set minor version range.
"dbt-extractor>=0.5.0,<=0.6",
"dbt-semantic-interfaces>=0.7.3,<0.8",
"dbt-semantic-interfaces>=0.7.4,<0.8",
# Minor versions for these are expected to be backwards-compatible
"dbt-common>=1.11.0,<2.0",
"dbt-adapters>=1.8.0,<2.0",
Expand Down
23 changes: 21 additions & 2 deletions tests/unit/contracts/graph/test_semantic_manifest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
from unittest.mock import patch

import pytest

from core.dbt.contracts.graph.manifest import Manifest
from core.dbt.contracts.graph.nodes import ModelNode
from dbt.contracts.graph.semantic_manifest import SemanticManifest


Expand All @@ -24,6 +28,21 @@ def metrics(


class TestSemanticManifest:

def test_validate(self, manifest):
sm_manifest = SemanticManifest(manifest)
assert sm_manifest.validate()
with patch("dbt.contracts.graph.semantic_manifest.get_flags") as patched_get_flags:
patched_get_flags.return_value.require_yaml_configuration_for_mf_time_spines = True
sm_manifest = SemanticManifest(manifest)
assert sm_manifest.validate()

def test_require_yaml_configuration_for_mf_time_spines(
self, manifest: Manifest, metricflow_time_spine_model: ModelNode
):
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_yaml_configuration_for_mf_time_spines = False
manifest.nodes[metricflow_time_spine_model.unique_id] = metricflow_time_spine_model
sm_manifest = SemanticManifest(manifest)
assert sm_manifest.validate()
assert patched_deprecations.warn.call_count == 1
1 change: 1 addition & 0 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ def test_event_codes(self):
package_name="my_package", materialization_name="view"
),
core_types.SourceFreshnessProjectHooksNotRun(),
core_types.MFTimespineWithoutYamlConfigurationDeprecation(),
# E - DB Adapter ======================
adapter_types.AdapterEventDebug(),
adapter_types.AdapterEventInfo(),
Expand Down

0 comments on commit 8a17a0d

Please sign in to comment.