From f92f38961244149eb34cead3ffb64b3e7375d4bd Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 15 Jan 2020 09:28:38 -0700 Subject: [PATCH 1/6] Make dbt support documentation for snapshots and seeds (and analyses) Add a deprecation warning for snapshots/seeds documented under "models" Add a bunch of tests --- core/dbt/contracts/graph/manifest.py | 18 +- core/dbt/contracts/graph/parsed.py | 10 +- core/dbt/contracts/graph/unparsed.py | 13 +- core/dbt/deprecations.py | 13 + core/dbt/node_types.py | 18 ++ core/dbt/parser/manifest.py | 8 +- core/dbt/parser/results.py | 5 + core/dbt/parser/schema_test_builders.py | 10 +- core/dbt/parser/schemas.py | 274 ++++++++++-------- core/dbt/utils.py | 21 +- .../models/schema.yml | 2 +- .../005_simple_seed_test/models-bq/schema.yml | 2 +- .../005_simple_seed_test/models-pg/schema.yml | 2 +- .../005_simple_seed_test/models-rs/schema.yml | 2 +- .../models-snowflake/schema.yml | 2 +- .../012_deprecation_tests/data/seed.csv | 3 + .../models-key-mismatch/schema.yml | 4 + .../test_deprecations.py | 32 +- .../014_hook_tests/seed-models-bq/schema.yml | 2 +- .../014_hook_tests/seed-models/schema.yml | 2 +- .../test-snapshot-models/schema.yml | 2 +- .../029_docs_generate_tests/models/schema.yml | 14 + .../test_docs_generate.py | 76 ++++- test/unit/test_contracts_graph_parsed.py | 18 +- test/unit/test_contracts_graph_unparsed.py | 38 ++- test/unit/test_parser.py | 45 +-- 26 files changed, 436 insertions(+), 200 deletions(-) create mode 100644 test/integration/012_deprecation_tests/data/seed.csv create mode 100644 test/integration/012_deprecation_tests/models-key-mismatch/schema.yml diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index 3ee4b52a4dc..49e67e7e1b4 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -18,6 +18,7 @@ from dbt.include.global_project import PACKAGES from dbt.logger import GLOBAL_LOGGER as logger from dbt.node_types import NodeType +from dbt import deprecations from dbt import tracking import dbt.utils @@ -439,12 +440,25 @@ def patch_nodes(self, patches): # nodes looking for matching names. We could use _find_by_name if we # were ok with doing an O(n*m) search (one nodes scan per patch) for node in self.nodes.values(): - if node.resource_type != NodeType.Model: + if node.resource_type == NodeType.Source: continue patch = patches.pop(node.name, None) if not patch: continue - node.patch(patch) + if node.resource_type.pluralize() == patch.yaml_key: + node.patch(patch) + elif patch.yaml_key == 'models': + deprecations.warn( + 'models-key-mismatch', patch=patch, node=node + ) + else: + raise_compiler_error( + f'patch instruction in {patch.original_file_path} under ' + f'key "{patch.yaml_key}" was for node "{node.name}", but ' + f'the node with the same name (from ' + f'{node.original_file_path}) had resource type ' + f'"{node.resource_type}"' + ) # log debug-level warning about nodes we couldn't find if patches: diff --git a/core/dbt/contracts/graph/parsed.py b/core/dbt/contracts/graph/parsed.py index db07683381a..fc32e9d0552 100644 --- a/core/dbt/contracts/graph/parsed.py +++ b/core/dbt/contracts/graph/parsed.py @@ -22,7 +22,7 @@ from dbt.contracts.graph.unparsed import ( UnparsedNode, UnparsedMacro, UnparsedDocumentationFile, Quoting, UnparsedBaseNode, FreshnessThreshold, ExternalTable, - AdditionalPropertiesAllowed + AdditionalPropertiesAllowed, HasYamlMetadata ) from dbt.contracts.util import Replaceable, list_str from dbt.logger import GLOBAL_LOGGER as logger # noqa @@ -173,16 +173,17 @@ def is_ephemeral_model(self): def depends_on_nodes(self): return self.depends_on.nodes - def patch(self, patch): + def patch(self, patch: 'ParsedNodePatch'): """Given a ParsedNodePatch, add the new information to the node.""" # explicitly pick out the parts to update so we don't inadvertently # step on the model name or anything - self.patch_path = patch.original_file_path + self.patch_path: Optional[str] = patch.original_file_path self.description = patch.description self.columns = patch.columns self.docrefs = patch.docrefs self.meta = patch.meta if dbt.flags.STRICT_MODE: + assert isinstance(self, JsonSchemaMixin) self.to_dict(validate=True) def get_materialization(self): @@ -452,10 +453,9 @@ def json_schema(cls, embeddable=False): # regular parsed node. Note that description and columns must be present, but # may be empty. @dataclass -class ParsedNodePatch(JsonSchemaMixin, Replaceable): +class ParsedNodePatch(HasYamlMetadata, Replaceable): name: str description: str - original_file_path: str columns: Dict[str, ColumnInfo] docrefs: List[Docref] meta: Dict[str, Any] diff --git a/core/dbt/contracts/graph/unparsed.py b/core/dbt/contracts/graph/unparsed.py index 855b7190a5f..73ded4dfe16 100644 --- a/core/dbt/contracts/graph/unparsed.py +++ b/core/dbt/contracts/graph/unparsed.py @@ -78,7 +78,14 @@ class NodeDescription(NamedTested): @dataclass -class UnparsedNodeUpdate(ColumnDescription, NodeDescription): +class HasYamlMetadata(JsonSchemaMixin): + original_file_path: str + yaml_key: str + package_name: str + + +@dataclass +class UnparsedNodeUpdate(ColumnDescription, NodeDescription, HasYamlMetadata): def __post_init__(self): NodeDescription.__post_init__(self) @@ -219,6 +226,10 @@ class UnparsedSourceDefinition(JsonSchemaMixin, Replaceable): loaded_at_field: Optional[str] = None tables: List[UnparsedSourceTableDefinition] = field(default_factory=list) + @property + def yaml_key(self) -> 'str': + return 'sources' + @dataclass class UnparsedDocumentationFile(JsonSchemaMixin, Replaceable): diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index 972732c84ea..a6101b8e8c1 100644 --- a/core/dbt/deprecations.py +++ b/core/dbt/deprecations.py @@ -96,6 +96,18 @@ class ColumnQuotingDeprecation(DBTDeprecation): ''' +class ModelsKeyNonModelDeprecation(DBTDeprecation): + _name = 'models-key-mismatch' + + _description = ''' + patch instruction in {patch.original_file_path} under key + "models" was for node "{node.name}", but the node with the same + name (from {node.original_file_path}) had resource type + "{node.resource_type}". Nodes should be described under their resource + specific key. Support for this will be removed in a future release. + '''.strip() + + _adapter_renamed_description = """\ The adapter function `adapter.{old_name}` is deprecated and will be removed in a future release of dbt. Please use `adapter.{new_name}` instead. @@ -136,6 +148,7 @@ def warn(name, *args, **kwargs): MaterializationReturnDeprecation(), NotADictionaryDeprecation(), ColumnQuotingDeprecation(), + ModelsKeyNonModelDeprecation(), ] deprecations: Dict[str, DBTDeprecation] = { diff --git a/core/dbt/node_types.py b/core/dbt/node_types.py index 84cdac2b6a9..15df0941672 100644 --- a/core/dbt/node_types.py +++ b/core/dbt/node_types.py @@ -1,3 +1,5 @@ +from typing import List + from hologram.helpers import StrEnum @@ -34,6 +36,22 @@ def refable(cls): cls.Snapshot, ]] + @classmethod + def documentable(cls) -> List['NodeType']: + return [ + cls.Model, + cls.Seed, + cls.Snapshot, + cls.Analysis, + cls.Source, + ] + + def pluralize(self) -> str: + if self == 'analysis': + return 'analyses' + else: + return f'{self}s' + class RunHookType(StrEnum): Start = 'on-run-start' diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 5bb857b7a5d..e7b61776c99 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -2,7 +2,7 @@ import os import pickle from datetime import datetime -from typing import Dict, Optional, Mapping, Callable, Any +from typing import Dict, Optional, Mapping, Callable, Any, List, Type from dbt.include.global_project import PACKAGES import dbt.exceptions @@ -15,7 +15,7 @@ from dbt.contracts.graph.compiled import CompileResultNode from dbt.contracts.graph.manifest import Manifest, FilePath, FileHash from dbt.exceptions import raise_compiler_error -from dbt.parser.base import BaseParser +from dbt.parser.base import BaseParser, Parser from dbt.parser.analysis import AnalysisParser from dbt.parser.data_test import DataTestParser from dbt.parser.docs import DocumentationParser @@ -36,7 +36,7 @@ DEFAULT_PARTIAL_PARSE = False -_parser_types = [ +_parser_types: List[Type[Parser]] = [ ModelParser, SnapshotParser, AnalysisParser, @@ -161,7 +161,7 @@ def parse_project( macro_manifest: Manifest, old_results: Optional[ParseResult], ) -> None: - parsers = [] + parsers: List[Parser] = [] for cls in _parser_types: parser = cls(self.results, project, self.root_project, macro_manifest) diff --git a/core/dbt/parser/results.py b/core/dbt/parser/results.py index a292afb4e00..520ec128adf 100644 --- a/core/dbt/parser/results.py +++ b/core/dbt/parser/results.py @@ -188,11 +188,16 @@ def sanitized_update( .format(node_id, old_file) ) + patched = False for name in old_file.patches: patch = _expect_value( name, old_result.patches, old_file, "patches" ) self.add_patch(source_file, patch) + patched = True + + if patched: + self.get_file(source_file).patches.sort() return True diff --git a/core/dbt/parser/schema_test_builders.py b/core/dbt/parser/schema_test_builders.py index 84ee7e22583..c2d7744d054 100644 --- a/core/dbt/parser/schema_test_builders.py +++ b/core/dbt/parser/schema_test_builders.py @@ -93,10 +93,10 @@ def tests(self) -> List[Union[Dict[str, Any], str]]: return self.table.tests -ModelTarget = UnparsedNodeUpdate +NodeTarget = UnparsedNodeUpdate -Target = TypeVar('Target', ModelTarget, SourceTarget) +Target = TypeVar('Target', NodeTarget, SourceTarget) @dataclass @@ -257,7 +257,7 @@ def macro_name(self) -> str: return macro_name def describe_test_target(self) -> str: - if isinstance(self.target, ModelTarget): + if isinstance(self.target, NodeTarget): fmt = "model('{0}')" elif isinstance(self.target, SourceTarget): fmt = "source('{0.source}', '{0.table}')" @@ -268,7 +268,7 @@ def describe_test_target(self) -> str: raise NotImplementedError('describe_test_target not implemented!') def get_test_name(self) -> Tuple[str, str]: - if isinstance(self.target, ModelTarget): + if isinstance(self.target, NodeTarget): name = self.name elif isinstance(self.target, SourceTarget): name = 'source_' + self.name @@ -290,7 +290,7 @@ def build_raw_sql(self) -> str: ) def build_model_str(self): - if isinstance(self.target, ModelTarget): + if isinstance(self.target, NodeTarget): fmt = "ref('{0.name}')" elif isinstance(self.target, SourceTarget): fmt = "source('{0.source.name}', '{0.table.name}')" diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 6117c79f8d8..87c485bff19 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -1,5 +1,7 @@ import os -from typing import Iterable, Dict, Any, Union, List, Optional + +from abc import abstractmethod +from typing import Iterable, Dict, Any, Union, List, Optional, Generic, TypeVar from hologram import ValidationError @@ -29,7 +31,7 @@ from dbt.parser.base import SimpleParser from dbt.parser.search import FileBlock, FilesystemSearcher from dbt.parser.schema_test_builders import ( - TestBuilder, SourceTarget, ModelTarget, Target, + TestBuilder, SourceTarget, NodeTarget, Target, SchemaTestBlock, TargetBlock, YamlBlock, ) from dbt.utils import get_pseudo_test_path @@ -97,7 +99,7 @@ class SchemaParser(SimpleParser[SchemaTestBlock, ParsedTestNode]): There are basically three phases to the schema parser: - read_yaml_{models,sources}: read in yaml as a dictionary, then validate it against the basic structures required so we can start - parsing (ModelTarget, SourceTarget) + parsing (NodeTarget, SourceTarget) - these return potentially many Targets per yaml block, since earch source can have multiple tables - parse_target_{model,source}: Read in the underlying target, parse and @@ -105,10 +107,6 @@ class SchemaParser(SimpleParser[SchemaTestBlock, ParsedTestNode]): any refs/descriptions, and return a parsed entity with the appropriate information. """ - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self._renderer = ConfigRenderer(self.root_project.cli_vars) - @classmethod def get_compiled_path(cls, block: FileBlock) -> str: # should this raise an error? @@ -145,62 +143,6 @@ def _parse_format_version( path, 'version {} is not supported'.format(version) ) - def _get_dicts_for( - self, yaml: YamlBlock, key: str - ) -> Iterable[Dict[str, Any]]: - data = yaml.data.get(key, []) - if not isinstance(data, list): - raise CompilationException( - '{} must be a list, got {} instead: ({})' - .format(key, type(data), _trimmed(str(data))) - ) - path = yaml.path.original_file_path - - for entry in data: - str_keys = ( - isinstance(entry, dict) and - all(isinstance(k, str) for k in entry) - ) - if str_keys: - yield entry - else: - msg = error_context( - path, key, data, 'expected a dict with string keys' - ) - raise CompilationException(msg) - - def read_yaml_models( - self, yaml: YamlBlock - ) -> Iterable[ModelTarget]: - path = yaml.path.original_file_path - yaml_key = 'models' - - for data in self._get_dicts_for(yaml, yaml_key): - try: - model = UnparsedNodeUpdate.from_dict(data) - except (ValidationError, JSONValidationException) as exc: - msg = error_context(path, yaml_key, data, exc) - raise CompilationException(msg) from exc - else: - yield model - - def read_yaml_sources( - self, yaml: YamlBlock - ) -> Iterable[SourceTarget]: - path = yaml.path.original_file_path - yaml_key = 'sources' - - for data in self._get_dicts_for(yaml, yaml_key): - try: - data = self._renderer.render_schema_source(data) - source = UnparsedSourceDefinition.from_dict(data) - except (ValidationError, JSONValidationException) as exc: - msg = error_context(path, yaml_key, data, exc) - raise CompilationException(msg) from exc - else: - for table in source.tables: - yield SourceTarget(source, table) - def _yaml_from_file( self, source_file: SourceFile ) -> Optional[Dict[str, Any]]: @@ -307,6 +249,120 @@ def parse_test( ) raise CompilationException(msg) from exc + def parse_tests(self, block: TargetBlock) -> ParserRef: + refs = ParserRef() + for column in block.columns: + self.parse_column(block, column, refs) + + for test in block.tests: + self.parse_test(block, test, None) + return refs + + def parse_file(self, block: FileBlock) -> None: + dct = self._yaml_from_file(block.file) + # mark the file as seen, even if there are no macros in it + self.results.get_file(block.file) + if dct: + yaml_block = YamlBlock.from_file_block(block, dct) + + self._parse_format_version(yaml_block) + + parser: YamlParser + for key in NodeType.documentable(): + if key == NodeType.Source: + parser = SourceParser(self, yaml_block, key.pluralize()) + parser.parse() + else: + parser = NodeParser(self, yaml_block, key.pluralize()) + parser.parse() + + +Parsed = TypeVar('Parsed', ParsedSourceDefinition, ParsedNodePatch) + + +class YamlParser(Generic[Target, Parsed]): + def __init__( + self, schema_parser: SchemaParser, yaml: YamlBlock, key: str + ) -> None: + self.schema_parser = schema_parser + self.key = key + self.yaml = yaml + + @property + def results(self): + return self.schema_parser.results + + @property + def project(self): + return self.schema_parser.project + + @property + def default_database(self): + return self.schema_parser.default_database + + @property + def root_project(self): + return self.schema_parser.root_project + + def get_key_dicts(self) -> Iterable[Dict[str, Any]]: + data = self.yaml.data.get(self.key, []) + if not isinstance(data, list): + raise CompilationException( + '{} must be a list, got {} instead: ({})' + .format(self.key, type(data), _trimmed(str(data))) + ) + path = self.yaml.path.original_file_path + + for entry in data: + str_keys = ( + isinstance(entry, dict) and + all(isinstance(k, str) for k in entry) + ) + if str_keys: + yield entry + else: + msg = error_context( + path, self.key, data, 'expected a dict with string keys' + ) + raise CompilationException(msg) + + def parse(self): + node: Target + for node in self.get_unparsed_target(): + node_block = TargetBlock.from_yaml_block(self.yaml, node) + refs = self.schema_parser.parse_tests(node_block) + self.parse_with_refs(node_block, refs) + + @abstractmethod + def get_unparsed_target(self) -> Iterable[Target]: + raise NotImplementedError('get_unparsed_target is abstract') + + @abstractmethod + def parse_with_refs( + self, block: TargetBlock[Target], refs: ParserRef + ) -> None: + raise NotImplementedError('parse_with_refs is abstract') + + +class SourceParser(YamlParser[SourceTarget, ParsedSourceDefinition]): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._renderer = ConfigRenderer(self.root_project.cli_vars) + + def get_unparsed_target(self) -> Iterable[SourceTarget]: + path = self.yaml.path.original_file_path + + for data in self.get_key_dicts(): + try: + data = self._renderer.render_schema_source(data) + source = UnparsedSourceDefinition.from_dict(data) + except (ValidationError, JSONValidationException) as exc: + msg = error_context(path, self.key, data, exc) + raise CompilationException(msg) from exc + else: + for table in source.tables: + yield SourceTarget(source, table) + def _calculate_freshness( self, source: UnparsedSourceDefinition, @@ -323,19 +379,9 @@ def _calculate_freshness( else: return None - def parse_tests(self, block: TargetBlock) -> ParserRef: - refs = ParserRef() - for column in block.columns: - self.parse_column(block, column, refs) - - for test in block.tests: - self.parse_test(block, test, None) - return refs - - def generate_source_node( - self, block: TargetBlock, refs: ParserRef - ) -> ParsedSourceDefinition: - assert isinstance(block.target, SourceTarget) + def parse_with_refs( + self, block: TargetBlock[SourceTarget], refs: ParserRef + ) -> None: source = block.target.source table = block.target.table unique_id = '.'.join([ @@ -353,7 +399,7 @@ def generate_source_node( path = block.path.original_file_path source_meta = source.meta or {} - return ParsedSourceDefinition( + result = ParsedSourceDefinition( package_name=self.project.project_name, database=(source.database or self.default_database), schema=(source.schema or source.name), @@ -378,59 +424,41 @@ def generate_source_node( resource_type=NodeType.Source, fqn=[self.project.project_name, source.name, table.name], ) + self.results.add_source(self.yaml.file, result) - def generate_node_patch( - self, block: TargetBlock, refs: ParserRef - ) -> ParsedNodePatch: - assert isinstance(block.target, UnparsedNodeUpdate) + +class NodeParser(YamlParser[NodeTarget, ParsedNodePatch]): + def get_unparsed_target(self) -> Iterable[NodeTarget]: + path = self.yaml.path.original_file_path + + for data in self.get_key_dicts(): + data.update({ + 'original_file_path': path, + 'yaml_key': self.key, + 'package_name': self.schema_parser.project.project_name, + }) + try: + model = UnparsedNodeUpdate.from_dict(data) + except (ValidationError, JSONValidationException) as exc: + msg = error_context(path, self.key, data, exc) + raise CompilationException(msg) from exc + else: + yield model + + def parse_with_refs( + self, block: TargetBlock[UnparsedNodeUpdate], refs: ParserRef + ) -> None: description = block.target.description collect_docrefs(block.target, refs, None, description) - return ParsedNodePatch( + result = ParsedNodePatch( name=block.target.name, - original_file_path=block.path.original_file_path, + original_file_path=block.target.original_file_path, + yaml_key=block.target.yaml_key, + package_name=block.target.package_name, description=description, columns=refs.column_info, docrefs=refs.docrefs, meta=block.target.meta, ) - - def parse_target_model( - self, target_block: TargetBlock[UnparsedNodeUpdate] - ) -> ParsedNodePatch: - refs = self.parse_tests(target_block) - patch = self.generate_node_patch(target_block, refs) - return patch - - def parse_target_source( - self, target_block: TargetBlock[SourceTarget] - ) -> ParsedSourceDefinition: - refs = self.parse_tests(target_block) - patch = self.generate_source_node(target_block, refs) - return patch - - def parse_yaml_models(self, yaml_block: YamlBlock): - for node in self.read_yaml_models(yaml_block): - node_block = TargetBlock.from_yaml_block(yaml_block, node) - patch = self.parse_target_model(node_block) - self.results.add_patch(yaml_block.file, patch) - - def parse_yaml_sources( - self, yaml_block: YamlBlock - ): - for source in self.read_yaml_sources(yaml_block): - source_block = TargetBlock.from_yaml_block(yaml_block, source) - source_table = self.parse_target_source(source_block) - self.results.add_source(yaml_block.file, source_table) - - def parse_file(self, block: FileBlock) -> None: - dct = self._yaml_from_file(block.file) - # mark the file as seen, even if there are no macros in it - self.results.get_file(block.file) - if dct: - yaml_block = YamlBlock.from_file_block(block, dct) - - self._parse_format_version(yaml_block) - - self.parse_yaml_models(yaml_block) - self.parse_yaml_sources(yaml_block) + self.results.add_patch(self.yaml.file, result) diff --git a/core/dbt/utils.py b/core/dbt/utils.py index 4076f9f5eb1..4e8d865f024 100644 --- a/core/dbt/utils.py +++ b/core/dbt/utils.py @@ -9,7 +9,7 @@ import os from enum import Enum from typing import ( - Tuple, Type, Any, Optional, TypeVar, Dict, Iterable, Set, List + Tuple, Type, Any, Optional, TypeVar, Dict, Iterable, Set, List, Union ) from typing_extensions import Protocol @@ -511,13 +511,20 @@ def translate_aliases(kwargs, aliases): return result -def pluralize(count, string): - if count == 1: - return "{} {}".format(count, string) - elif string == 'analysis': - return "{} {}".format(count, 'analyses') +def _pluralize(string: Union[str, NodeType]) -> str: + try: + convert = NodeType(string) + except ValueError: + return f'{string}s' else: - return "{} {}s".format(count, string) + return convert.pluralize() + + +def pluralize(count, string: Union[str, NodeType]): + pluralized: str = str(string) + if count != 1: + pluralized = _pluralize(string) + return f'{count} {pluralized}' def restrict_to(*restrictions): diff --git a/test/integration/004_simple_snapshot_test/models/schema.yml b/test/integration/004_simple_snapshot_test/models/schema.yml index 187a9d13ab2..259e55b95fc 100644 --- a/test/integration/004_simple_snapshot_test/models/schema.yml +++ b/test/integration/004_simple_snapshot_test/models/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +snapshots: - name: snapshot_actual tests: - mutually_exclusive_ranges diff --git a/test/integration/005_simple_seed_test/models-bq/schema.yml b/test/integration/005_simple_seed_test/models-bq/schema.yml index c894438f8ec..019a9524f0f 100644 --- a/test/integration/005_simple_seed_test/models-bq/schema.yml +++ b/test/integration/005_simple_seed_test/models-bq/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +seeds: - name: seed_enabled columns: - name: birthday diff --git a/test/integration/005_simple_seed_test/models-pg/schema.yml b/test/integration/005_simple_seed_test/models-pg/schema.yml index 2e35d6db239..52eb3f3be24 100644 --- a/test/integration/005_simple_seed_test/models-pg/schema.yml +++ b/test/integration/005_simple_seed_test/models-pg/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +seeds: - name: seed_enabled columns: - name: birthday diff --git a/test/integration/005_simple_seed_test/models-rs/schema.yml b/test/integration/005_simple_seed_test/models-rs/schema.yml index f3c1fae55b4..00a79bf9338 100644 --- a/test/integration/005_simple_seed_test/models-rs/schema.yml +++ b/test/integration/005_simple_seed_test/models-rs/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +seeds: - name: seed_enabled columns: - name: birthday diff --git a/test/integration/005_simple_seed_test/models-snowflake/schema.yml b/test/integration/005_simple_seed_test/models-snowflake/schema.yml index 10ada11348e..c8c725fc20a 100644 --- a/test/integration/005_simple_seed_test/models-snowflake/schema.yml +++ b/test/integration/005_simple_seed_test/models-snowflake/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +seeds: - name: seed_enabled columns: - name: birthday diff --git a/test/integration/012_deprecation_tests/data/seed.csv b/test/integration/012_deprecation_tests/data/seed.csv new file mode 100644 index 00000000000..69ae389bec4 --- /dev/null +++ b/test/integration/012_deprecation_tests/data/seed.csv @@ -0,0 +1,3 @@ +a,b +1,hello +2,goodbye diff --git a/test/integration/012_deprecation_tests/models-key-mismatch/schema.yml b/test/integration/012_deprecation_tests/models-key-mismatch/schema.yml new file mode 100644 index 00000000000..0f713412edf --- /dev/null +++ b/test/integration/012_deprecation_tests/models-key-mismatch/schema.yml @@ -0,0 +1,4 @@ +version: 2 +models: + - name: seed + description: my cool seed diff --git a/test/integration/012_deprecation_tests/test_deprecations.py b/test/integration/012_deprecation_tests/test_deprecations.py index f7f67e3263e..42e681dfbda 100644 --- a/test/integration/012_deprecation_tests/test_deprecations.py +++ b/test/integration/012_deprecation_tests/test_deprecations.py @@ -60,18 +60,6 @@ def test_postgres_deprecations(self): class TestMaterializationReturnDeprecation(BaseTestDeprecations): - def setUp(self): - super().setUp() - deprecations.reset_deprecations() - - @property - def schema(self): - return "deprecation_test_012" - - @staticmethod - def dir(path): - return path.lstrip("/") - @property def models(self): return self.dir('custom-models') @@ -93,3 +81,23 @@ def test_postgres_deprecations(self): self.run_dbt(strict=False) expected = {'materialization-return'} self.assertEqual(expected, deprecations.active_deprecations) + + +class TestModelsKeyMismatchDeprecation(BaseTestDeprecations): + @property + def models(self): + return self.dir('models-key-mismatch') + + @use_profile('postgres') + def test_postgres_deprecations_fail(self): + # this should fail at compile_time + with self.assertRaises(dbt.exceptions.CompilationException) as exc: + self.run_dbt(strict=True) + self.assertIn('had resource type', str(exc.exception)) + + @use_profile('postgres') + def test_postgres_deprecations(self): + self.assertEqual(deprecations.active_deprecations, set()) + self.run_dbt(strict=False) + expected = {'models-key-mismatch'} + self.assertEqual(expected, deprecations.active_deprecations) diff --git a/test/integration/014_hook_tests/seed-models-bq/schema.yml b/test/integration/014_hook_tests/seed-models-bq/schema.yml index 187a0db9bfe..21bbf202f16 100644 --- a/test/integration/014_hook_tests/seed-models-bq/schema.yml +++ b/test/integration/014_hook_tests/seed-models-bq/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +seeds: - name: example_seed columns: - name: a diff --git a/test/integration/014_hook_tests/seed-models/schema.yml b/test/integration/014_hook_tests/seed-models/schema.yml index 5017027d8f5..c877d9d3a21 100644 --- a/test/integration/014_hook_tests/seed-models/schema.yml +++ b/test/integration/014_hook_tests/seed-models/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +seeds: - name: example_seed columns: - name: new_col diff --git a/test/integration/014_hook_tests/test-snapshot-models/schema.yml b/test/integration/014_hook_tests/test-snapshot-models/schema.yml index e02fd09ca75..12922bb91d2 100644 --- a/test/integration/014_hook_tests/test-snapshot-models/schema.yml +++ b/test/integration/014_hook_tests/test-snapshot-models/schema.yml @@ -1,5 +1,5 @@ version: 2 -models: +snapshots: - name: example_snapshot columns: - name: new_col diff --git a/test/integration/029_docs_generate_tests/models/schema.yml b/test/integration/029_docs_generate_tests/models/schema.yml index 6f3fc661976..7f0d9518a63 100644 --- a/test/integration/029_docs_generate_tests/models/schema.yml +++ b/test/integration/029_docs_generate_tests/models/schema.yml @@ -19,3 +19,17 @@ models: description: The last time this user's email was updated tests: - test.nothing +seeds: + - name: seed + description: "The test seed" + columns: + - name: id + description: The user ID number + - name: first_name + description: The user's first name + - name: email + description: The user's email + - name: ip_address + description: The user's IP address + - name: updated_at + description: The last time this user's email was updated diff --git a/test/integration/029_docs_generate_tests/test_docs_generate.py b/test/integration/029_docs_generate_tests/test_docs_generate.py index 8fc2c38d05d..4d518aacfdf 100644 --- a/test/integration/029_docs_generate_tests/test_docs_generate.py +++ b/test/integration/029_docs_generate_tests/test_docs_generate.py @@ -949,7 +949,7 @@ def expected_seeded_manifest(self, model_database=None): 'tags': [], 'quote_columns': True, }, - 'patch_path': None, + 'patch_path': schema_yml_path, 'path': 'seed.csv', 'name': 'seed', 'root_path': self.test_root_dir, @@ -969,8 +969,39 @@ def expected_seeded_manifest(self, model_database=None): 'schema': my_schema_name, 'database': self.default_database, 'alias': 'seed', - 'description': '', - 'columns': {}, + 'description': 'The test seed', + 'columns': { + 'id': { + 'name': 'id', + 'description': 'The user ID number', + 'data_type': None, + 'meta': {}, + }, + 'first_name': { + 'name': 'first_name', + 'description': "The user's first name", + 'data_type': None, + 'meta': {}, + }, + 'email': { + 'name': 'email', + 'description': "The user's email", + 'data_type': None, + 'meta': {}, + }, + 'ip_address': { + 'name': 'ip_address', + 'description': "The user's IP address", + 'data_type': None, + 'meta': {}, + }, + 'updated_at': { + 'name': 'updated_at', + 'description': "The last time this user's email was updated", + 'data_type': None, + 'meta': {}, + }, + }, 'docrefs': [], 'compiled': True, 'compiled_sql': '', @@ -1197,7 +1228,7 @@ def expected_seeded_manifest(self, model_database=None): 'docs': [], 'macros': [], 'nodes': ['test.test.unique_model_id', 'test.test.not_null_model_id', 'test.test.test_nothing_model_'], - 'patches': ['model'], + 'patches': ['model', 'seed'], 'sources': [], }, }, @@ -2510,7 +2541,38 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'node': { 'alias': 'seed', 'build_path': None, - 'columns': {}, + 'columns': { + 'id': { + 'description': 'The user ID number', + 'name': 'id', + 'data_type': None, + 'meta': {} + }, + 'first_name': { + 'description': "The user's first name", + 'name': 'first_name', + 'data_type': None, + 'meta': {} + }, + 'email': { + 'description': "The user's email", + 'name': 'email', + 'data_type': None, + 'meta': {} + }, + 'ip_address': { + 'description': "The user's IP address", + 'name': 'ip_address', + 'data_type': None, + 'meta': {} + }, + 'updated_at': { + 'description': "The last time this user's email was updated", + 'name': 'updated_at', + 'data_type': None, + 'meta': {} + } + }, 'compiled': True, 'compiled_sql': '', 'config': { @@ -2527,7 +2589,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, }, 'sources': [], 'depends_on': {'macros': [], 'nodes': []}, - 'description': '', + 'description': 'The test seed', 'docrefs': [], 'extra_ctes': [], 'extra_ctes_injected': True, @@ -2537,7 +2599,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'name': 'seed', 'original_file_path': self.dir('seed/seed.csv'), 'package_name': 'test', - 'patch_path': None, + 'patch_path': schema_yml_path, 'path': 'seed.csv', 'raw_sql': '', 'refs': [], diff --git a/test/unit/test_contracts_graph_parsed.py b/test/unit/test_contracts_graph_parsed.py index 610bea7ffc0..1652cba44a7 100644 --- a/test/unit/test_contracts_graph_parsed.py +++ b/test/unit/test_contracts_graph_parsed.py @@ -303,6 +303,8 @@ def test_patch_ok(self): ) patch = ParsedNodePatch( name='foo', + yaml_key='models', + package_name='test', description='The foo model', original_file_path='/path/to/schema.yml', columns={'a': ColumnInfo(name='a', description='a text field', meta={})}, @@ -408,6 +410,8 @@ def patch_invalid(self): # invalid patch: description can't be None patch = ParsedNodePatch( name='foo', + yaml_key='models', + package_name='test', description=None, original_file_path='/path/to/schema.yml', columns={}, @@ -1360,10 +1364,14 @@ def test_empty(self): 'columns': {}, 'docrefs': [], 'meta': {}, + 'yaml_key': 'models', + 'package_name': 'test', } - patch = ParsedNodePatch( + patch = self.ContractType( name='foo', description='The foo model', + yaml_key='models', + package_name='test', original_file_path='/path/to/schema.yml', columns={}, docrefs=[], @@ -1383,9 +1391,11 @@ def test_populated(self): 'documentation_package': 'test', } ], - 'meta': {'key': ['value']} + 'meta': {'key': ['value']}, + 'yaml_key': 'models', + 'package_name': 'test', } - patch = ParsedNodePatch( + patch = self.ContractType( name='foo', description='The foo model', original_file_path='/path/to/schema.yml', @@ -1394,6 +1404,8 @@ def test_populated(self): Docref(documentation_name='foo', documentation_package='test'), ], meta={'key': ['value']}, + yaml_key='models', + package_name='test', ) self.assert_symmetric(patch, dct) pickle.loads(pickle.dumps(patch)) diff --git a/test/unit/test_contracts_graph_unparsed.py b/test/unit/test_contracts_graph_unparsed.py index 5f1e4f2916c..4f1ebd790d1 100644 --- a/test/unit/test_contracts_graph_unparsed.py +++ b/test/unit/test_contracts_graph_unparsed.py @@ -374,10 +374,23 @@ class TestUnparsedNodeUpdate(ContractTestCase): ContractType = UnparsedNodeUpdate def test_defaults(self): - minimum = self.ContractType(name='foo') - from_dict = {'name': 'foo'} + minimum = self.ContractType( + name='foo', + yaml_key='models', + original_file_path='/some/fake/path', + package_name='test', + ) + from_dict = { + 'name': 'foo', + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', + 'package_name': 'test', + } to_dict = { 'name': 'foo', + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', + 'package_name': 'test', 'columns': [], 'description': '', 'tests': [], @@ -389,6 +402,9 @@ def test_defaults(self): def test_contents(self): update = self.ContractType( name='foo', + yaml_key='models', + original_file_path='/some/fake/path', + package_name='test', description='a description', tests=['table_test'], meta={'key': ['value1', 'value2']}, @@ -411,6 +427,9 @@ def test_contents(self): ) dct = { 'name': 'foo', + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', + 'package_name': 'test', 'description': 'a description', 'tests': ['table_test'], 'meta': {'key': ['value1', 'value2']}, @@ -438,6 +457,9 @@ def test_contents(self): def test_bad_test_type(self): dct = { 'name': 'foo', + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', + 'package_name': 'test', 'description': 'a description', 'tests': ['table_test'], 'meta': {'key': ['value1', 'value2']}, @@ -456,6 +478,8 @@ def test_bad_test_type(self): {'accepted_values': {'values': ['blue', 'green']}} ], 'meta': {}, + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', }, ], } @@ -463,6 +487,9 @@ def test_bad_test_type(self): dct = { 'name': 'foo', + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', + 'package_name': 'test', 'description': 'a description', 'tests': ['table_test'], 'meta': {'key': ['value1', 'value2']}, @@ -481,6 +508,8 @@ def test_bad_test_type(self): {'accepted_values': {'values': ['blue', 'green']}} ], 'meta': {}, + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', }, ], } @@ -488,6 +517,9 @@ def test_bad_test_type(self): # missing a name dct = { + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', + 'package_name': 'test', 'description': 'a description', 'tests': ['table_test'], 'meta': {'key': ['value1', 'value2']}, @@ -506,6 +538,8 @@ def test_bad_test_type(self): {'accepted_values': {'values': ['blue', 'green']}} ], 'meta': {}, + 'yaml_key': 'models', + 'original_file_path': '/some/fake/path', }, ], } diff --git a/test/unit/test_parser.py b/test/unit/test_parser.py index c9f0dc6fa9c..c7a34a58f6d 100644 --- a/test/unit/test_parser.py +++ b/test/unit/test_parser.py @@ -11,6 +11,7 @@ ModelParser, MacroParser, DataTestParser, SchemaParser, ParserUtils, ParseResult, SnapshotParser, AnalysisParser ) +from dbt.parser.schemas import NodeParser, SourceParser from dbt.parser.search import FileBlock from dbt.parser.schema_test_builders import YamlBlock @@ -190,19 +191,20 @@ def yaml_block_for(self, test_yml: str, filename: str): class SchemaParserSourceTest(SchemaParserTest): def test__read_basic_source(self): block = self.yaml_block_for(SINGLE_TABLE_SOURCE, 'test_one.yml') - self.assertEqual(len(list(self.parser.read_yaml_models(yaml=block))), 0) - results = list(self.parser.read_yaml_sources(yaml=block)) + NodeParser(self.parser, block, 'models').parse() + SourceParser(self.parser, block, 'sources').parse() + self.assertEqual(len(list(self.parser.results.patches)), 0) + self.assertEqual(len(list(self.parser.results.nodes)), 0) + results = list(self.parser.results.sources.values()) self.assertEqual(len(results), 1) - self.assertEqual(results[0].source.name, 'my_source') - self.assertEqual(results[0].table.name, 'my_table') - self.assertEqual(results[0].table.description, '') - self.assertEqual(len(results[0].tests), 0) + self.assertEqual(results[0].source_name, 'my_source') + self.assertEqual(results[0].name, 'my_table') + self.assertEqual(results[0].description, '') self.assertEqual(len(results[0].columns), 0) def test__parse_basic_source(self): block = self.file_block_for(SINGLE_TABLE_SOURCE, 'test_one.yml') self.parser.parse_file(block) - # self.parser.parse_yaml_sources(yaml_block=block) self.assert_has_results_length(self.parser.results, sources=1) src = list(self.parser.results.sources.values())[0] expected = ParsedSourceDefinition( @@ -227,15 +229,17 @@ def test__parse_basic_source(self): def test__read_basic_source_tests(self): block = self.yaml_block_for(SINGLE_TABLE_SOURCE_TESTS, 'test_one.yml') - self.assertEqual(len(list(self.parser.read_yaml_models(yaml=block))), 0) - results = list(self.parser.read_yaml_sources(yaml=block)) + NodeParser(self.parser, block, 'models').parse() + self.assertEqual(len(list(self.parser.results.nodes)), 0) + SourceParser(self.parser, block, 'sources').parse() + self.assertEqual(len(list(self.parser.results.patches)), 0) + self.assertEqual(len(list(self.parser.results.nodes)), 2) + results = list(self.parser.results.sources.values()) self.assertEqual(len(results), 1) - self.assertEqual(results[0].source.name, 'my_source') - self.assertEqual(results[0].table.name, 'my_table') - self.assertEqual(results[0].table.description, 'A description of my table') + self.assertEqual(results[0].source_name, 'my_source') + self.assertEqual(results[0].name, 'my_table') + self.assertEqual(results[0].description, 'A description of my table') self.assertEqual(len(results[0].columns), 1) - self.assertEqual(len(results[0].columns[0].tests), 2) - self.assertEqual(len(results[0].tests), 0) def test__parse_basic_source_tests(self): block = self.file_block_for(SINGLE_TABLE_SOURCE_TESTS, 'test_one.yml') @@ -274,13 +278,10 @@ def test__parse_basic_source_tests(self): class SchemaParserModelsTest(SchemaParserTest): def test__read_basic_model_tests(self): block = self.yaml_block_for(SINGLE_TABLE_MODEL_TESTS, 'test_one.yml') - self.assertEqual(len(list(self.parser.read_yaml_sources(yaml=block))), 0) - results = list(self.parser.read_yaml_models(yaml=block)) - self.assertEqual(len(results), 1) - self.assertEqual(results[0].name, 'my_model') - self.assertEqual(len(results[0].columns), 1) - self.assertEqual(len(results[0].columns[0].tests), 3) - self.assertEqual(len(results[0].tests), 0) + self.parser.parse_file(block) + self.assertEqual(len(list(self.parser.results.patches)), 1) + self.assertEqual(len(list(self.parser.results.sources)), 0) + self.assertEqual(len(list(self.parser.results.nodes)), 3) def test__parse_basic_model_tests(self): block = self.file_block_for(SINGLE_TABLE_MODEL_TESTS, 'test_one.yml') @@ -298,6 +299,8 @@ def test__parse_basic_model_tests(self): docrefs=[], original_file_path=normalize('models/test_one.yml'), meta={}, + yaml_key='models', + package_name='snowplow', ) self.assertEqual(patch, expected_patch) From c4a5eb9803be25930440d35341f9fbc60ab03cc7 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Thu, 16 Jan 2020 08:01:48 -0700 Subject: [PATCH 2/6] PR feedback add support for newlines in output line-wrap deprecations --- core/dbt/contracts/graph/manifest.py | 37 +++++---- core/dbt/deprecations.py | 78 +++++++++---------- core/dbt/exceptions.py | 15 ++-- core/dbt/node_types.py | 1 - core/dbt/parser/results.py | 34 ++++---- core/dbt/ui/printer.py | 12 ++- .../test_deprecations.py | 3 +- 7 files changed, 99 insertions(+), 81 deletions(-) diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index 49e67e7e1b4..5ccd3a8514d 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -18,6 +18,7 @@ from dbt.include.global_project import PACKAGES from dbt.logger import GLOBAL_LOGGER as logger from dbt.node_types import NodeType +from dbt.ui import printer from dbt import deprecations from dbt import tracking import dbt.utils @@ -445,20 +446,30 @@ def patch_nodes(self, patches): patch = patches.pop(node.name, None) if not patch: continue - if node.resource_type.pluralize() == patch.yaml_key: + expected_key = node.resource_type.pluralize() + if expected_key == patch.yaml_key: node.patch(patch) - elif patch.yaml_key == 'models': - deprecations.warn( - 'models-key-mismatch', patch=patch, node=node - ) - else: - raise_compiler_error( - f'patch instruction in {patch.original_file_path} under ' - f'key "{patch.yaml_key}" was for node "{node.name}", but ' - f'the node with the same name (from ' - f'{node.original_file_path}) had resource type ' - f'"{node.resource_type}"' - ) + if expected_key != patch.yaml_key: + if patch.yaml_key == 'models': + deprecations.warn( + 'models-key-mismatch', + patch=patch, node=node, expected_key=expected_key + ) + else: + msg = printer.line_wrap_message( + f'''\ + '{node.name}' is a {node.resource_type} node, but it is + specified in the {patch.yaml_key} section of + {patch.original_file_path}. + + + + To fix this error, place the `{node.name}` + specification under the {expected_key} key instead. + ''' + ) + raise_compiler_error(msg) + node.patch(patch) # log debug-level warning about nodes we couldn't find if patches: diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index a6101b8e8c1..701452f52f2 100644 --- a/core/dbt/deprecations.py +++ b/core/dbt/deprecations.py @@ -3,6 +3,7 @@ import dbt.links import dbt.exceptions import dbt.flags +from dbt.ui import printer class DBTDeprecation: @@ -28,57 +29,47 @@ def description(self) -> str: def show(self, *args, **kwargs) -> None: if self.name not in active_deprecations: desc = self.description.format(**kwargs) - dbt.exceptions.warn_or_error( - "* Deprecation Warning: {}\n".format(desc) - ) + msg = printer.line_wrap_message(f"* Deprecation Warning: {desc}\n") + dbt.exceptions.warn_or_error(msg) active_deprecations.add(self.name) -class DBTRepositoriesDeprecation(DBTDeprecation): - _name = "repositories" - - _description = """ - The dbt_project.yml configuration option 'repositories' is - deprecated. Please place dependencies in the `packages.yml` file instead. - The 'repositories' option will be removed in a future version of dbt. - - For more information, see: https://docs.getdbt.com/docs/package-management +class GenerateSchemaNameSingleArgDeprecated(DBTDeprecation): + _name = 'generate-schema-name-single-arg' - # Example packages.yml contents: + _description = '''\ + As of dbt v0.14.0, the `generate_schema_name` macro accepts a second "node" + argument. The one-argument form of `generate_schema_name` is deprecated, + and will become unsupported in a future release. -{recommendation} - """.lstrip() -class GenerateSchemaNameSingleArgDeprecated(DBTDeprecation): - _name = 'generate-schema-name-single-arg' - - _description = '''As of dbt v0.14.0, the `generate_schema_name` macro - accepts a second "node" argument. The one-argument form of `generate_schema_name` - is deprecated, and will become unsupported in a future release. + For more information, see: - For more information, see: https://docs.getdbt.com/v0.14/docs/upgrading-to-014 - ''' # noqa + ''' class MaterializationReturnDeprecation(DBTDeprecation): _name = 'materialization-return' - _description = ''' + _description = '''\ The materialization ("{materialization}") did not explicitly return a list of relations to add to the cache. By default the target relation will be added, but this behavior will be removed in a future version of dbt. - For more information, see: - https://docs.getdbt.com/v0.15/docs/creating-new-materializations#section-6-returning-relations + + + For more information, see: + + https://docs.getdbt.com/v0.15/docs/creating-new-materializations#section-6-returning-relations '''.lstrip() class NotADictionaryDeprecation(DBTDeprecation): _name = 'not-a-dictionary' - _description = ''' + _description = '''\ The object ("{obj}") was used as a dictionary. In a future version of dbt this capability will be removed from objects of this type. '''.lstrip() @@ -87,11 +78,14 @@ class NotADictionaryDeprecation(DBTDeprecation): class ColumnQuotingDeprecation(DBTDeprecation): _name = 'column-quoting-unset' - _description = ''' + _description = '''\ The quote_columns parameter was not set for seeds, so the default value of False was chosen. The default will change to True in a future release. + + For more information, see: + https://docs.getdbt.com/v0.15/docs/seeds#section-specify-column-quoting ''' @@ -99,20 +93,27 @@ class ColumnQuotingDeprecation(DBTDeprecation): class ModelsKeyNonModelDeprecation(DBTDeprecation): _name = 'models-key-mismatch' - _description = ''' - patch instruction in {patch.original_file_path} under key - "models" was for node "{node.name}", but the node with the same - name (from {node.original_file_path}) had resource type - "{node.resource_type}". Nodes should be described under their resource - specific key. Support for this will be removed in a future release. - '''.strip() + _description = '''\ + "{node.name}" is a {node.resource_type} node, but it is specified in + the {patch.yaml_key} section of {patch.original_file_path}. + + + + To fix this warning, place the `{node.name}` specification under + the {expected_key} key instead. + + This warning will become an error in a future release. + '''.strip() _adapter_renamed_description = """\ The adapter function `adapter.{old_name}` is deprecated and will be removed in - a future release of dbt. Please use `adapter.{new_name}` instead. - Documentation for {new_name} can be found here: - https://docs.getdbt.com/docs/adapter""" +a future release of dbt. Please use `adapter.{new_name}` instead. + +Documentation for {new_name} can be found here: + + https://docs.getdbt.com/docs/adapter +""" def renamed_method(old_name: str, new_name: str): @@ -143,7 +144,6 @@ def warn(name, *args, **kwargs): active_deprecations: Set[str] = set() deprecations_list: List[DBTDeprecation] = [ - DBTRepositoriesDeprecation(), GenerateSchemaNameSingleArgDeprecated(), MaterializationReturnDeprecation(), NotADictionaryDeprecation(), diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 50eb7b4d438..e4c3f5832b5 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -445,13 +445,14 @@ 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 model '{}' {}which {}" - .format(model.resource_type.title(), - model.unique_id, - source_path_string, - target_model_name, - target_package_string, - reason)) + return "{} '{}'{} depends on a node named '{}' {}which {}".format( + model.resource_type.title(), + model.unique_id, + source_path_string, + target_model_name, + target_package_string, + reason + ) def get_target_disabled_msg(model, target_model_name, target_model_package): diff --git a/core/dbt/node_types.py b/core/dbt/node_types.py index 15df0941672..9a0857de2c5 100644 --- a/core/dbt/node_types.py +++ b/core/dbt/node_types.py @@ -42,7 +42,6 @@ def documentable(cls) -> List['NodeType']: cls.Model, cls.Seed, cls.Snapshot, - cls.Analysis, cls.Source, ] diff --git a/core/dbt/parser/results.py b/core/dbt/parser/results.py index 520ec128adf..ec44821d862 100644 --- a/core/dbt/parser/results.py +++ b/core/dbt/parser/results.py @@ -94,25 +94,21 @@ def add_macro(self, source_file: SourceFile, macro: ParsedMacro): # subtract 2 for the "Compilation Error" indent # note that the line wrap eats newlines, so if you want newlines, # this is the result :( - msg = '\n'.join([ - printer.line_wrap_message( - f'''\ - dbt found two macros named "{macro.name}" in the project - "{macro.package_name}". - ''', - subtract=2 - ), - '', - printer.line_wrap_message( - f'''\ - To fix this error, rename or remove one of the following - macros: - ''', - subtract=2 - ), - f' - {macro.original_file_path}', - f' - {other_path}' - ]) + msg = printer.line_wrap_message( + f'''\ + dbt found two macros named "{macro.name}" in the project + "{macro.package_name}". + + + To fix this error, rename or remove one of the following + macros: + + - {macro.original_file_path} + + - {other_path} + ''', + subtract=2 + ) raise_compiler_error(msg) self.macros[macro.unique_id] = macro diff --git a/core/dbt/ui/printer.py b/core/dbt/ui/printer.py index f785d75c424..e572c44c0d7 100644 --- a/core/dbt/ui/printer.py +++ b/core/dbt/ui/printer.py @@ -384,7 +384,17 @@ def print_run_end_messages(results, early_exit: bool = False) -> None: def line_wrap_message(msg: str, subtract: int = 0, dedent: bool = True) -> str: + ''' + Line wrap the given message to PRINTER_WIDTH - {subtract}. Convert double + newlines to newlines and avoid calling textwrap.fill() on them (like + markdown) + ''' width = PRINTER_WIDTH - subtract if dedent: msg = textwrap.dedent(msg) - return textwrap.fill(msg, width=width) + + # If the input had an explicit double newline, we want to preserve that + # (we'll turn it into a single line soon). Support windows, too. + splitter = '\r\n\r\n' if '\r\n\r\n' in msg else '\n\n' + chunks = msg.split(splitter) + return '\n'.join(textwrap.fill(chunk, width=width) for chunk in chunks) diff --git a/test/integration/012_deprecation_tests/test_deprecations.py b/test/integration/012_deprecation_tests/test_deprecations.py index 42e681dfbda..70af3f2c5d2 100644 --- a/test/integration/012_deprecation_tests/test_deprecations.py +++ b/test/integration/012_deprecation_tests/test_deprecations.py @@ -93,7 +93,8 @@ def test_postgres_deprecations_fail(self): # this should fail at compile_time with self.assertRaises(dbt.exceptions.CompilationException) as exc: self.run_dbt(strict=True) - self.assertIn('had resource type', str(exc.exception)) + exc_str = ' '.join(str(exc.exception).split()) # flatten all whitespace + self.assertIn('"seed" is a seed node, but it is specified in the models section', exc_str) @use_profile('postgres') def test_postgres_deprecations(self): From f0221b1fdb1e91595cce1088e2364d4869988bae Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Thu, 16 Jan 2020 13:27:24 -0700 Subject: [PATCH 3/6] expand search for docs and schema.yml update tests appease mypy --- core/dbt/config/profile.py | 153 ++++++++----- core/dbt/config/project.py | 176 ++++++++------ core/dbt/config/runtime.py | 91 +++----- core/dbt/contracts/project.py | 7 + core/dbt/parser/base.py | 4 - core/dbt/parser/schemas.py | 10 +- core/dbt/utils.py | 11 + .../029_docs_generate_tests/models/schema.yml | 14 -- .../029_docs_generate_tests/seed/schema.yml | 15 ++ .../test_docs_generate.py | 214 +++++++++++++++--- test/unit/test_config.py | 6 +- 11 files changed, 460 insertions(+), 241 deletions(-) create mode 100644 test/integration/029_docs_generate_tests/seed/schema.yml diff --git a/core/dbt/config/profile.py b/core/dbt/config/profile.py index db29e5aa360..81a04ba171b 100644 --- a/core/dbt/config/profile.py +++ b/core/dbt/config/profile.py @@ -1,9 +1,12 @@ +from dataclasses import dataclass +from typing import Any, Dict, Optional, Tuple import os from hologram import ValidationError from dbt.clients.system import load_file_contents from dbt.clients.yaml_helper import load_yaml_text +from dbt.contracts.connection import Credentials from dbt.contracts.project import ProfileConfig, UserConfig from dbt.exceptions import DbtProfileError from dbt.exceptions import DbtProjectError @@ -11,7 +14,7 @@ from dbt.exceptions import RuntimeException from dbt.exceptions import validator_error_message from dbt.logger import GLOBAL_LOGGER as logger -from dbt.utils import parse_cli_vars +from dbt.utils import parse_cli_vars, coerce_dict_str from .renderer import ConfigRenderer @@ -42,7 +45,7 @@ """.format(profiles_file=PROFILES_DIR) -def read_profile(profiles_dir): +def read_profile(profiles_dir: str) -> Dict[str, Any]: path = os.path.join(profiles_dir, 'profiles.yml') contents = None @@ -57,29 +60,32 @@ def read_profile(profiles_dir): return {} -def read_user_config(directory): +def read_user_config(directory: str) -> UserConfig: try: user_cfg = None profile = read_profile(directory) if profile: - user_cfg = profile.get('config', {}) + user_cfg = coerce_dict_str(profile.get('config', {})) + if user_cfg is not None: + return UserConfig.from_dict(user_cfg) + return UserConfig() + return UserConfig.from_dict(user_cfg) except (RuntimeException, ValidationError): return UserConfig() +@dataclass class Profile: - def __init__(self, profile_name, target_name, config, threads, - credentials): - self.profile_name = profile_name - self.target_name = target_name - if isinstance(config, dict): - config = UserConfig.from_dict(config) - self.config = config - self.threads = threads - self.credentials = credentials - - def to_profile_info(self, serialize_credentials=False): + profile_name: str + target_name: str + config: UserConfig + threads: int + credentials: Credentials + + def to_profile_info( + self, serialize_credentials: bool = False + ) -> Dict[str, Any]: """Unlike to_project_config, this dict is not a mirror of any existing on-disk data structure. It's used when creating a new profile from an existing one. @@ -91,21 +97,19 @@ def to_profile_info(self, serialize_credentials=False): result = { 'profile_name': self.profile_name, 'target_name': self.target_name, - 'config': self.config.to_dict(), + 'config': self.config, 'threads': self.threads, 'credentials': self.credentials, } if serialize_credentials: - result['credentials'] = result['credentials'].to_dict() + result['config'] = self.config.to_dict() + result['credentials'] = self.credentials.to_dict() return result - def __str__(self): - return str(self.to_profile_info()) - - def __eq__(self, other): + def __eq__(self, other: object) -> bool: if not (isinstance(other, self.__class__) and isinstance(self, other.__class__)): - return False + return NotImplemented return self.to_profile_info() == other.to_profile_info() def validate(self): @@ -119,7 +123,9 @@ def validate(self): raise DbtProfileError(validator_error_message(exc)) from exc @staticmethod - def _credentials_from_profile(profile, profile_name, target_name): + def _credentials_from_profile( + profile: Dict[str, Any], profile_name: str, target_name: str + ) -> Credentials: # avoid an import cycle from dbt.adapters.factory import load_plugin # credentials carry their 'type' in their actual type, not their @@ -143,7 +149,9 @@ def _credentials_from_profile(profile, profile_name, target_name): return credentials @staticmethod - def pick_profile_name(args_profile_name, project_profile_name=None): + def pick_profile_name( + args_profile_name: str, project_profile_name: Optional[str] = None, + ) -> str: profile_name = project_profile_name if args_profile_name is not None: profile_name = args_profile_name @@ -152,7 +160,9 @@ def pick_profile_name(args_profile_name, project_profile_name=None): return profile_name @staticmethod - def _get_profile_data(profile, profile_name, target_name): + def _get_profile_data( + profile: Dict[str, Any], profile_name: str, target_name: str + ) -> Dict[str, Any]: if 'outputs' not in profile: raise DbtProfileError( "outputs not specified in profile '{}'".format(profile_name) @@ -170,19 +180,25 @@ def _get_profile_data(profile, profile_name, target_name): return profile_data @classmethod - def from_credentials(cls, credentials, threads, profile_name, target_name, - user_cfg=None): + def from_credentials( + cls, + credentials: Credentials, + threads: int, + profile_name: str, + target_name: str, + user_cfg: Optional[Dict[str, Any]] = None + ) -> 'Profile': """Create a profile from an existing set of Credentials and the remaining information. - :param credentials dict: The credentials dict for this profile. - :param threads int: The number of threads to use for connections. - :param profile_name str: The profile name used for this profile. - :param target_name str: The target name used for this profile. - :param user_cfg Optional[dict]: The user-level config block from the + :param credentials: The credentials dict for this profile. + :param threads: The number of threads to use for connections. + :param profile_name: The profile name used for this profile. + :param target_name: The target name used for this profile. + :param user_cfg: The user-level config block from the raw profiles, if specified. :raises DbtProfileError: If the profile is invalid. - :returns Profile: The new Profile object. + :returns: The new Profile object. """ if user_cfg is None: user_cfg = {} @@ -199,8 +215,13 @@ def from_credentials(cls, credentials, threads, profile_name, target_name, return profile @classmethod - def render_profile(cls, raw_profile, profile_name, target_override, - cli_vars): + def render_profile( + cls, + raw_profile: Dict[str, Any], + profile_name: str, + target_override: Optional[str], + cli_vars: Dict[str, Any], + ) -> Tuple[str, Dict[str, Any]]: """This is a containment zone for the hateful way we're rendering profiles. """ @@ -233,27 +254,33 @@ def render_profile(cls, raw_profile, profile_name, target_override, return target_name, profile_data @classmethod - def from_raw_profile_info(cls, raw_profile, profile_name, cli_vars, - user_cfg=None, target_override=None, - threads_override=None): + def from_raw_profile_info( + cls, + raw_profile: Dict[str, Any], + profile_name: str, + cli_vars: Dict[str, Any], + user_cfg: Optional[Dict[str, Any]] = None, + target_override: Optional[str] = None, + threads_override: Optional[int] = None, + ) -> 'Profile': """Create a profile from its raw profile information. (this is an intermediate step, mostly useful for unit testing) - :param raw_profile dict: The profile data for a single profile, from + :param raw_profile: The profile data for a single profile, from disk as yaml and its values rendered with jinja. - :param profile_name str: The profile name used. - :param cli_vars dict: The command-line variables passed as arguments, + :param profile_name: The profile name used. + :param cli_vars: The command-line variables passed as arguments, as a dict. - :param user_cfg Optional[dict]: The global config for the user, if it + :param user_cfg: The global config for the user, if it was present. - :param target_override Optional[str]: The target to use, if provided on + :param target_override: The target to use, if provided on the command line. - :param threads_override Optional[str]: The thread count to use, if + :param threads_override: The thread count to use, if provided on the command line. :raises DbtProfileError: If the profile is invalid or missing, or the target could not be found - :returns Profile: The new Profile object. + :returns: The new Profile object. """ # user_cfg is not rendered. if user_cfg is None: @@ -270,7 +297,7 @@ def from_raw_profile_info(cls, raw_profile, profile_name, cli_vars, if threads_override is not None: threads = threads_override - credentials = cls._credentials_from_profile( + credentials: Credentials = cls._credentials_from_profile( profile_data, profile_name, target_name ) @@ -283,22 +310,28 @@ def from_raw_profile_info(cls, raw_profile, profile_name, cli_vars, ) @classmethod - def from_raw_profiles(cls, raw_profiles, profile_name, cli_vars, - target_override=None, threads_override=None): + def from_raw_profiles( + cls, + raw_profiles: Dict[str, Any], + profile_name: str, + cli_vars: Dict[str, Any], + target_override: Optional[str] = None, + threads_override: Optional[int] = None, + ) -> 'Profile': """ - :param raw_profiles dict: The profile data, from disk as yaml. - :param profile_name str: The profile name to use. - :param cli_vars dict: The command-line variables passed as arguments, - as a dict. - :param target_override Optional[str]: The target to use, if provided on - the command line. - :param threads_override Optional[str]: The thread count to use, if - provided on the command line. + :param raw_profiles: The profile data, from disk as yaml. + :param profile_name: The profile name to use. + :param cli_vars: The command-line variables passed as arguments, as a + dict. + :param target_override: The target to use, if provided on the command + line. + :param threads_override: The thread count to use, if provided on the + command line. :raises DbtProjectError: If there is no profile name specified in the project or the command line arguments :raises DbtProfileError: If the profile is invalid or missing, or the target could not be found - :returns Profile: The new Profile object. + :returns: The new Profile object. """ if profile_name not in raw_profiles: raise DbtProjectError( @@ -321,7 +354,11 @@ def from_raw_profiles(cls, raw_profiles, profile_name, cli_vars, ) @classmethod - def from_args(cls, args, project_profile_name=None): + def from_args( + cls, + args: Any, + project_profile_name: Optional[str] = None, + ) -> 'Profile': """Given the raw profiles as read from disk and the name of the desired profile if specified, return the profile component of the runtime config. diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 40a8554e074..620aa0773bc 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -1,4 +1,7 @@ from copy import deepcopy +from dataclasses import dataclass +from itertools import chain +from typing import List, Dict, Any, Optional, TypeVar, Union, Tuple, Callable import hashlib import os @@ -21,7 +24,10 @@ from dbt.source_config import SourceConfig from dbt.contracts.graph.manifest import ManifestMetadata -from dbt.contracts.project import Project as ProjectContract +from dbt.contracts.project import ( + Project as ProjectContract, + SemverString, +) from dbt.contracts.project import PackageConfig from hologram import ValidationError @@ -129,7 +135,7 @@ def package_config_from_data(packages_data): return packages -def _parse_versions(versions): +def _parse_versions(versions: Union[List[str], str]) -> List[VersionSpecifier]: """Parse multiple versions as read from disk. The versions value may be any one of: - a single version string ('>0.12.1') @@ -144,44 +150,61 @@ def _parse_versions(versions): return [VersionSpecifier.from_version_string(v) for v in versions] +def _all_source_paths( + source_paths: List[str], data_paths: List[str], snapshot_paths: List[str] +) -> List[str]: + return list(chain(source_paths, data_paths, snapshot_paths)) + + +T = TypeVar('T') + + +def value_or(value: Optional[T], default: T) -> T: + if value is None: + return default + else: + return value + + +@dataclass class Project: - def __init__(self, project_name, version, project_root, profile_name, - source_paths, macro_paths, data_paths, test_paths, - analysis_paths, docs_paths, target_path, snapshot_paths, - clean_targets, log_path, modules_path, quoting, models, - on_run_start, on_run_end, seeds, snapshots, dbt_version, - packages, query_comment): - self.project_name = project_name - self.version = version - self.project_root = project_root - self.profile_name = profile_name - self.source_paths = source_paths - self.macro_paths = macro_paths - self.data_paths = data_paths - self.test_paths = test_paths - self.analysis_paths = analysis_paths - self.docs_paths = docs_paths - self.target_path = target_path - self.snapshot_paths = snapshot_paths - self.clean_targets = clean_targets - self.log_path = log_path - self.modules_path = modules_path - self.quoting = quoting - self.models = models - self.on_run_start = on_run_start - self.on_run_end = on_run_end - self.seeds = seeds - self.snapshots = snapshots - self.dbt_version = dbt_version - self.packages = packages - self.query_comment = query_comment + project_name: str + version: Union[SemverString, float] + project_root: str + profile_name: str + source_paths: List[str] + macro_paths: List[str] + data_paths: List[str] + test_paths: List[str] + analysis_paths: List[str] + docs_paths: List[str] + target_path: str + snapshot_paths: List[str] + clean_targets: List[str] + log_path: str + modules_path: str + quoting: Dict[str, Any] + models: Dict[str, Any] + on_run_start: List[str] + on_run_end: List[str] + seeds: Dict[str, Any] + snapshots: Dict[str, Any] + dbt_version: List[VersionSpecifier] + packages: Dict[str, Any] + query_comment: Optional[Union[str, NoValue]] + + @property + def all_source_paths(self) -> List[str]: + return _all_source_paths( + self.source_paths, self.data_paths, self.snapshot_paths + ) @staticmethod - def _preprocess(project_dict): + def _preprocess(project_dict: Dict[str, Any]) -> Dict[str, Any]: """Pre-process certain special keys to convert them from None values into empty containers, and to turn strings into arrays of strings. """ - handlers = { + handlers: Dict[Tuple[str, ...], Callable[[Any], Any]] = { ('on-run-start',): _list_if_none_or_string, ('on-run-end',): _list_if_none_or_string, } @@ -193,7 +216,7 @@ def _preprocess(project_dict): handlers[(k, 'post-hook')] = _list_if_none_or_string handlers[('seeds', 'column_types')] = _dict_if_none - def converter(value, keypath): + def converter(value: Any, keypath: Tuple[str, ...]) -> Any: if keypath in handlers: handler = handlers[keypath] return handler(value) @@ -203,16 +226,20 @@ def converter(value, keypath): return deep_map(converter, project_dict) @classmethod - def from_project_config(cls, project_dict, packages_dict=None): + def from_project_config( + cls, + project_dict: Dict[str, Any], + packages_dict: Optional[Dict[str, Any]] = None, + ) -> 'Project': """Create a project from its project and package configuration, as read by yaml.safe_load(). - :param project_dict dict: The dictionary as read from disk - :param packages_dict Optional[dict]: If it exists, the packages file as + :param project_dict: The dictionary as read from disk + :param packages_dict: If it exists, the packages file as read from disk. :raises DbtProjectError: If the project is missing or invalid, or if the packages file exists and is invalid. - :returns Project: The project, with defaults populated. + :returns: The project, with defaults populated. """ try: project_dict = cls._preprocess(project_dict) @@ -221,45 +248,60 @@ def from_project_config(cls, project_dict, packages_dict=None): 'Cycle detected: Project input has a reference to itself', project=project_dict ) - # just for validation. try: - ProjectContract.from_dict(project_dict) + cfg = ProjectContract.from_dict(project_dict) except ValidationError as e: raise DbtProjectError(validator_error_message(e)) from e # name/version are required in the Project definition, so we can assume # they are present - name = project_dict['name'] - version = project_dict['version'] + name = cfg.name + version = cfg.version # this is added at project_dict parse time and should always be here # once we see it. - project_root = project_dict['project-root'] + if cfg.project_root is None: + raise DbtProjectError('cfg must have a project root!') + else: + project_root = cfg.project_root # this is only optional in the sense that if it's not present, it needs # to have been a cli argument. - profile_name = project_dict.get('profile') - # these are optional - source_paths = project_dict.get('source-paths', ['models']) - macro_paths = project_dict.get('macro-paths', ['macros']) - data_paths = project_dict.get('data-paths', ['data']) - test_paths = project_dict.get('test-paths', ['test']) - analysis_paths = project_dict.get('analysis-paths', []) - docs_paths = project_dict.get('docs-paths', source_paths[:]) - target_path = project_dict.get('target-path', 'target') - snapshot_paths = project_dict.get('snapshot-paths', ['snapshots']) - # should this also include the modules path by default? - clean_targets = project_dict.get('clean-targets', [target_path]) - log_path = project_dict.get('log-path', 'logs') - modules_path = project_dict.get('modules-path', 'dbt_modules') + profile_name = cfg.profile + # these are all the defaults + source_paths: List[str] = value_or(cfg.source_paths, ['models']) + macro_paths: List[str] = value_or(cfg.macro_paths, ['macros']) + data_paths: List[str] = value_or(cfg.data_paths, ['data']) + test_paths: List[str] = value_or(cfg.test_paths, ['test']) + analysis_paths: List[str] = value_or(cfg.analysis_paths, []) + snapshot_paths: List[str] = value_or(cfg.snapshot_paths, ['snapshots']) + + all_source_paths: List[str] = _all_source_paths( + source_paths, data_paths, snapshot_paths + ) + + docs_paths: List[str] = value_or(cfg.docs_paths, all_source_paths) + target_path: str = value_or(cfg.target_path, 'target') + clean_targets: List[str] = value_or(cfg.clean_targets, [target_path]) + log_path: str = value_or(cfg.log_path, 'logs') + modules_path: str = value_or(cfg.modules_path, 'dbt_modules') # in the default case we'll populate this once we know the adapter type - quoting = project_dict.get('quoting', {}) - - models = project_dict.get('models', {}) - on_run_start = project_dict.get('on-run-start', []) - on_run_end = project_dict.get('on-run-end', []) - seeds = project_dict.get('seeds', {}) - snapshots = project_dict.get('snapshots', {}) - dbt_raw_version = project_dict.get('require-dbt-version', '>=0.0.0') - query_comment = project_dict.get('query-comment', NoValue()) + # It would be nice to just pass along a Quoting here, but that would + # break many things + quoting: Dict[str, Any] = {} + if cfg.quoting is not None: + quoting = cfg.quoting.to_dict() + + models: Dict[str, Any] = cfg.models + seeds: Dict[str, Any] = cfg.seeds + snapshots: Dict[str, Any] = cfg.snapshots + + on_run_start: List[str] = value_or(cfg.on_run_start, []) + on_run_end: List[str] = value_or(cfg.on_run_end, []) + + # weird type handling: no value_or use + dbt_raw_version: Union[List[str], str] = '>=0.0.0' + if cfg.require_dbt_version is not None: + dbt_raw_version = cfg.require_dbt_version + query_comment = cfg.query_comment try: dbt_version = _parse_versions(dbt_raw_version) diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index 6ea50bfc275..29270571940 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -1,4 +1,6 @@ from copy import deepcopy +from dataclasses import dataclass +from typing import Dict, Any, Optional from .profile import Profile from .project import Project @@ -12,74 +14,36 @@ from hologram import ValidationError +@dataclass class RuntimeConfig(Project, Profile): """The runtime configuration, as constructed from its components. There's a lot because there is a lot of stuff! """ - def __init__(self, project_name, version, project_root, source_paths, - macro_paths, data_paths, test_paths, analysis_paths, - docs_paths, target_path, snapshot_paths, clean_targets, - log_path, modules_path, quoting, models, on_run_start, - on_run_end, seeds, snapshots, dbt_version, profile_name, - target_name, config, threads, credentials, packages, - query_comment, args): - # 'vars' - self.args = args - self.cli_vars = parse_cli_vars(getattr(args, 'vars', '{}')) - # 'project' - Project.__init__( - self, - project_name=project_name, - version=version, - project_root=project_root, - profile_name=profile_name, - source_paths=source_paths, - macro_paths=macro_paths, - data_paths=data_paths, - test_paths=test_paths, - analysis_paths=analysis_paths, - docs_paths=docs_paths, - target_path=target_path, - snapshot_paths=snapshot_paths, - clean_targets=clean_targets, - log_path=log_path, - modules_path=modules_path, - quoting=quoting, - models=models, - on_run_start=on_run_start, - on_run_end=on_run_end, - seeds=seeds, - snapshots=snapshots, - dbt_version=dbt_version, - packages=packages, - query_comment=query_comment, - ) - # 'profile' - Profile.__init__( - self, - profile_name=profile_name, - target_name=target_name, - config=config, - threads=threads, - credentials=credentials - ) + args: Any + cli_vars: Dict[str, Any] + + def __post_init__(self): self.validate() @classmethod - def from_parts(cls, project, profile, args): + def from_parts( + cls, project: Project, profile: Profile, args: Any, + ) -> 'RuntimeConfig': """Instantiate a RuntimeConfig from its components. - :param profile Profile: A parsed dbt Profile. - :param project Project: A parsed dbt Project. - :param args argparse.Namespace: The parsed command-line arguments. + :param profile: A parsed dbt Profile. + :param project: A parsed dbt Project. + :param args: The parsed command-line arguments. :returns RuntimeConfig: The new configuration. """ - quoting = ( + quoting: Dict[str, Any] = ( get_relation_class_by_name(profile.credentials.type) .get_default_quote_policy() .replace_dict(project.quoting) ).to_dict() + cli_vars: Dict[str, Any] = parse_cli_vars(getattr(args, 'vars', '{}')) + return cls( project_name=project.project_name, version=project.version, @@ -109,17 +73,18 @@ def from_parts(cls, project, profile, args): config=profile.config, threads=profile.threads, credentials=profile.credentials, - args=args + args=args, + cli_vars=cli_vars, ) - def new_project(self, project_root): + def new_project(self, project_root: str) -> 'RuntimeConfig': """Given a new project root, read in its project dictionary, supply the existing project's profile info, and create a new project file. - :param project_root str: A filepath to a dbt project. + :param project_root: A filepath to a dbt project. :raises DbtProfileError: If the profile is invalid. :raises DbtProjectError: If project is missing or invalid. - :returns RuntimeConfig: The new configuration. + :returns: The new configuration. """ # copy profile profile = Profile(**self.to_profile_info()) @@ -136,7 +101,7 @@ def new_project(self, project_root): cfg.quoting = deepcopy(self.quoting) return cfg - def serialize(self): + def serialize(self) -> Dict[str, Any]: """Serialize the full configuration to a single dictionary. For any instance that has passed validate() (which happens in __init__), it matches the Configuration contract. @@ -150,9 +115,6 @@ def serialize(self): result['cli_vars'] = deepcopy(self.cli_vars) return result - def __str__(self): - return str(self.serialize()) - def validate(self): """Validate the configuration against its contract. @@ -167,16 +129,21 @@ def validate(self): self.validate_version() @classmethod - def from_args(cls, args): + def from_args( + cls, args: Any, project_profile_name: Optional[str] = None + ) -> 'RuntimeConfig': """Given arguments, read in dbt_project.yml from the current directory, read in packages.yml if it exists, and use them to find the profile to load. - :param args argparse.Namespace: The arguments as parsed from the cli. + :param args: The arguments as parsed from the cli. :raises DbtProjectError: If the project is invalid or missing. :raises DbtProfileError: If the profile is invalid or missing. :raises ValidationException: If the cli variables are invalid. """ + # project_profile_name is ignored, we just need it to appease mypy + # (Profile.from_args uses it) + # build the project and read in packages.yml project = Project.from_args(args) diff --git a/core/dbt/contracts/project.py b/core/dbt/contracts/project.py index e13e5c2b2b2..c6ce52a422b 100644 --- a/core/dbt/contracts/project.py +++ b/core/dbt/contracts/project.py @@ -193,6 +193,13 @@ def set_values(self, cookie_dir): if self.printer_width: printer.printer_width(self.printer_width) + @classmethod + def from_maybe_dict(cls, value: Optional[Dict[str, Any]]) -> 'UserConfig': + if value is None: + return cls() + else: + return cls.from_dict(value) + @dataclass class ProfileConfig(HyphenatedJsonSchemaMixin, Replaceable): diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 4a27d13789d..ab13cf8c74f 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -79,10 +79,6 @@ def load_file(self, path: FilePath) -> SourceFile: source_file.contents = file_contents.strip() return source_file - def parse_file_from_path(self, path: FilePath): - block = FileBlock(file=self.load_file(path)) - self.parse_file(block) - class Parser(BaseParser[FinalValue], Generic[FinalValue]): def __init__( diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 87c485bff19..86094721a50 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -34,7 +34,7 @@ TestBuilder, SourceTarget, NodeTarget, Target, SchemaTestBlock, TargetBlock, YamlBlock, ) -from dbt.utils import get_pseudo_test_path +from dbt.utils import get_pseudo_test_path, coerce_dict_str UnparsedSchemaYaml = Union[UnparsedSourceDefinition, UnparsedNodeUpdate] @@ -118,7 +118,7 @@ def resource_type(self) -> NodeType: def get_paths(self): return FilesystemSearcher( - self.project, self.project.source_paths, '.yml' + self.project, self.project.all_source_paths, '.yml' ) def parse_from_dict(self, dct, validate=True) -> ParsedTestNode: @@ -314,11 +314,7 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: path = self.yaml.path.original_file_path for entry in data: - str_keys = ( - isinstance(entry, dict) and - all(isinstance(k, str) for k in entry) - ) - if str_keys: + if coerce_dict_str(entry) is not None: yield entry else: msg = error_context( diff --git a/core/dbt/utils.py b/core/dbt/utils.py index 4e8d865f024..d8dd6547d72 100644 --- a/core/dbt/utils.py +++ b/core/dbt/utils.py @@ -532,6 +532,17 @@ def restrict_to(*restrictions): return {'restrict': list(restrictions)} +def coerce_dict_str(value: Any) -> Optional[Dict[str, Any]]: + """For annoying mypy reasons, this helper makes dealing with nested dicts + easier. You get either `None` if it's not a Dict[str, Any], or the + Dict[str, Any] you expected (to pass it to JsonSchemaMixin.from_dict(...)). + """ + if (isinstance(value, dict) and all(isinstance(k, str) for k in value)): + return value + else: + return None + + # some types need to make constants available to the jinja context as # attributes, and regular properties only work with objects. maybe this should # be handled by the RelationProxy? diff --git a/test/integration/029_docs_generate_tests/models/schema.yml b/test/integration/029_docs_generate_tests/models/schema.yml index 7f0d9518a63..6f3fc661976 100644 --- a/test/integration/029_docs_generate_tests/models/schema.yml +++ b/test/integration/029_docs_generate_tests/models/schema.yml @@ -19,17 +19,3 @@ models: description: The last time this user's email was updated tests: - test.nothing -seeds: - - name: seed - description: "The test seed" - columns: - - name: id - description: The user ID number - - name: first_name - description: The user's first name - - name: email - description: The user's email - - name: ip_address - description: The user's IP address - - name: updated_at - description: The last time this user's email was updated diff --git a/test/integration/029_docs_generate_tests/seed/schema.yml b/test/integration/029_docs_generate_tests/seed/schema.yml new file mode 100644 index 00000000000..ef5e7dc9efb --- /dev/null +++ b/test/integration/029_docs_generate_tests/seed/schema.yml @@ -0,0 +1,15 @@ +version: 2 +seeds: + - name: seed + description: "The test seed" + columns: + - name: id + description: The user ID number + - name: first_name + description: The user's first name + - name: email + description: The user's email + - name: ip_address + description: The user's IP address + - name: updated_at + description: The last time this user's email was updated diff --git a/test/integration/029_docs_generate_tests/test_docs_generate.py b/test/integration/029_docs_generate_tests/test_docs_generate.py index 4d518aacfdf..07770908068 100644 --- a/test/integration/029_docs_generate_tests/test_docs_generate.py +++ b/test/integration/029_docs_generate_tests/test_docs_generate.py @@ -848,7 +848,9 @@ def verify_manifest_macros(self, manifest): def expected_seeded_manifest(self, model_database=None): models_path = self.dir('models') model_sql_path = os.path.join(models_path, 'model.sql') - schema_yml_path = os.path.join(models_path, 'schema.yml') + model_schema_yml_path = os.path.join(models_path, 'schema.yml') + seed_schema_yml_path = os.path.join(self.dir('seed'), 'schema.yml') + my_schema_name = self.unique_schema() if model_database is None: @@ -924,7 +926,7 @@ def expected_seeded_manifest(self, model_database=None): 'meta': {}, }, }, - 'patch_path': schema_yml_path, + 'patch_path': model_schema_yml_path, 'docrefs': [], 'compiled': True, 'compiled_sql': ANY, @@ -949,7 +951,7 @@ def expected_seeded_manifest(self, model_database=None): 'tags': [], 'quote_columns': True, }, - 'patch_path': schema_yml_path, + 'patch_path': seed_schema_yml_path, 'path': 'seed.csv', 'name': 'seed', 'root_path': self.test_root_dir, @@ -1032,7 +1034,7 @@ def expected_seeded_manifest(self, model_database=None): 'description': '', 'fqn': ['test', 'schema_test', 'not_null_model_id'], 'name': 'not_null_model_id', - 'original_file_path': schema_yml_path, + 'original_file_path': model_schema_yml_path, 'package_name': 'test', 'patch_path': None, 'path': Normalized('schema_test/not_null_model_id.sql'), @@ -1080,7 +1082,7 @@ def expected_seeded_manifest(self, model_database=None): 'description': '', 'fqn': ['test', 'schema_test', 'test_nothing_model_'], 'name': 'test_nothing_model_', - 'original_file_path': schema_yml_path, + 'original_file_path': model_schema_yml_path, 'package_name': 'test', 'patch_path': None, 'path': normalize('schema_test/test_nothing_model_.sql'), @@ -1128,7 +1130,7 @@ def expected_seeded_manifest(self, model_database=None): 'description': '', 'fqn': ['test', 'schema_test', 'unique_model_id'], 'name': 'unique_model_id', - 'original_file_path': schema_yml_path, + 'original_file_path': model_schema_yml_path, 'package_name': 'test', 'patch_path': None, 'path': normalize('schema_test/unique_model_id.sql'), @@ -1228,7 +1230,16 @@ def expected_seeded_manifest(self, model_database=None): 'docs': [], 'macros': [], 'nodes': ['test.test.unique_model_id', 'test.test.not_null_model_id', 'test.test.test_nothing_model_'], - 'patches': ['model', 'seed'], + 'patches': ['model'], + 'sources': [], + }, + normalize('seed/schema.yml'): { + 'path': self._path_to('seed', 'schema.yml'), + 'checksum': self._checksum_file('seed/schema.yml'), + 'docs': [], + 'macros': [], + 'nodes': [], + 'patches': ['seed'], 'sources': [], }, }, @@ -1466,7 +1477,38 @@ def expected_postgres_references_manifest(self, model_database=None): 'seed.test.seed': { 'alias': 'seed', 'build_path': None, - 'columns': {}, + 'columns': { + 'id': { + 'name': 'id', + 'description': 'The user ID number', + 'data_type': None, + 'meta': {}, + }, + 'first_name': { + 'name': 'first_name', + 'description': "The user's first name", + 'data_type': None, + 'meta': {}, + }, + 'email': { + 'name': 'email', + 'description': "The user's email", + 'data_type': None, + 'meta': {}, + }, + 'ip_address': { + 'name': 'ip_address', + 'description': "The user's IP address", + 'data_type': None, + 'meta': {}, + }, + 'updated_at': { + 'name': 'updated_at', + 'description': "The last time this user's email was updated", + 'data_type': None, + 'meta': {}, + }, + }, 'config': { 'column_types': {}, 'enabled': True, @@ -1481,13 +1523,13 @@ def expected_postgres_references_manifest(self, model_database=None): }, 'sources': [], 'depends_on': {'macros': [], 'nodes': []}, - 'description': '', + 'description': 'The test seed', 'docrefs': [], 'fqn': ['test', 'seed'], 'name': 'seed', 'original_file_path': self.dir('seed/seed.csv'), 'package_name': 'test', - 'patch_path': None, + 'patch_path': self.dir('seed/schema.yml'), 'path': 'seed.csv', 'raw_sql': '', 'refs': [], @@ -1735,7 +1777,15 @@ def expected_postgres_references_manifest(self, model_database=None): 'path': self._path_to('ref_models', 'schema.yml'), 'sources': ['source.test.my_source.my_table'], }, - + normalize('seed/schema.yml'): { + 'path': self._path_to('seed', 'schema.yml'), + 'checksum': self._checksum_file('seed/schema.yml'), + 'docs': [], + 'macros': [], + 'nodes': [], + 'patches': ['seed'], + 'sources': [], + }, }, } @@ -2017,7 +2067,7 @@ def expected_bigquery_complex_manifest(self): }, 'seed.test.seed': { 'build_path': None, - 'patch_path': None, + 'patch_path': self.dir('seed/schema.yml'), 'path': 'seed.csv', 'name': 'seed', 'root_path': self.test_root_dir, @@ -2051,8 +2101,39 @@ def expected_bigquery_complex_manifest(self): 'schema': my_schema_name, 'database': self.default_database, 'alias': 'seed', - 'columns': {}, - 'description': '', + 'columns': { + 'id': { + 'name': 'id', + 'description': 'The user ID number', + 'data_type': None, + 'meta': {}, + }, + 'first_name': { + 'name': 'first_name', + 'description': "The user's first name", + 'data_type': None, + 'meta': {}, + }, + 'email': { + 'name': 'email', + 'description': "The user's email", + 'data_type': None, + 'meta': {}, + }, + 'ip_address': { + 'name': 'ip_address', + 'description': "The user's IP address", + 'data_type': None, + 'meta': {}, + }, + 'updated_at': { + 'name': 'updated_at', + 'description': "The last time this user's email was updated", + 'data_type': None, + 'meta': {}, + }, + }, + 'description': 'The test seed', 'docrefs': [], 'compiled': True, 'compiled_sql': '', @@ -2152,6 +2233,15 @@ def expected_bigquery_complex_manifest(self): 'macros': [], 'sources': [], }, + normalize('seed/schema.yml'): { + 'path': self._path_to('seed', 'schema.yml'), + 'checksum': self._checksum_file('seed/schema.yml'), + 'docs': [], + 'macros': [], + 'nodes': [], + 'patches': ['seed'], + 'sources': [], + }, }, } @@ -2265,7 +2355,7 @@ def expected_redshift_incremental_view_manifest(self): }, 'seed.test.seed': { 'build_path': None, - 'patch_path': None, + 'patch_path': self.dir('seed/schema.yml'), 'path': 'seed.csv', 'name': 'seed', 'root_path': self.test_root_dir, @@ -2299,8 +2389,39 @@ def expected_redshift_incremental_view_manifest(self): 'schema': my_schema_name, 'database': self.default_database, 'alias': 'seed', - 'columns': {}, - 'description': '', + 'columns': { + 'id': { + 'name': 'id', + 'description': 'The user ID number', + 'data_type': None, + 'meta': {}, + }, + 'first_name': { + 'name': 'first_name', + 'description': "The user's first name", + 'data_type': None, + 'meta': {}, + }, + 'email': { + 'name': 'email', + 'description': "The user's email", + 'data_type': None, + 'meta': {}, + }, + 'ip_address': { + 'name': 'ip_address', + 'description': "The user's IP address", + 'data_type': None, + 'meta': {}, + }, + 'updated_at': { + 'name': 'updated_at', + 'description': "The last time this user's email was updated", + 'data_type': None, + 'meta': {}, + }, + }, + 'description': 'The test seed', 'docrefs': [], 'compiled': True, 'compiled_sql': ANY, @@ -2367,6 +2488,15 @@ def expected_redshift_incremental_view_manifest(self): 'patches': ['model'], 'sources': [] }, + normalize('seed/schema.yml'): { + 'path': self._path_to('seed', 'schema.yml'), + 'checksum': self._checksum_file('seed/schema.yml'), + 'docs': [], + 'macros': [], + 'nodes': [], + 'patches': ['seed'], + 'sources': [], + }, }, } @@ -2418,7 +2548,8 @@ def expected_run_results(self, quote_schema=True, quote_model=False, """ models_path = self.dir('models') model_sql_path = os.path.join(models_path, 'model.sql') - schema_yml_path = os.path.join(models_path, 'schema.yml') + model_schema_yml_path = os.path.join(models_path, 'schema.yml') + seed_schema_yml_path = os.path.join(self.dir('seed'), 'schema.yml') if model_database is None: model_database = self.alternative_database @@ -2516,7 +2647,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'name': 'model', 'original_file_path': model_sql_path, 'package_name': 'test', - 'patch_path': schema_yml_path, + 'patch_path': model_schema_yml_path, 'path': 'model.sql', 'raw_sql': LineIndifferent(_read_file(model_sql_path).rstrip('\r\n')), 'refs': [['seed']], @@ -2599,7 +2730,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'name': 'seed', 'original_file_path': self.dir('seed/seed.csv'), 'package_name': 'test', - 'patch_path': schema_yml_path, + 'patch_path': seed_schema_yml_path, 'path': 'seed.csv', 'raw_sql': '', 'refs': [], @@ -2651,7 +2782,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'injected_sql': AnyStringWith('id is null'), 'meta': {}, 'name': 'not_null_model_id', - 'original_file_path': schema_yml_path, + 'original_file_path': model_schema_yml_path, 'package_name': 'test', 'patch_path': None, 'path': Normalized('schema_test/not_null_model_id.sql'), @@ -2709,7 +2840,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'injected_sql': AnyStringWith('select 0'), 'meta': {}, 'name': 'test_nothing_model_', - 'original_file_path': schema_yml_path, + 'original_file_path': model_schema_yml_path, 'package_name': 'test', 'patch_path': None, 'path': Normalized('schema_test/test_nothing_model_.sql'), @@ -2767,7 +2898,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False, 'injected_sql': AnyStringWith('count(*)'), 'meta': {}, 'name': 'unique_model_id', - 'original_file_path': schema_yml_path, + 'original_file_path': model_schema_yml_path, 'package_name': 'test', 'patch_path': None, 'path': Normalized('schema_test/unique_model_id.sql'), @@ -3013,7 +3144,38 @@ def expected_postgres_references_run_results(self): 'node': { 'alias': 'seed', 'build_path': None, - 'columns': {}, + 'columns': { + 'id': { + 'name': 'id', + 'description': 'The user ID number', + 'data_type': None, + 'meta': {}, + }, + 'first_name': { + 'name': 'first_name', + 'description': "The user's first name", + 'data_type': None, + 'meta': {}, + }, + 'email': { + 'name': 'email', + 'description': "The user's email", + 'data_type': None, + 'meta': {}, + }, + 'ip_address': { + 'name': 'ip_address', + 'description': "The user's IP address", + 'data_type': None, + 'meta': {}, + }, + 'updated_at': { + 'name': 'updated_at', + 'description': "The last time this user's email was updated", + 'data_type': None, + 'meta': {}, + }, + }, 'compiled': True, 'compiled_sql': '', 'config': { @@ -3030,7 +3192,7 @@ def expected_postgres_references_run_results(self): }, 'sources': [], 'depends_on': {'macros': [], 'nodes': []}, - 'description': '', + 'description': 'The test seed', 'docrefs': [], 'extra_ctes': [], 'extra_ctes_injected': True, @@ -3040,7 +3202,7 @@ def expected_postgres_references_run_results(self): 'name': 'seed', 'original_file_path': self.dir('seed/seed.csv'), 'package_name': 'test', - 'patch_path': None, + 'patch_path': self.dir('seed/schema.yml'), 'path': 'seed.csv', 'raw_sql': '', 'refs': [], diff --git a/test/unit/test_config.py b/test/unit/test_config.py index b8eb444dd14..eea3e25ddec 100644 --- a/test/unit/test_config.py +++ b/test/unit/test_config.py @@ -537,7 +537,7 @@ def test_defaults(self): self.assertEqual(project.data_paths, ['data']) self.assertEqual(project.test_paths, ['test']) self.assertEqual(project.analysis_paths, []) - self.assertEqual(project.docs_paths, ['models']) + self.assertEqual(project.docs_paths, ['models', 'data', 'snapshots']) self.assertEqual(project.target_path, 'target') self.assertEqual(project.clean_targets, ['target']) self.assertEqual(project.log_path, 'logs') @@ -577,7 +577,7 @@ def test_implicit_overrides(self): project = dbt.config.Project.from_project_config( self.default_project_data ) - self.assertEqual(project.docs_paths, ['other-models']) + self.assertEqual(project.docs_paths, ['other-models', 'data', 'snapshots']) self.assertEqual(project.clean_targets, ['other-target']) def test_hashed_name(self): @@ -1089,7 +1089,7 @@ def test_from_args(self): self.assertEqual(config.data_paths, ['data']) self.assertEqual(config.test_paths, ['test']) self.assertEqual(config.analysis_paths, []) - self.assertEqual(config.docs_paths, ['models']) + self.assertEqual(config.docs_paths, ['models', 'data', 'snapshots']) self.assertEqual(config.target_path, 'target') self.assertEqual(config.clean_targets, ['target']) self.assertEqual(config.log_path, 'logs') From 7a89ef24654d0526a91f6a27993a9d7ac5ab7a54 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 22 Jan 2020 10:20:26 -0700 Subject: [PATCH 4/6] PR feedback --- core/dbt/config/runtime.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index 29270571940..3a190792ef5 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -16,9 +16,6 @@ @dataclass class RuntimeConfig(Project, Profile): - """The runtime configuration, as constructed from its components. There's a - lot because there is a lot of stuff! - """ args: Any cli_vars: Dict[str, Any] From 21c3f41814239e1df9bc1589480a6de68d8f880d Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 22 Jan 2020 11:39:02 -0700 Subject: [PATCH 5/6] fix dedent/formatting with "Deprecation Warning" prefix --- core/dbt/deprecations.py | 18 +++++++++--------- core/dbt/ui/printer.py | 7 ++++++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index 701452f52f2..ee6d5fa14b7 100644 --- a/core/dbt/deprecations.py +++ b/core/dbt/deprecations.py @@ -29,7 +29,7 @@ def description(self) -> str: def show(self, *args, **kwargs) -> None: if self.name not in active_deprecations: desc = self.description.format(**kwargs) - msg = printer.line_wrap_message(f"* Deprecation Warning: {desc}\n") + msg = printer.line_wrap_message(desc, prefix='* Deprecation Warning: ') dbt.exceptions.warn_or_error(msg) active_deprecations.add(self.name) @@ -63,7 +63,7 @@ class MaterializationReturnDeprecation(DBTDeprecation): For more information, see: https://docs.getdbt.com/v0.15/docs/creating-new-materializations#section-6-returning-relations - '''.lstrip() + ''' class NotADictionaryDeprecation(DBTDeprecation): @@ -72,7 +72,7 @@ class NotADictionaryDeprecation(DBTDeprecation): _description = '''\ The object ("{obj}") was used as a dictionary. In a future version of dbt this capability will be removed from objects of this type. - '''.lstrip() + ''' class ColumnQuotingDeprecation(DBTDeprecation): @@ -94,16 +94,16 @@ class ModelsKeyNonModelDeprecation(DBTDeprecation): _name = 'models-key-mismatch' _description = '''\ - "{node.name}" is a {node.resource_type} node, but it is specified in - the {patch.yaml_key} section of {patch.original_file_path}. + "{node.name}" is a {node.resource_type} node, but it is specified in + the {patch.yaml_key} section of {patch.original_file_path}. - To fix this warning, place the `{node.name}` specification under - the {expected_key} key instead. + To fix this warning, place the `{node.name}` specification under + the {expected_key} key instead. - This warning will become an error in a future release. - '''.strip() + This warning will become an error in a future release. + ''' _adapter_renamed_description = """\ diff --git a/core/dbt/ui/printer.py b/core/dbt/ui/printer.py index e572c44c0d7..eff5a3380cf 100644 --- a/core/dbt/ui/printer.py +++ b/core/dbt/ui/printer.py @@ -383,7 +383,9 @@ def print_run_end_messages(results, early_exit: bool = False) -> None: print_run_status_line(results) -def line_wrap_message(msg: str, subtract: int = 0, dedent: bool = True) -> str: +def line_wrap_message( + msg: str, subtract: int = 0, dedent: bool = True, prefix: str = '' +) -> str: ''' Line wrap the given message to PRINTER_WIDTH - {subtract}. Convert double newlines to newlines and avoid calling textwrap.fill() on them (like @@ -393,6 +395,9 @@ def line_wrap_message(msg: str, subtract: int = 0, dedent: bool = True) -> str: if dedent: msg = textwrap.dedent(msg) + if prefix: + msg = f'{prefix}{msg}' + # If the input had an explicit double newline, we want to preserve that # (we'll turn it into a single line soon). Support windows, too. splitter = '\r\n\r\n' if '\r\n\r\n' in msg else '\n\n' From 8bdaa69a2c4736e19c7dab102c07daedf988109e Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Thu, 23 Jan 2020 11:08:21 -0700 Subject: [PATCH 6/6] flake8 --- core/dbt/deprecations.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index ee6d5fa14b7..1996d62c145 100644 --- a/core/dbt/deprecations.py +++ b/core/dbt/deprecations.py @@ -29,7 +29,9 @@ def description(self) -> str: def show(self, *args, **kwargs) -> None: if self.name not in active_deprecations: desc = self.description.format(**kwargs) - msg = printer.line_wrap_message(desc, prefix='* Deprecation Warning: ') + msg = printer.line_wrap_message( + desc, prefix='* Deprecation Warning: ' + ) dbt.exceptions.warn_or_error(msg) active_deprecations.add(self.name)