diff --git a/dbt/compilation.py b/dbt/compilation.py index 0af3539c21e..bb14c653f69 100644 --- a/dbt/compilation.py +++ b/dbt/compilation.py @@ -248,8 +248,9 @@ def compile(self): self._check_resource_uniqueness(manifest) resource_fqns = manifest.get_resource_fqns() + disabled_fqns = [n.fqn for n in manifest.disabled] self.config.warn_for_unused_resource_config_paths(resource_fqns, - manifest.disabled) + disabled_fqns) self.link_graph(linker, manifest) diff --git a/dbt/context/runtime.py b/dbt/context/runtime.py index 7ae496f40c8..ca2b95de502 100644 --- a/dbt/context/runtime.py +++ b/dbt/context/runtime.py @@ -3,7 +3,7 @@ import dbt.clients.jinja import dbt.context.common import dbt.flags -import dbt.parser +from dbt.parser import ParserUtils from dbt.logger import GLOBAL_LOGGER as logger # noqa @@ -26,14 +26,14 @@ def do_ref(*args): else: dbt.exceptions.ref_invalid_args(model, args) - target_model = dbt.parser.ParserUtils.resolve_ref( + target_model = ParserUtils.resolve_ref( manifest, target_model_name, target_model_package, current_project, model.get('package_name')) - if target_model is None: + if target_model is None or target_model is ParserUtils.DISABLED: dbt.exceptions.ref_target_not_found( model, target_model_name, diff --git a/dbt/contracts/graph/manifest.py b/dbt/contracts/graph/manifest.py index 2e43a178ece..815f42fc7ac 100644 --- a/dbt/contracts/graph/manifest.py +++ b/dbt/contracts/graph/manifest.py @@ -81,14 +81,8 @@ 'docs': PARSED_DOCUMENTATIONS_CONTRACT, 'disabled': { 'type': 'array', - 'items': { - 'type': 'array', - 'items': { - 'type': 'string', - }, - 'description': 'A disabled node FQN', - }, - 'description': 'An array of disabled node FQNs', + 'items': PARSED_NODE_CONTRACT, + 'description': 'An array of disabled nodes', }, 'generated_at': { 'type': 'string', @@ -221,9 +215,13 @@ def serialize(self): 'child_map': forward_edges, 'generated_at': self.generated_at, 'metadata': self.metadata, - 'disabled': self.disabled, + 'disabled': [v.serialize() for v in self.disabled], } + def find_disabled_by_name(self, name, package=None): + return dbt.utils.find_in_list_by_name(self.disabled, name, package, + NodeType.refable()) + def _find_by_name(self, name, package, subgraph, nodetype): """ diff --git a/dbt/exceptions.py b/dbt/exceptions.py index 0aabeb9b372..c4d5130322c 100644 --- a/dbt/exceptions.py +++ b/dbt/exceptions.py @@ -271,27 +271,46 @@ def doc_target_not_found(model, target_doc_name, target_doc_package): raise_compiler_error(msg, model) -def get_target_not_found_msg(model, target_model_name, target_model_package, - path=None): +def _get_target_failure_msg(model, target_model_name, target_model_package, + include_path, reason): target_package_string = '' - if target_model_package is not None: target_package_string = "in package '{}' ".format(target_model_package) source_path_string = '' - if path is not None: - source_path_string = ' ({})'.format(path) + if include_path: + source_path_string = ' ({})'.format(model.get('original_file_path')) + + return ("Model '{}'{} depends on model '{}' {}which {}" + .format(model.get('unique_id'), + source_path_string, + target_model_name, + target_package_string, + reason)) + + +def get_target_disabled_msg(model, target_model_name, target_model_package): + return _get_target_failure_msg(model, target_model_name, + target_model_package, include_path=True, + reason='is disabled') + + +def get_target_not_found_msg(model, target_model_name, target_model_package): + return _get_target_failure_msg(model, target_model_name, + target_model_package, include_path=True, + reason='was not found') + - return ("Model '{}'{} depends on model '{}' {}which was not found or is" - " disabled".format(model.get('unique_id'), - source_path_string, - target_model_name, - target_package_string)) +def get_target_not_found_or_disabled_msg(model, target_model_name, + target_model_package): + return _get_target_failure_msg(model, target_model_name, + target_model_package, include_path=False, + reason='was not found or is disabled') def ref_target_not_found(model, target_model_name, target_model_package): - msg = get_target_not_found_msg(model, target_model_name, - target_model_package) + msg = get_target_not_found_or_disabled_msg(model, target_model_name, + target_model_package) raise_compiler_error(msg, model) diff --git a/dbt/parser/base_sql.py b/dbt/parser/base_sql.py index bbb3670f691..ff8b18ad1b5 100644 --- a/dbt/parser/base_sql.py +++ b/dbt/parser/base_sql.py @@ -99,7 +99,7 @@ def parse_sql_nodes(cls, nodes, root_project, projects, # Ignore disabled nodes if not node_parsed['config']['enabled']: - disabled.append(node_parsed['fqn']) + disabled.append(node_parsed) continue # Check for duplicate model names diff --git a/dbt/parser/util.py b/dbt/parser/util.py index 85c5a84ec66..52d3aaf0bd9 100644 --- a/dbt/parser/util.py +++ b/dbt/parser/util.py @@ -35,6 +35,8 @@ def do_docs(*args): class ParserUtils(object): + DISABLED = object() + @classmethod def resolve_ref(cls, manifest, target_model_name, target_model_package, current_project, node_package): @@ -44,6 +46,7 @@ def resolve_ref(cls, manifest, target_model_name, target_model_package, target_model_package) target_model = None + disabled_target = None # first pass: look for models in the current_project # second pass: look for models in the node's package @@ -58,6 +61,15 @@ def resolve_ref(cls, manifest, target_model_name, target_model_package, if target_model is not None and dbt.utils.is_enabled(target_model): return target_model + + # it's possible that the node is disabled + if disabled_target is None: + disabled_target = manifest.find_disabled_by_name( + target_model_name, candidate + ) + + if disabled_target is not None: + return cls.DISABLED return None @classmethod @@ -147,12 +159,14 @@ def process_refs(cls, manifest, current_project): current_project, node.get('package_name')) - if target_model is None: + if target_model is None or target_model is cls.DISABLED: # This may raise. Even if it doesn't, we don't want to add # this node to the graph b/c there is no destination node node.config['enabled'] = False dbt.utils.invalid_ref_fail_unless_test( - node, target_model_name, target_model_package) + node, target_model_name, target_model_package, + disabled=(target_model is cls.DISABLED) + ) continue diff --git a/dbt/utils.py b/dbt/utils.py index 89b2f0d5513..2281a337a5c 100644 --- a/dbt/utils.py +++ b/dbt/utils.py @@ -100,6 +100,30 @@ def find_by_name(flat_graph, target_name, target_package, subgraph, nodetype) +def id_matches(unique_id, target_name, target_package, nodetypes, model): + """Return True if the unique ID matches the given name, package, and type. + + If package is None, any package is allowed. + nodetypes should be a container of NodeTypes that implements the 'in' + operator. + """ + node_parts = unique_id.split('.') + if len(node_parts) != 3: + node_type = model.get('resource_type', 'node') + msg = "{} names cannot contain '.' characters".format(node_type) + dbt.exceptions.raise_compiler_error(msg, model) + + resource_type, package_name, node_name = node_parts + + if resource_type not in nodetypes: + return False + + if target_name != node_name: + return False + + return target_package is None or target_package == package_name + + def find_in_subgraph_by_name(subgraph, target_name, target_package, nodetype): """Find an entry in a subgraph by name. Any mapping that implements .items() and maps unique id -> something can be used as the subgraph. @@ -110,18 +134,17 @@ def find_in_subgraph_by_name(subgraph, target_name, target_package, nodetype): You can use `None` for the package name as a wildcard. """ for name, model in subgraph.items(): - node_parts = name.split('.') - if len(node_parts) != 3: - node_type = model.get('resource_type', 'node') - msg = "{} names cannot contain '.' characters".format(node_type) - dbt.exceptions.raise_compiler_error(msg, model) - - resource_type, package_name, node_name = node_parts - - if (resource_type in nodetype and - ((target_name == node_name) and - (target_package is None or - target_package == package_name))): + if id_matches(name, target_name, target_package, nodetype, model): + return model + + return None + + +def find_in_list_by_name(haystack, target_name, target_package, nodetype): + """Find an entry in the given list by name.""" + for model in haystack: + name = model.get('unique_id') + if id_matches(name, target_name, target_package, nodetype, model): return model return None @@ -423,15 +446,29 @@ def __get__(self, obj, objtype): return functools.partial(self.__call__, obj) +def invalid_ref_test_message(node, target_model_name, target_model_package, + disabled): + if disabled: + msg = dbt.exceptions.get_target_disabled_msg( + node, target_model_name, target_model_package + ) + else: + msg = dbt.exceptions.get_target_not_found_msg( + node, target_model_name, target_model_package + ) + return 'WARNING: {}'.format(msg) + + def invalid_ref_fail_unless_test(node, target_model_name, - target_model_package): + target_model_package, disabled): if node.get('resource_type') == NodeType.Test: - warning = dbt.exceptions.get_target_not_found_msg( - node, - target_model_name, - target_model_package, - node.get('original_file_path')) - logger.warning("WARNING: {}".format(warning)) + msg = invalid_ref_test_message(node, target_model_name, + target_model_package, disabled) + if disabled: + logger.debug(msg) + else: + logger.warning(msg) + else: dbt.exceptions.ref_target_not_found( node, diff --git a/test/unit/test_parser.py b/test/unit/test_parser.py index 226aa9a5533..6114d3399b4 100644 --- a/test/unit/test_parser.py +++ b/test/unit/test_parser.py @@ -1202,6 +1202,7 @@ def test__other_project_config(self): 'materialized': 'ephemeral' }) + sort_config = self.model_config.copy() sort_config.update({ 'enabled': False, @@ -1319,7 +1320,53 @@ def test__other_project_config(self): description='', columns={} ), - }, [['snowplow', 'disabled'], ['snowplow', 'views', 'package']]) + }, + [ + ParsedNode( + name='disabled', + resource_type='model', + package_name='snowplow', + path='disabled.sql', + original_file_path='disabled.sql', + root_path=get_os_path('/usr/src/app'), + raw_sql=("select * from events"), + schema='analytics', + refs=[], + depends_on={ + 'nodes': [], + 'macros': [] + }, + config=disabled_config, + tags=[], + empty=False, + alias='disabled', + unique_id='model.snowplow.disabled', + fqn=['snowplow', 'disabled'], + columns={} + ), + ParsedNode( + name='package', + resource_type='model', + package_name='snowplow', + path=get_os_path('views/package.sql'), + original_file_path=get_os_path('views/package.sql'), + root_path=get_os_path('/usr/src/app'), + raw_sql=("select * from events"), + schema='analytics', + refs=[], + depends_on={ + 'nodes': [], + 'macros': [] + }, + config=sort_config, + tags=[], + empty=False, + alias='package', + unique_id='model.snowplow.package', + fqn=['snowplow', 'views', 'package'], + columns={} + ) + ]) ) def test__simple_schema_v1_test(self):