diff --git a/.changes/unreleased/Fixes-20220426-202104.yaml b/.changes/unreleased/Fixes-20220426-202104.yaml new file mode 100644 index 00000000000..a09201a9ec6 --- /dev/null +++ b/.changes/unreleased/Fixes-20220426-202104.yaml @@ -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" diff --git a/core/dbt/contracts/graph/unparsed.py b/core/dbt/contracts/graph/unparsed.py index 80e956c06e0..066bd02a911 100644 --- a/core/dbt/contracts/graph/unparsed.py +++ b/core/dbt/contracts/graph/unparsed.py @@ -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 @@ -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") diff --git a/test/integration/075_metrics_tests/invalid-models/people.sql b/test/integration/075_metrics_tests/invalid-models/people.sql deleted file mode 100644 index ce58d41a599..00000000000 --- a/test/integration/075_metrics_tests/invalid-models/people.sql +++ /dev/null @@ -1,3 +0,0 @@ -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 diff --git a/test/integration/075_metrics_tests/invalid-models/people_metrics.yml b/test/integration/075_metrics_tests/invalid-models/people_metrics.yml deleted file mode 100644 index a859a15c0b8..00000000000 --- a/test/integration/075_metrics_tests/invalid-models/people_metrics.yml +++ /dev/null @@ -1,30 +0,0 @@ -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' diff --git a/test/integration/075_metrics_tests/models/people.sql b/test/integration/075_metrics_tests/models/people.sql deleted file mode 100644 index ce58d41a599..00000000000 --- a/test/integration/075_metrics_tests/models/people.sql +++ /dev/null @@ -1,3 +0,0 @@ -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 diff --git a/test/integration/075_metrics_tests/models/people_metrics.yml b/test/integration/075_metrics_tests/models/people_metrics.yml deleted file mode 100644 index 763bc0bcafb..00000000000 --- a/test/integration/075_metrics_tests/models/people_metrics.yml +++ /dev/null @@ -1,30 +0,0 @@ -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' diff --git a/test/integration/075_metrics_tests/test_metrics.py b/test/integration/075_metrics_tests/test_metrics.py deleted file mode 100644 index f1cce3521f9..00000000000 --- a/test/integration/075_metrics_tests/test_metrics.py +++ /dev/null @@ -1,59 +0,0 @@ -from test.integration.base import DBTIntegrationTest, use_profile, normalize, get_manifest -from dbt.exceptions import ParsingException - -class BaseMetricTest(DBTIntegrationTest): - - @property - def schema(self): - return "test_075" - - @property - def models(self): - return "models" - - @property - def project_config(self): - return { - 'config-version': 2, - 'seed-paths': ['seeds'], - 'seeds': { - 'quote_columns': False, - }, - } - - @use_profile('postgres') - def test_postgres_simple_metric(self): - # initial run - results = self.run_dbt(["run"]) - self.assertEqual(len(results), 1) - manifest = get_manifest() - metric_ids = list(manifest.metrics.keys()) - expected_metric_ids = ['metric.test.number_of_people', 'metric.test.collective_tenure'] - self.assertEqual(metric_ids, expected_metric_ids) - -class InvalidRefMetricTest(DBTIntegrationTest): - - @property - def schema(self): - return "test_075" - - @property - def models(self): - return "invalid-models" - - @property - def project_config(self): - return { - 'config-version': 2, - 'seed-paths': ['seeds'], - 'seeds': { - 'quote_columns': False, - }, - } - - @use_profile('postgres') - def test_postgres_simple_metric(self): - # initial run - with self.assertRaises(ParsingException): - results = self.run_dbt(["run"]) - diff --git a/tests/functional/metrics/test_metrics.py b/tests/functional/metrics/test_metrics.py new file mode 100644 index 00000000000..30f816a9564 --- /dev/null +++ b/tests/functional/metrics/test_metrics.py @@ -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"])