Merge pull request #1169 from fishtown-analytics/feature/log-warnings…

…-on-missing-test-refs

log warnings on missing test refs [#968]

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)
 

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,

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):
  """
 

dbt/exceptions.py

@@ -271,21 +271,47 @@ 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):
+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)
 
- return ("Model '{}' depends on model '{}' {}which was not found or is"
- " disabled".format(model.get('unique_id'),
- target_model_name,
- target_package_string))
+ source_path_string = ''
+ if include_path:
+ source_path_string = ' ({})'.format(model.get('original_file_path'))
+
+ return ("{} '{}'{} depends on model '{}' {}which {}"
+ .format(model.get('resource_type').title(),
+ 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')
+
+
+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)
 
 

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

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
 

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,14 +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)
- logger.debug("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,

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):