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

support meta at config level for metric nodes #9580

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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/Features-20240215-120811.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Support meta at the config level for Metric nodes
time: 2024-02-15T12:08:11.927789-06:00
custom:
Author: emmyoop
Issue: "9441"
6 changes: 4 additions & 2 deletions core/dbt/artifacts/resources/v1/metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
SourceFileMetadata,
WhereFilterIntersection,
)
from dbt_common.contracts.config.base import BaseConfig, CompareBehavior
from dbt_common.contracts.config.base import BaseConfig, CompareBehavior, MergeBehavior
from dbt_common.dataclass_schema import dbtClassMixin
from dbt_semantic_interfaces.references import MeasureReference, MetricReference
from dbt_semantic_interfaces.type_enums import (
Expand Down Expand Up @@ -101,6 +101,8 @@ class MetricConfig(BaseConfig):
metadata=CompareBehavior.Exclude.meta(),
)

meta: Dict[str, Any] = field(default_factory=dict, metadata=MergeBehavior.Update.meta())


@dataclass
class Metric(GraphResource):
Expand All @@ -112,7 +114,7 @@ class Metric(GraphResource):
filter: Optional[WhereFilterIntersection] = None
metadata: Optional[SourceFileMetadata] = None
resource_type: Literal[NodeType.Metric]
meta: Dict[str, Any] = field(default_factory=dict)
meta: Dict[str, Any] = field(default_factory=dict, metadata=MergeBehavior.Update.meta())
tags: List[str] = field(default_factory=list)
config: MetricConfig = field(default_factory=MetricConfig)
unrendered_config: Dict[str, Any] = field(default_factory=dict)
Expand Down
5 changes: 5 additions & 0 deletions core/dbt/parser/schema_yaml_readers.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,11 @@
f"Calculated a {type(config)} for a metric, but expected a MetricConfig"
)

# If we have meta in the config, copy to node level, for backwards
# compatibility with earlier node-only config.
if "meta" in config and config["meta"]:
unparsed.meta = config["meta"]

Check warning on line 372 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L372

Added line #L372 was not covered by tests

parsed = Metric(
resource_type=NodeType.Metric,
package_name=package_name,
Expand Down
86 changes: 85 additions & 1 deletion tests/functional/metrics/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,62 @@
models_people_metrics_yml = """
version: 2

metrics:

- name: number_of_people
label: "Number of people"
description: Total count of people
type: simple
type_params:
measure: people
config:
meta:
my_meta_config: 'config'

- name: collective_tenure
label: "Collective tenure"
description: Total number of years of team experience
type: simple
type_params:
measure:
name: years_tenure
filter: "{{ Dimension('id__loves_dbt') }} is true"
join_to_timespine: true
fill_nulls_with: 0

- name: collective_window
label: "Collective window"
description: Testing window
type: simple
type_params:
measure:
name: years_tenure
filter: "{{ Dimension('id__loves_dbt') }} is true"
window: 14 days

- name: average_tenure
label: Average Tenure
description: The average tenure of our people
type: ratio
type_params:
numerator: collective_tenure
denominator: number_of_people

- name: average_tenure_minus_people
label: Average Tenure minus People
description: Well this isn't really useful is it?
type: derived
type_params:
expr: average_tenure - number_of_people
metrics:
- average_tenure
- number_of_people

"""

models_people_metrics_meta_top_yml = """
version: 2

metrics:

- name: number_of_people
Expand All @@ -106,7 +162,7 @@
type_params:
measure: people
meta:
my_meta: 'testing'
my_meta_top: 'top'

- name: collective_tenure
label: "Collective tenure"
Expand Down Expand Up @@ -458,6 +514,34 @@

"""

meta_metric_level_schema_yml = """
version: 2

metrics:

- name: number_of_people
label: "Number of people"
description: Total count of people
type: simple
type_params:
measure: people
config:
meta:
my_meta_config: 'config
meta:
my_meta_direct: 'direct'

- name: collective_tenure
label: "Collective tenure"
description: Total number of years of team experience
type: simple
type_params:
measure:
name: years_tenure
filter: "{{ Dimension('id__loves_dbt') }} is true"

"""

enabled_metric_level_schema_yml = """
version: 2

Expand Down
96 changes: 90 additions & 6 deletions tests/functional/metrics/test_metric_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from tests.functional.metrics.fixtures import (
models_people_sql,
models_people_metrics_yml,
models_people_metrics_meta_top_yml,
metricflow_time_spine_sql,
disabled_metric_level_schema_yml,
enabled_metric_level_schema_yml,
Expand Down Expand Up @@ -40,30 +41,32 @@ def models(self):
def project_config_update(self):
return {
"metrics": {
"average_tenure_minus_people": {
"enabled": True,
},
"test": {
"average_tenure_minus_people": {
"enabled": False,
},
}
}
}

def test_enabled_metric_config_dbt_project(self, project):
run_dbt(["parse"])
manifest = get_manifest(project.project_root)
assert "metric.test.average_tenure_minus_people" in manifest.metrics
assert "metric.test.average_tenure_minus_people" not in manifest.metrics

new_enabled_config = {
"metrics": {
"test": {
"average_tenure_minus_people": {
"enabled": False,
"enabled": True,
},
}
}
}
update_config_file(new_enabled_config, project.project_root, "dbt_project.yml")
run_dbt(["parse"])
manifest = get_manifest(project.project_root)
assert "metric.test.average_tenure_minus_people" not in manifest.metrics
assert "metric.test.average_tenure_minus_people" in manifest.metrics
assert "metric.test.collective_tenure" in manifest.metrics


Expand Down Expand Up @@ -204,3 +207,84 @@ def test_disabling_upstream_metric_errors(self, project):
"The metric `number_of_people` is disabled and thus cannot be referenced."
)
assert expected_msg in str(excinfo.value)


# Test meta config in dbt_project.yml
class TestMetricMetaConfigProjectLevel(MetricConfigTests):
@pytest.fixture(scope="class")
def models(self):
return {
"people.sql": models_people_sql,
"metricflow_time_spine.sql": metricflow_time_spine_sql,
"semantic_model_people.yml": semantic_model_people_yml,
"schema.yml": models_people_metrics_yml,
}

@pytest.fixture(scope="class")
def project_config_update(self):
return {
"metrics": {
"test": {
"average_tenure_minus_people": {
"+meta": {"project_field": "project_value"},
},
}
}
}

def test_meta_metric_config_dbt_project(self, project):
run_dbt(["parse"])
manifest = get_manifest(project.project_root)
assert "metric.test.average_tenure_minus_people" in manifest.metrics
# for backwards compatibility the config level meta gets copied to the top level meta
assert manifest.metrics.get("metric.test.average_tenure_minus_people").config.meta == {
"project_field": "project_value"
}
assert manifest.metrics.get("metric.test.average_tenure_minus_people").meta == {
"project_field": "project_value"
}


# Test setting config at config level
class TestMetricMetaConfigLevel(MetricConfigTests):
@pytest.fixture(scope="class")
def models(self):
return {
"people.sql": models_people_sql,
"metricflow_time_spine.sql": metricflow_time_spine_sql,
"semantic_model_people.yml": semantic_model_people_yml,
"schema.yml": models_people_metrics_yml,
}

def test_meta_metric_config_yaml(self, project):
run_dbt(["parse"])
manifest = get_manifest(project.project_root)
assert "metric.test.number_of_people" in manifest.metrics
assert manifest.metrics.get("metric.test.number_of_people").config.meta == {
"my_meta_config": "config"
}
assert manifest.metrics.get("metric.test.number_of_people").meta == {
"my_meta_config": "config"
}


# Test setting config at metric level- expect to exist in config after parsing
class TestMetricMetaTopLevel(MetricConfigTests):
@pytest.fixture(scope="class")
def models(self):
return {
"people.sql": models_people_sql,
"metricflow_time_spine.sql": metricflow_time_spine_sql,
"semantic_model_people.yml": semantic_model_people_yml,
"schema.yml": models_people_metrics_meta_top_yml,
}

def test_meta_metric_config_yaml(self, project):
run_dbt(["parse"])
manifest = get_manifest(project.project_root)
assert "metric.test.number_of_people" in manifest.metrics
# for backwards compatibility the config level meta gets copied to the top level meta
assert manifest.metrics.get("metric.test.number_of_people").config.meta != {
"my_meta_top": "top"
}
assert manifest.metrics.get("metric.test.number_of_people").meta == {"my_meta_top": "top"}
Loading