Skip to content

Commit

Permalink
Merge pull request #1514 from fishtown-analytics/fix/dbt-ls-selectors
Browse files Browse the repository at this point in the history
Fix dbt ls selectors
  • Loading branch information
beckjake authored Jun 13, 2019
2 parents 24adb74 + 5833acb commit f0635a0
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 37 deletions.
1 change: 1 addition & 0 deletions core/dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ class Hook(APIObject):
HAS_UNIQUE_ID_CONTRACT,
HAS_DOCREFS_CONTRACT,
HAS_RELATION_METADATA_CONTRACT,
HAS_FQN_CONTRACT,
{
'description': (
'A source table definition, as parsed from the one provided in the'
Expand Down
14 changes: 10 additions & 4 deletions core/dbt/graph/selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,19 +171,25 @@ def get_nodes_by_tag(self, graph, tag_name):
def get_nodes_by_source(self, graph, source_full_name):
"""yields nodes from graph are the specified source."""
parts = source_full_name.split('.')
target_package = SELECTOR_GLOB
if len(parts) == 1:
target_source, target_table = parts[0], None
elif len(parts) == 2:
target_source, target_table = parts
else: # len(parts) > 2 or len(parts) == 0
elif len(parts) == 3:
target_package, target_source, target_table = parts
else: # len(parts) > 3 or len(parts) == 0
msg = (
'Invalid source selector value "{}". Sources must be of the '
'form `${{source_name}}` or '
'`${{source_name}}.${{target_name}}`'
'form `${{source_name}}`, '
'`${{source_name}}.${{target_name}}`, or '
'`${{package_name}}.${{source_name}}.${{target_name}}'
).format(source_full_name)
raise dbt.exceptions.RuntimeException(msg)

for node, real_node in self.source_nodes(graph):
if target_package not in (real_node.package_name, SELECTOR_GLOB):
continue
if target_source not in (real_node.source_name, SELECTOR_GLOB):
continue
if target_table in (None, real_node.name, SELECTOR_GLOB):
Expand Down Expand Up @@ -294,7 +300,7 @@ def _is_match(self, node_name, resource_types, tags, required):
def get_selected(self, include, exclude, resource_types, tags, required):
graph = self.linker.graph

include = coalesce(include, ['*'])
include = coalesce(include, ['fqn:*', 'source:*'])
exclude = coalesce(exclude, [])
tags = coalesce(tags, [])

Expand Down
3 changes: 2 additions & 1 deletion core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,8 @@ def generate_source_node(self, source, table, path, package_name, root_dir,
loaded_at_field=loaded_at_field,
freshness=freshness,
quoting=quoting,
resource_type=NodeType.Source
resource_type=NodeType.Source,
fqn=[package_name, source.name, table.name]
)

def parse_source_table(self, source, table, path, package_name, root_dir):
Expand Down
5 changes: 3 additions & 2 deletions core/dbt/task/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ def _iterate_selected_nodes(self):

def generate_selectors(self):
for node in self._iterate_selected_nodes():
selector = '.'.join(node.fqn)
if node.resource_type == NodeType.Source:
yield 'source:{}'.format(node.unique_id)
yield 'source:{}'.format(selector)
else:
yield node.unique_id
yield selector

def generate_names(self):
for node in self._iterate_selected_nodes():
Expand Down
10 changes: 5 additions & 5 deletions test/integration/029_docs_generate_tests/test_docs_generate.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
from __future__ import unicode_literals
import json
from numbers import Integral
import os
from datetime import datetime, timedelta
from datetime import datetime
from mock import ANY, patch

from test.integration.base import DBTIntegrationTest, use_profile, AnyFloat, \
Expand All @@ -23,7 +22,7 @@ def __eq__(self, other):
return self.expected == other.replace('\r', '')

def __repr__(self):
return 'LineIndifferent("{}")'.format(self.expected)
return 'LineIndifferent({!r})'.format(self.expected)

def __str__(self):
return self.__repr__()
Expand Down Expand Up @@ -471,7 +470,7 @@ def expected_postgres_references_catalog(self):
'owner': role,
},
"stats": stats,
'columns': seed_columns
'columns': seed_columns,
},
}

Expand Down Expand Up @@ -1289,7 +1288,8 @@ def expected_postgres_references_manifest(self, model_database=None):
'schema': my_schema_name,
'source_description': 'My source',
'source_name': 'my_source',
'unique_id': 'source.test.my_source.my_table'
'unique_id': 'source.test.my_source.my_table',
'fqn': ['test', 'my_source', 'my_table'],
}
},
'docs': {
Expand Down
74 changes: 50 additions & 24 deletions test/integration/047_dbt_ls_test/test_ls.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def project_config(self):
'snapshot-paths': [self.dir('snapshots')],
'macro-paths': [self.dir('macros')],
'data-paths': [self.dir('data')],
'test-paths': [self.dir('tests')],
}

def run_dbt_ls(self, args=None, expect_pass=True):
Expand Down Expand Up @@ -58,7 +59,7 @@ def expect_given_output(self, args, expectations):
def expect_snapshot_output(self):
expectations = {
'name': 'my_snapshot',
'selector': 'snapshot.test.my_snapshot',
'selector': 'test.my_snapshot',
'json': {
'name': 'my_snapshot',
'package_name': 'test',
Expand Down Expand Up @@ -89,10 +90,10 @@ def expect_snapshot_output(self):

def expect_analyses_output(self):
expectations = {
'name': 'analysis',
'selector': 'analysis.test.analysis',
'name': 'a',
'selector': 'test.analysis.a',
'json': {
'name': 'analysis',
'name': 'a',
'package_name': 'test',
'depends_on': {'nodes': [], 'macros': []},
'tags': [],
Expand All @@ -107,17 +108,17 @@ def expect_analyses_output(self):
'column_types': {},
'persist_docs': {},
},
'alias': 'analysis',
'alias': 'a',
'resource_type': 'analysis',
},
'path': self.dir('analyses/analysis.sql'),
'path': self.dir('analyses/a.sql'),
}
self.expect_given_output(['--resource-type', 'analysis'], expectations)

def expect_model_output(self):
expectations = {
'name': ('inner', 'outer'),
'selector': ('model.test.inner', 'model.test.outer'),
'selector': ('test.sub.inner', 'test.outer'),
'json': (
{
'name': 'inner',
Expand Down Expand Up @@ -165,7 +166,7 @@ def expect_model_output(self):
def expect_source_output(self):
expectations = {
'name': 'my_source.my_table',
'selector': 'source:source.test.my_source.my_table',
'selector': 'source:test.my_source.my_table',
'json': {
'package_name': 'test',
'name': 'my_table',
Expand All @@ -181,7 +182,7 @@ def expect_source_output(self):
def expect_seed_output(self):
expectations = {
'name': 'seed',
'selector': 'seed.test.seed',
'selector': 'test.seed',
'json': {
'name': 'seed',
'package_name': 'test',
Expand All @@ -207,8 +208,8 @@ def expect_seed_output(self):

def expect_test_output(self):
expectations = {
'name': ('not_null_outer_id', 'unique_outer_id'),
'selector': ('test.test.not_null_outer_id', 'test.test.unique_outer_id'),
'name': ('not_null_outer_id', 't', 'unique_outer_id'),
'selector': ('test.schema_test.not_null_outer_id', 'test.data_test.t', 'test.schema_test.unique_outer_id'),
'json': (
{
'name': 'not_null_outer_id',
Expand All @@ -231,6 +232,26 @@ def expect_test_output(self):
'resource_type': 'test',

},
{
'name': 't',
'package_name': 'test',
'depends_on': {'nodes': [], 'macros': []},
'tags': ['data'],
'config': {
'enabled': True,
'materialized': 'view',
'post-hook': [],
'severity': 'ERROR',
'tags': [],
'pre-hook': [],
'quoting': {},
'vars': {},
'column_types': {},
'persist_docs': {},
},
'alias': 't',
'resource_type': 'test',
},
{
'name': 'unique_outer_id',
'package_name': 'test',
Expand All @@ -252,21 +273,26 @@ def expect_test_output(self):
'resource_type': 'test',
},
),
'path': (self.dir('models/schema.yml'), self.dir('models/schema.yml')),
'path': (self.dir('models/schema.yml'), self.dir('tests/t.sql'), self.dir('models/schema.yml')),
}
self.expect_given_output(['--resource-type', 'test'], expectations)

def expect_all_output(self):
# tests have their type inserted into their fqn, after the package
# but models don't! they just have (package.name)
# sources are like models - (package.source_name.table_name)
expected_default = {
'snapshot.test.my_snapshot',
'model.test.inner',
'model.test.outer',
'seed.test.seed',
'source:source.test.my_source.my_table',
'test.test.not_null_outer_id',
'test.test.unique_outer_id',
'test.my_snapshot',
'test.sub.inner',
'test.outer',
'test.seed',
'source:test.my_source.my_table',
'test.schema_test.not_null_outer_id',
'test.schema_test.unique_outer_id',
'test.data_test.t',
}
expected_all = expected_default | {'analysis.test.analysis'}
# analyses have their type inserted into their fqn like tests
expected_all = expected_default | {'test.analysis.a'}

results = self.run_dbt_ls(['--resource-type', 'all', '--select', '*', 'source:*'])
self.assertEqual(set(results), expected_all)
Expand All @@ -281,18 +307,18 @@ def expect_all_output(self):

def expect_select(self):
results = self.run_dbt_ls(['--resource-type', 'test', '--select', 'outer'])
self.assertEqual(set(results), {'test.test.not_null_outer_id', 'test.test.unique_outer_id'})
self.assertEqual(set(results), {'test.schema_test.not_null_outer_id', 'test.schema_test.unique_outer_id'})

self.run_dbt_ls(['--resource-type', 'test', '--select', 'inner'], expect_pass=False)

results = self.run_dbt_ls(['--resource-type', 'test', '--select', '+inner'])
self.assertEqual(set(results), {'test.test.not_null_outer_id', 'test.test.unique_outer_id'})
self.assertEqual(set(results), {'test.schema_test.not_null_outer_id', 'test.schema_test.unique_outer_id'})

results = self.run_dbt_ls(['--resource-type', 'model', '--select', 'outer+'])
self.assertEqual(set(results), {'model.test.outer', 'model.test.inner'})
self.assertEqual(set(results), {'test.outer', 'test.sub.inner'})

results = self.run_dbt_ls(['--resource-type', 'model', '--exclude', 'inner'])
self.assertEqual(set(results), {'model.test.outer'})
self.assertEqual(set(results), {'test.outer'})

@use_profile('postgres')
def test_postgres_ls(self):
Expand Down
1 change: 1 addition & 0 deletions test/integration/047_dbt_ls_test/tests/t.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select 1 as id limit 0
3 changes: 2 additions & 1 deletion test/unit/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ def setUp(self):
quoting={
'schema': True,
'identifier': False,
}
},
fqn=['root', 'my_source', 'my_table']
)

self._expected_source_tests = [
Expand Down

0 comments on commit f0635a0

Please sign in to comment.