PR feedback

Handle empty vars dict
Check sources for unused configs
  - add a test
Warn when dbt finds "vars" in "models", "seeds", etc blocks
  - add a test
Clean up enabled/disabled code to share between sources and nodes
  - only log downstream tests at debug level when a source is disabled
  - include the offending project name in the v1 deprecation message
  - Fix tests that care about error messages, add new ones

core/dbt/config/project.py

@@ -435,7 +435,11 @@ def from_project_config(
             seeds = cfg.seeds
             snapshots = cfg.snapshots
             sources = cfg.sources
-            vars_value = V2VarProvider(cfg.vars)
+            if cfg.vars is None:
+                vars_dict: Dict[str, Any] = {}
+            else:
+                vars_dict = cfg.vars
+            vars_value = V2VarProvider(vars_dict)
         else:
             raise ValidationError(
                 f'Got unsupported config_version={cfg.config_version}'

core/dbt/config/runtime.py

@@ -229,6 +229,12 @@ def _get_v2_config_paths(
     ) -> PathSet:
         for key, value in config.items():
             if isinstance(value, dict) and not key.startswith('+'):
+                if key == 'vars':
+                    warn_or_error(
+                        f'Found a "vars" dictionary in a config block for a '
+                        f'dbt_project.yml file with config-version 2 '
+                        f'({self.project_root}/dbt_project.yml)'
+                    )
                 self._get_v2_config_paths(value, path + (key,), paths)
             else:
                 paths.add(path)
@@ -278,6 +284,7 @@ def get_resource_config_paths(self) -> Dict[str, PathSet]:
             'models': self._get_config_paths(self.models),
             'seeds': self._get_config_paths(self.seeds),
             'snapshots': self._get_config_paths(self.snapshots),
+            'sources': self._get_config_paths(self.sources),
         }
 
     def get_unused_resource_config_paths(

core/dbt/context/context_config.py

@@ -137,7 +137,6 @@ def _update_from_config(
             validate=validate
         )
 
-    # TODO: is fqn[0] always the project name?
     def calculate_node_config(
         self,
         config_calls: List[Dict[str, Any]],

core/dbt/context/providers.py

@@ -315,6 +315,7 @@ def resolve(
                 self.model,
                 target_name,
                 target_package,
+                disabled=isinstance(target_model, Disabled),
             )
         self.validate(target_model, target_name, target_package)
         return self.create_relation(target_model, target_name)
@@ -385,7 +386,7 @@ def resolve(self, source_name: str, table_name: str):
             self.model.package_name,
         )
 
-        if target_source is None:
+        if target_source is None or isinstance(target_source, Disabled):
             source_target_not_found(
                 self.model,
                 source_name,

core/dbt/contracts/graph/manifest.py

@@ -14,8 +14,8 @@
 from hologram import JsonSchemaMixin
 
 from dbt.contracts.graph.parsed import (
-    ParsedNode, ParsedMacro, ParsedDocumentation, ParsedNodePatch,
-    ParsedMacroPatch, ParsedSourceDefinition
+    ParsedMacro, ParsedDocumentation, ParsedNodePatch, ParsedMacroPatch,
+    ParsedSourceDefinition
 )
 from dbt.contracts.graph.compiled import CompileResultNode, NonSourceNode
 from dbt.contracts.util import Writable, Replaceable
@@ -383,9 +383,24 @@ def search(self, haystack: Iterable[N]) -> Optional[N]:
         return None
 
 
+D = TypeVar('D')
+
+
 @dataclass
-class Disabled:
-    target: ParsedNode
+class Disabled(Generic[D]):
+    target: D
+
+
+MaybeParsedSource = Optional[Union[
+    ParsedSourceDefinition,
+    Disabled[ParsedSourceDefinition],
+]]
+
+
+MaybeNonSource = Optional[Union[
+    NonSourceNode,
+    Disabled[NonSourceNode]
+]]
 
 
 @dataclass
@@ -396,7 +411,7 @@ class Manifest:
     macros: MutableMapping[str, ParsedMacro]
     docs: MutableMapping[str, ParsedDocumentation]
     generated_at: datetime
-    disabled: List[ParsedNode]
+    disabled: List[CompileResultNode]
     files: MutableMapping[str, SourceFile]
     metadata: ManifestMetadata = field(default_factory=ManifestMetadata)
     flat_graph: Dict[str, Any] = field(default_factory=dict)
@@ -448,13 +463,23 @@ def build_flat_graph(self):
 
     def find_disabled_by_name(
         self, name: str, package: Optional[str] = None
-    ) -> Optional[ParsedNode]:
+    ) -> Optional[NonSourceNode]:
         searcher: NameSearcher = NameSearcher(
             name, package, NodeType.refable()
         )
         result = searcher.search(self.disabled)
+        return result
+
+    def find_disabled_source_by_name(
+        self, source_name: str, table_name: str, package: Optional[str] = None
+    ) -> Optional[ParsedSourceDefinition]:
+        search_name = f'{source_name}.{table_name}'
+        searcher: NameSearcher = NameSearcher(
+            search_name, package, [NodeType.Source]
+        )
+        result = searcher.search(self.disabled)
         if result is not None:
-            assert isinstance(result, ParsedNode)
+            assert isinstance(result, ParsedSourceDefinition)
         return result
 
     def find_docs_by_name(
@@ -597,9 +622,7 @@ def find_materialization_macro_by_name(
     def get_resource_fqns(self) -> Mapping[str, PathSet]:
         resource_fqns: Dict[str, Set[Tuple[str, ...]]] = {}
         for unique_id, node in self.nodes.items():
-            if node.resource_type == NodeType.Source:
-                continue  # sources have no FQNs and can't be configured
-            resource_type_plural = node.resource_type + 's'
+            resource_type_plural = node.resource_type.pluralize()
             if resource_type_plural not in resource_fqns:
                 resource_fqns[resource_type_plural] = set()
             resource_fqns[resource_type_plural].add(tuple(node.fqn))
@@ -761,14 +784,14 @@ def resolve_ref(
         target_model_package: Optional[str],
         current_project: str,
         node_package: str,
-    ) -> Optional[Union[NonSourceNode, Disabled]]:
+    ) -> MaybeNonSource:
         if target_model_package is not None:
             return self.find_refable_by_name(
                 target_model_name,
                 target_model_package)
 
-        target_model = None
-        disabled_target = None
+        target_model: Optional[NonSourceNode] = None
+        disabled_target: Optional[NonSourceNode] = None
 
         # first pass: look for models in the current_project
         # second pass: look for models in the node's package
@@ -801,9 +824,12 @@ def resolve_source(
         target_table_name: str,
         current_project: str,
         node_package: str
-    ) -> Optional[ParsedSourceDefinition]:
+    ) -> MaybeParsedSource:
         candidate_targets = [current_project, node_package, None]
-        target_source = None
+
+        target_source: Optional[ParsedSourceDefinition] = None
+        disabled_target: Optional[ParsedSourceDefinition] = None
+
         for candidate in candidate_targets:
             target_source = self.find_source_by_name(
                 target_source_name,
@@ -813,6 +839,13 @@ def resolve_source(
             if target_source is not None and target_source.config.enabled:
                 return target_source
 
+            if disabled_target is None:
+                disabled_target = self.find_disabled_source_by_name(
+                    target_source_name, target_table_name, candidate
+                )
+
+        if disabled_target is not None:
+            return Disabled(disabled_target)
         return None
 
     def resolve_doc(
@@ -861,7 +894,7 @@ class WritableManifest(JsonSchemaMixin, Writable):
             'The docs defined in the dbt project and its dependencies'
         ))
     )
-    disabled: Optional[List[ParsedNode]] = field(metadata=dict(
+    disabled: Optional[List[CompileResultNode]] = field(metadata=dict(
         description='A list of the disabled nodes in the target'
     ))
     generated_at: datetime = field(metadata=dict(

core/dbt/contracts/project.py

@@ -203,8 +203,8 @@ class ProjectV2(HyphenatedJsonSchemaMixin, Replaceable):
     snapshots: Dict[str, Any] = field(default_factory=dict)
     analyses: Dict[str, Any] = field(default_factory=dict)
     sources: Dict[str, Any] = field(default_factory=dict)
-    vars: Dict[str, Any] = field(
-        default_factory=dict,
+    vars: Optional[Dict[str, Any]] = field(
+        default=None,
         metadata=dict(
             description='map project names to their vars override dicts',
         ),

core/dbt/deprecations.py

@@ -99,6 +99,8 @@ class DbtProjectYamlDeprecation(DBTDeprecation):
     has been upgraded to config version 2. A future version of dbt will remove
     support for the existing ("version 1") format.
 
+    dbt found a version 1 dbt_project.yml in the project "{project_name}"
+
     Documentation for dbt_project.yml version 2 can be found here:
 
         DOCS LINK GOES HERE

core/dbt/exceptions.py

@@ -462,8 +462,10 @@ def doc_target_not_found(
     raise_compiler_error(msg, model)
 
 
-def _get_target_failure_msg(model, target_model_name, target_model_package,
-                            include_path, reason):
+def _get_target_failure_msg(
+    model, target_name: str, target_model_package: Optional[str],
+    include_path: bool, reason: str, target_kind: str
+) -> str:
     target_package_string = ''
     if target_model_package is not None:
         target_package_string = "in package '{}' ".format(target_model_package)
@@ -472,52 +474,73 @@ def _get_target_failure_msg(model, target_model_name, target_model_package,
     if include_path:
         source_path_string = ' ({})'.format(model.original_file_path)
 
-    return "{} '{}'{} depends on a node named '{}' {}which {}".format(
+    return "{} '{}'{} depends on a {} named '{}' {}which {}".format(
         model.resource_type.title(),
         model.unique_id,
         source_path_string,
-        target_model_name,
+        target_kind,
+        target_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 get_target_not_found_or_disabled_msg(
+    model, target_model_name: str, target_model_package: Optional[str],
+    disabled: Optional[bool] = None,
+) -> str:
+    if disabled is None:
+        reason = 'was not found or is disabled'
+    elif disabled is True:
+        reason = 'is disabled'
+    else:
+        reason = 'was not found'
+    return _get_target_failure_msg(
+        model, target_model_name, target_model_package, include_path=True,
+        reason=reason, target_kind='node'
+    )
 
 
-def ref_target_not_found(model, target_model_name, target_model_package):
-    msg = get_target_not_found_or_disabled_msg(model, target_model_name,
-                                               target_model_package)
+def ref_target_not_found(
+    model,
+    target_model_name: str,
+    target_model_package: Optional[str],
+    disabled: Optional[bool] = None,
+) -> NoReturn:
+    msg = get_target_not_found_or_disabled_msg(
+        model, target_model_name, target_model_package, disabled
+    )
     raise_compiler_error(msg, model)
 
 
-def source_disabled_message(model, target_name, target_table_name):
-    return ("{} '{}' ({}) depends on source '{}.{}' which was not found"
-            .format(model.resource_type.title(),
-                    model.unique_id,
-                    model.original_file_path,
-                    target_name,
-                    target_table_name))
+def get_source_not_found_or_disabled_msg(
+    model,
+    target_name: str,
+    target_table_name: str,
+    disabled: Optional[bool] = None,
+) -> str:
+    full_name = f'{target_name}.{target_table_name}'
+    if disabled is None:
+        reason = 'was not found or is disabled'
+    elif disabled is True:
+        reason = 'is disabled'
+    else:
+        reason = 'was not found'
+    return _get_target_failure_msg(
+        model, full_name, None, include_path=True,
+        reason=reason, target_kind='source'
+    )
 
 
-def source_target_not_found(model, target_name, target_table_name) -> NoReturn:
-    msg = source_disabled_message(model, target_name, target_table_name)
+def source_target_not_found(
+    model,
+    target_name: str,
+    target_table_name: str,
+    disabled: Optional[bool] = None
+) -> NoReturn:
+    msg = get_source_not_found_or_disabled_msg(
+        model, target_name, target_table_name, disabled
+    )
     raise_compiler_error(msg, model)
 
 

core/dbt/parser/manifest.py

@@ -334,7 +334,10 @@ def load_all(
             projects = root_config.load_dependencies()
             for project in projects.values():
                 if project.config_version == 1:
-                    deprecations.warn('dbt-project-yaml-v1')
+                    deprecations.warn(
+                        'dbt-project-yaml-v1',
+                        project_name=project.project_name,
+                    )
             loader = cls(root_config, projects, macro_hook)
             loader.load(internal_manifest=internal_manifest)
             loader.write_parse_results()
@@ -545,7 +548,7 @@ def process_refs(manifest: Manifest, current_project: str):
 def _process_sources_for_node(
     manifest: Manifest, current_project: str, node: NonSourceNode
 ):
-    target_source = None
+    target_source: Optional[Union[Disabled, ParsedSourceDefinition]] = None
     for source_name, table_name in node.sources:
         target_source = manifest.resolve_source(
             source_name,
@@ -554,13 +557,15 @@ def _process_sources_for_node(
             node.package_name,
         )
 
-        if target_source is None:
+        if target_source is None or isinstance(target_source, Disabled):
             # this folows the same pattern as refs
             node.config.enabled = False
             dbt.utils.invalid_source_fail_unless_test(
                 node,
                 source_name,
-                table_name)
+                table_name,
+                disabled=(isinstance(target_source, Disabled))
+            )
             continue
         target_source_id = target_source.unique_id
         node.depends_on.nodes.append(target_source_id)