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 for mf timespine without yaml configuration #10857

Merged
Merged
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20241016-110321.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-16T11:03:21.123552-05:00
custom:
Author: DevonFulcher
Issue: None
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 (
DevonFulcher marked this conversation as resolved.
Show resolved Hide resolved
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",
)
Copy link
Contributor

@MichelleArk MichelleArk Oct 29, 2024

Choose a reason for hiding this comment

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

Presumably this warning would not be terribly noisy -- (at most) one per invocation? Let me know if I'm off!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is validation for the semantic manifest which there is only one of per project. I can't think of a way that this would fire more than once per invocation.


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.
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for these updates <3


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