Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
add support for newlines in output
line-wrap deprecations
  • Loading branch information
Jacob Beck committed Jan 16, 2020
1 parent f92f389 commit c4a5eb9
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 81 deletions.
37 changes: 24 additions & 13 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
78 changes: 39 additions & 39 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import dbt.links
import dbt.exceptions
import dbt.flags
from dbt.ui import printer


class DBTDeprecation:
Expand All @@ -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()
Expand All @@ -87,32 +78,42 @@ 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
'''


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):
Expand Down Expand Up @@ -143,7 +144,6 @@ def warn(name, *args, **kwargs):
active_deprecations: Set[str] = set()

deprecations_list: List[DBTDeprecation] = [
DBTRepositoriesDeprecation(),
GenerateSchemaNameSingleArgDeprecated(),
MaterializationReturnDeprecation(),
NotADictionaryDeprecation(),
Expand Down
15 changes: 8 additions & 7 deletions core/dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 0 additions & 1 deletion core/dbt/node_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ def documentable(cls) -> List['NodeType']:
cls.Model,
cls.Seed,
cls.Snapshot,
cls.Analysis,
cls.Source,
]

Expand Down
34 changes: 15 additions & 19 deletions core/dbt/parser/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion core/dbt/ui/printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
3 changes: 2 additions & 1 deletion test/integration/012_deprecation_tests/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit c4a5eb9

Please sign in to comment.