Skip to content

Commit

Permalink
[#4252] Serialization error when missing quotes in metrics model ref(…
Browse files Browse the repository at this point in the history
…) call (#4287)
  • Loading branch information
gshank authored Nov 17, 2021
1 parent aea23a4 commit bd950f6
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
### Fixes
- Allow specifying default in Jinja config.get with default keyword ([#4273](https://github.com/dbt-labs/dbt-core/issues/4273), [#4297](https://github.com/dbt-labs/dbt-core/pull/4297))

### Fixes
- Fix serialization error with missing quotes in metrics model ref ([#4252](https://github.com/dbt-labs/dbt-core/issues/4252), [#4287](https://github.com/dbt-labs/dbt-core/pull/4289))

### Under the hood
- Add --indirect-selection parameter to profiles.yml and builtin DBT_ env vars; stringified parameter to enable multi-modal use ([#3997](https://github.com/dbt-labs/dbt-core/issues/3997), [#4270](https://github.com/dbt-labs/dbt-core/pull/4270))
- Fix filesystem searcher test failure on Python 3.9 ([#3689](https://github.com/dbt-labs/dbt-core/issues/3689), [#4271](https://github.com/dbt-labs/dbt-core/pull/4271))
Expand Down
16 changes: 15 additions & 1 deletion core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
)
from dbt.exceptions import (
CompilationException,
ParsingException,
InternalException,
ValidationException,
RuntimeException,
Expand Down Expand Up @@ -1388,11 +1389,24 @@ def generate_parse_exposure(

class MetricRefResolver(BaseResolver):
def __call__(self, *args) -> str:
if len(args) not in (1, 2):
package = None
if len(args) == 1:
name = args[0]
elif len(args) == 2:
package, name = args
else:
ref_invalid_args(self.model, args)
self.validate_args(name, package)
self.model.refs.append(list(args))
return ''

def validate_args(self, name, package):
if not isinstance(name, str):
raise ParsingException(
f'In a metrics section in {self.model.original_file_path} '
f'the name argument to ref() must be a string'
)


def generate_parse_metrics(
metric: ParsedMetric,
Expand Down
3 changes: 3 additions & 0 deletions test/integration/075_metrics_tests/invalid-models/people.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
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'
3 changes: 3 additions & 0 deletions test/integration/075_metrics_tests/models/people.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
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
30 changes: 30 additions & 0 deletions test/integration/075_metrics_tests/models/people_metrics.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
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'
59 changes: 59 additions & 0 deletions test/integration/075_metrics_tests/test_metrics.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
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"])

0 comments on commit bd950f6

Please sign in to comment.