Skip to content

Commit

Permalink
Behavior change for mf timespinse without yaml config
Browse files Browse the repository at this point in the history
  • Loading branch information
DevonFulcher committed Oct 15, 2024
1 parent ef9abe6 commit d0e339f
Show file tree
Hide file tree
Showing 11 changed files with 836 additions and 765 deletions.
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
15 changes: 15 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 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,19 @@ def validate(self) -> bool:
semantic_manifest = self._get_pydantic_semantic_manifest()
validator = SemanticManifestValidator[PydanticSemanticManifest]()
validation_results = validator.validate_semantic_manifest(semantic_manifest)
new_time_spines = semantic_manifest.project_configuration.time_spines
old_time_spines = semantic_manifest.project_configuration.time_spine_table_configurations
new_time_spines_contain_day = any(
c for c in new_time_spines if c.primary_column.time_granularity == TimeGranularity.DAY
)
if (
get_flags().allow_mf_time_spines_without_yaml_configuration is False
and old_time_spines
and not new_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
allow_mf_time_spines_without_yaml_configuration: 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,
"allow_mf_time_spines_without_yaml_configuration": self.allow_mf_time_spines_without_yaml_configuration,
}


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,519 changes: 761 additions & 758 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.9.0,<2.0",
"dbt-adapters>=1.7.0,<2.0",
Expand Down
27 changes: 25 additions & 2 deletions tests/unit/contracts/graph/test_semantic_manifest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
from argparse import Namespace
from unittest.mock import patch

import pytest

from core.dbt.artifacts.resources.v1.model import TimeSpine
from core.dbt.contracts.graph.manifest import Manifest
from core.dbt.contracts.graph.nodes import ModelNode
from core.dbt.flags import set_from_args
from dbt.contracts.graph.semantic_manifest import SemanticManifest
from tests.unit.utils.manifest import metricflow_time_spine_model


# Overwrite the default nods to construct the manifest
Expand All @@ -24,6 +32,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.allow_mf_time_spines_without_yaml_configuration = True
sm_manifest = SemanticManifest(manifest)
assert sm_manifest.validate()

def test_allow_mf_time_spines_without_yaml_configuration(
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.allow_mf_time_spines_without_yaml_configuration = 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 d0e339f

Please sign in to comment.