Skip to content

Commit

Permalink
Ct-65 metrics names with spaces (#5173)
Browse files Browse the repository at this point in the history
* Convert existing metrics test

* add non-failing test for names with spaces

* Raise ParsingException if metrics name contains spaces

* Remove old metrics tests
  • Loading branch information
gshank authored Apr 27, 2022
1 parent 6267572 commit 4e57c51
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 126 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20220426-202104.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Fixes
body: Ensure the metric name does not contain spaces
time: 2022-04-26T20:21:04.360693-04:00
custom:
Author: gshank
Issue: "4572"
PR: "5173"
8 changes: 7 additions & 1 deletion core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

# trigger the PathEncoder
import dbt.helper_types # noqa:F401
from dbt.exceptions import CompilationException
from dbt.exceptions import CompilationException, ParsingException

from dbt.dataclass_schema import dbtClassMixin, StrEnum, ExtensibleDbtClassMixin

Expand Down Expand Up @@ -460,3 +460,9 @@ class UnparsedMetric(dbtClassMixin, Replaceable):
filters: List[MetricFilter] = field(default_factory=list)
meta: Dict[str, Any] = field(default_factory=dict)
tags: List[str] = field(default_factory=list)

@classmethod
def validate(cls, data):
super(UnparsedMetric, cls).validate(data)
if "name" in data and " " in data["name"]:
raise ParsingException(f"Metrics name '{data['name']}' cannot contain spaces")
3 changes: 0 additions & 3 deletions test/integration/075_metrics_tests/invalid-models/people.sql

This file was deleted.

This file was deleted.

3 changes: 0 additions & 3 deletions test/integration/075_metrics_tests/models/people.sql

This file was deleted.

30 changes: 0 additions & 30 deletions test/integration/075_metrics_tests/models/people_metrics.yml

This file was deleted.

59 changes: 0 additions & 59 deletions test/integration/075_metrics_tests/test_metrics.py

This file was deleted.

169 changes: 169 additions & 0 deletions tests/functional/metrics/test_metrics.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import pytest

from dbt.tests.util import run_dbt, get_manifest
from dbt.exceptions import ParsingException


models__people_metrics_yml = """
version: 2
metrics:
- model: "ref('people')"
name: number_of_people
description: Total count of people
label: "Number of people"
type: count
sql: "*"
timestamp: created_at
time_grains: [day, week, month]
dimensions:
- favorite_color
- loves_dbt
meta:
my_meta: 'testing'
- model: "ref('people')"
name: collective_tenure
description: Total number of years of team experience
label: "Collective tenure"
type: sum
sql: tenure
timestamp: created_at
time_grains: [day]
filters:
- field: loves_dbt
operator: is
value: 'true'
"""

models__people_sql = """
select 1 as id, 'Drew' as first_name, 'Banin' as last_name, 'yellow' as favorite_color, true as loves_dbt, 5 as tenure, current_timestamp as created_at
union all
select 1 as id, 'Jeremy' as first_name, 'Cohen' as last_name, 'indigo' as favorite_color, true as loves_dbt, 4 as tenure, current_timestamp as created_at
"""


class TestSimpleMetrics:
@pytest.fixture(scope="class")
def models(self):
return {
"people_metrics.yml": models__people_metrics_yml,
"people.sql": models__people_sql,
}

def test_simple_metric(
self,
project,
):
# initial run
results = run_dbt(["run"])
assert len(results) == 1
manifest = get_manifest(project.project_root)
metric_ids = list(manifest.metrics.keys())
expected_metric_ids = ["metric.test.number_of_people", "metric.test.collective_tenure"]
assert metric_ids == expected_metric_ids


invalid_models__people_metrics_yml = """
version: 2
metrics:
- model: "ref(people)"
name: number_of_people
description: Total count of people
label: "Number of people"
type: count
sql: "*"
timestamp: created_at
time_grains: [day, week, month]
dimensions:
- favorite_color
- loves_dbt
meta:
my_meta: 'testing'
- model: "ref(people)"
name: collective_tenure
description: Total number of years of team experience
label: "Collective tenure"
type: sum
sql: tenure
timestamp: created_at
time_grains: [day]
filters:
- field: loves_dbt
operator: is
value: 'true'
"""


class TestInvalidRefMetrics:
@pytest.fixture(scope="class")
def models(self):
return {
"people_metrics.yml": invalid_models__people_metrics_yml,
"people.sql": models__people_sql,
}

# tests that we get a ParsingException with an invalid model ref, where
# the model name does not have quotes
def test_simple_metric(
self,
project,
):
# initial run
with pytest.raises(ParsingException):
run_dbt(["run"])


names_with_spaces_metrics_yml = """
version: 2
metrics:
- model: "ref('people')"
name: number of people
description: Total count of people
label: "Number of people"
type: count
sql: "*"
timestamp: created_at
time_grains: [day, week, month]
dimensions:
- favorite_color
- loves_dbt
meta:
my_meta: 'testing'
- model: "ref('people')"
name: collective tenure
description: Total number of years of team experience
label: "Collective tenure"
type: sum
sql: tenure
timestamp: created_at
time_grains: [day]
filters:
- field: loves_dbt
operator: is
value: 'true'
"""


class TestNamesWithSpaces:
@pytest.fixture(scope="class")
def models(self):
return {
"people_metrics.yml": names_with_spaces_metrics_yml,
"people.sql": models__people_sql,
}

def test_names_with_spaces(self, project):
with pytest.raises(ParsingException):
run_dbt(["run"])

0 comments on commit 4e57c51

Please sign in to comment.