diff --git a/CHANGELOG.md b/CHANGELOG.md index 0021a9f9ff4..ab5dda16639 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Added state and defer arguments to the RPC client, matching the CLI ([#2678](https://github.com/fishtown-analytics/dbt/issues/2678), [#2736](https://github.com/fishtown-analytics/dbt/pull/2736)) - Added schema and dbt versions to JSON artifacts ([#2670](https://github.com/fishtown-analytics/dbt/issues/2670), [#2767](https://github.com/fishtown-analytics/dbt/pull/2767)) - Added ability to snapshot hard-deleted records (opt-in with `invalidate_hard_deletes` config option). ([#249](https://github.com/fishtown-analytics/dbt/issues/249), [#2749](https://github.com/fishtown-analytics/dbt/pull/2749)) +- Improved error messages for YAML selectors ([#2700](https://github.com/fishtown-analytics/dbt/issues/2700), [#2781](https://github.com/fishtown-analytics/dbt/pull/2781)) Contributors: - [@joelluijmes](https://github.com/joelluijmes) ([#2749](https://github.com/fishtown-analytics/dbt/pull/2749)) diff --git a/core/dbt/config/selectors.py b/core/dbt/config/selectors.py index 74c85aae344..ad2604855d4 100644 --- a/core/dbt/config/selectors.py +++ b/core/dbt/config/selectors.py @@ -1,5 +1,6 @@ from pathlib import Path from typing import Dict, Any +import yaml from hologram import ValidationError @@ -33,7 +34,17 @@ def from_dict(cls, data: Dict[str, Any]) -> 'SelectorConfig': try: selector_file = SelectorFile.from_dict(data) selectors = parse_from_selectors_definition(selector_file) - except (ValidationError, RuntimeException) as exc: + except ValidationError as exc: + yaml_sel_cfg = yaml.dump(exc.instance) + raise DbtSelectorsError( + f"Could not parse selector file data: \n{yaml_sel_cfg}\n" + f"Valid root-level selector definitions: " + f"union, intersection, string, dictionary. No lists. " + f"\nhttps://docs.getdbt.com/reference/node-selection/" + f"yaml-selectors", + result_type='invalid_selector' + ) from exc + except RuntimeException as exc: raise DbtSelectorsError( f'Could not read selector file data: {exc}', result_type='invalid_selector', diff --git a/core/dbt/graph/cli.py b/core/dbt/graph/cli.py index b743022472c..64f0259f41b 100644 --- a/core/dbt/graph/cli.py +++ b/core/dbt/graph/cli.py @@ -1,5 +1,6 @@ # special support for CLI argument parsing. import itertools +import yaml from typing import ( Dict, List, Optional, Tuple, Any, Union @@ -116,8 +117,7 @@ def _get_list_dicts( values = dct[key] if not isinstance(values, list): raise ValidationException( - f'Invalid value type {type(values)} in key "{key}" ' - f'(value "{values}")' + f'Invalid value for key "{key}". Expected a list.' ) for value in values: if isinstance(value, dict): @@ -165,9 +165,10 @@ def _parse_include_exclude_subdefs( if isinstance(definition, dict) and 'exclude' in definition: # do not allow multiple exclude: defs at the same level if diff_arg is not None: + yaml_sel_cfg = yaml.dump(definition) raise ValidationException( - f'Got multiple exclusion definitions in definition list ' - f'{definitions}' + f"You cannot provide multiple exclude arguments to the " + f"same selector set operator:\n{yaml_sel_cfg}" ) diff_arg = _parse_exclusions(definition) else: @@ -198,6 +199,7 @@ def parse_intersection_definition( intersection_def_parts = _get_list_dicts(definition, 'intersection') include, exclude = _parse_include_exclude_subdefs(intersection_def_parts) intersection = SelectionIntersection(components=include) + if exclude is None: intersection.raw = definition return intersection @@ -210,7 +212,6 @@ def parse_intersection_definition( def parse_dict_definition(definition: Dict[str, Any]) -> SelectionSpec: diff_arg: Optional[SelectionSpec] = None - if len(definition) == 1: key = list(definition)[0] value = definition[key] @@ -230,7 +231,7 @@ def parse_dict_definition(definition: Dict[str, Any]) -> SelectionSpec: dct = {k: v for k, v in dct.items() if k != 'exclude'} else: raise ValidationException( - f'Expected exactly 1 key in the selection definition or "method" ' + f'Expected either 1 key or else "method" ' f'and "value" keys, but got {list(definition)}' ) @@ -242,7 +243,18 @@ def parse_dict_definition(definition: Dict[str, Any]) -> SelectionSpec: return SelectionDifference(components=[base, diff_arg]) -def parse_from_definition(definition: RawDefinition) -> SelectionSpec: +def parse_from_definition( + definition: RawDefinition, rootlevel=False +) -> SelectionSpec: + + if (isinstance(definition, dict) and + ('union' in definition or 'intersection' in definition) and + rootlevel and len(definition) > 1): + keys = ",".join(definition.keys()) + raise ValidationException( + f"Only a single 'union' or 'intersection' key is allowed " + f"in a root level selector definition; found {keys}." + ) if isinstance(definition, str): return SelectionCriteria.from_single_spec(definition) elif 'union' in definition: @@ -253,8 +265,8 @@ def parse_from_definition(definition: RawDefinition) -> SelectionSpec: return parse_dict_definition(definition) else: raise ValidationException( - f'Expected to find str or dict, instead found ' - f'{type(definition)}: {definition}' + f'Expected to find union, intersection, str or dict, instead ' + f'found {type(definition)}: {definition}' ) @@ -264,5 +276,6 @@ def parse_from_selectors_definition( result: Dict[str, SelectionSpec] = {} selector: SelectorDefinition for selector in source.selectors: - result[selector.name] = parse_from_definition(selector.definition) + result[selector.name] = parse_from_definition(selector.definition, + rootlevel=True) return result diff --git a/core/dbt/graph/selector_spec.py b/core/dbt/graph/selector_spec.py index f8547c4d2c7..c5d13905bfd 100644 --- a/core/dbt/graph/selector_spec.py +++ b/core/dbt/graph/selector_spec.py @@ -93,7 +93,9 @@ def parse_method( try: method_name = MethodName(method_parts[0]) except ValueError as exc: - raise InvalidSelectorException(method_parts[0]) from exc + raise InvalidSelectorException( + f"'{method_parts[0]}' is not a valid method name" + ) from exc method_arguments: List[str] = method_parts[1:] diff --git a/test/unit/test_selector_errors.py b/test/unit/test_selector_errors.py new file mode 100644 index 00000000000..a5019c1211a --- /dev/null +++ b/test/unit/test_selector_errors.py @@ -0,0 +1,179 @@ +import dbt.exceptions +import textwrap +import yaml +import unittest +from dbt.config.selectors import ( + selector_config_from_data +) + +from dbt.config.selectors import SelectorConfig + + +def get_selector_dict(txt: str) -> dict: + txt = textwrap.dedent(txt) + dct = yaml.safe_load(txt) + return dct + + +class SelectorUnitTest(unittest.TestCase): + + def test_parse_multiple_excludes(self): + dct = get_selector_dict('''\ + selectors: + - name: mult_excl + definition: + union: + - method: tag + value: nightly + - exclude: + - method: tag + value: hourly + - exclude: + - method: tag + value: daily + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + 'cannot provide multiple exclude arguments' + ): + selector_config_from_data(dct) + + def test_parse_set_op_plus(self): + dct = get_selector_dict('''\ + selectors: + - name: union_plus + definition: + - union: + - method: tag + value: nightly + - exclude: + - method: tag + value: hourly + - method: tag + value: foo + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + 'Valid root-level selector definitions' + ): + selector_config_from_data(dct) + + def test_parse_multiple_methods(self): + dct = get_selector_dict('''\ + selectors: + - name: mult_methods + definition: + - tag:hourly + - tag:nightly + - fqn:start + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + 'Valid root-level selector definitions' + ): + selector_config_from_data(dct) + + def test_parse_set_with_method(self): + dct = get_selector_dict('''\ + selectors: + - name: mixed_syntaxes + definition: + key: value + method: tag + value: foo + union: + - method: tag + value: m1234 + - exclude: + - method: tag + value: m5678 + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + "Only a single 'union' or 'intersection' key is allowed" + ): + selector_config_from_data(dct) + + def test_complex_sector(self): + dct = get_selector_dict('''\ + selectors: + - name: nightly_diet_snowplow + definition: + union: + - intersection: + - method: source + value: snowplow + childrens_parents: true + - method: tag + value: nightly + - method: path + value: models/export + - exclude: + - intersection: + - method: package + value: snowplow + - method: config.materialized + value: incremental + - method: fqn + value: export_performance_timing + ''') + selectors = selector_config_from_data(dct) + assert(isinstance(selectors, SelectorConfig)) + + def test_exclude_not_list(self): + dct = get_selector_dict('''\ + selectors: + - name: summa_exclude + definition: + union: + - method: tag + value: nightly + - exclude: + method: tag + value: daily + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + "Expected a list" + ): + selector_config_from_data(dct) + + def test_invalid_key(self): + dct = get_selector_dict('''\ + selectors: + - name: summa_nothing + definition: + method: tag + key: nightly + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + "Expected either 1 key" + ): + selector_config_from_data(dct) + + def test_invalid_single_def(self): + dct = get_selector_dict('''\ + selectors: + - name: summa_nothing + definition: + fubar: tag + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + "not a valid method name" + ): + selector_config_from_data(dct) + + def test_method_no_value(self): + dct = get_selector_dict('''\ + selectors: + - name: summa_nothing + definition: + method: tag + ''') + with self.assertRaisesRegex( + dbt.exceptions.DbtSelectorsError, + "not a valid method name" + ): + selector_config_from_data(dct)