Skip to content

Commit

Permalink
Feature/3997 profiles test selection flag (#4270)
Browse files Browse the repository at this point in the history
* Address 3997. Test selection flag can be in profile.yml.

* Per Jerco's 4104 PR unresolved comments, unify i.s. predicate and add env var.

* Couple of flake8 touchups.

* Classier error handling using enum semantics.

* Cherry-pick in part of Gerda's commit to hopefully avoid a future merge conflict.

* Add 3997 to changelog.

Co-authored-by: Mila Page <versusfacit@users.noreply.github.com>
  • Loading branch information
VersusFacit and VersusFacit authored Nov 15, 2021
1 parent 4a8a680 commit 5d1b104
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 30 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## dbt-core 1.0.0rc2 (TBD)

### Under the hood
Add --indirect-selection parameter to profiles.yml and builtin DBT_ env vars; stringified parameter to enable multi-modal use ([#3997](https://github.com/dbt-labs/dbt-core/issues/3997), [PR #4270](https://github.com/dbt-labs/dbt-core/pull/4270))


## dbt-core 1.0.0rc1 (November 10, 2021)

### Breaking changes
Expand Down
1 change: 1 addition & 0 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ class UserConfig(ExtensibleDbtClassMixin, Replaceable, UserConfigContract):
fail_fast: Optional[bool] = None
use_experimental_parser: Optional[bool] = None
static_parser: Optional[bool] = None
indirect_selection: Optional[str] = None


@dataclass
Expand Down
12 changes: 7 additions & 5 deletions core/dbt/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
STRICT_MODE = False # Only here for backwards compatibility
FULL_REFRESH = False # subcommand
STORE_FAILURES = False # subcommand
EAGER_INDIRECT_SELECTION = True # subcommand

# Global CLI commands
USE_EXPERIMENTAL_PARSER = None
Expand All @@ -33,6 +32,7 @@
SEND_ANONYMOUS_USAGE_STATS = None
PRINTER_WIDTH = 80
WHICH = None
INDIRECT_SELECTION = None

# Global CLI defaults. These flags are set from three places:
# CLI args, environment variables, and user_config (profiles.yml).
Expand All @@ -50,7 +50,8 @@
"VERSION_CHECK": True,
"FAIL_FAST": False,
"SEND_ANONYMOUS_USAGE_STATS": True,
"PRINTER_WIDTH": 80
"PRINTER_WIDTH": 80,
"INDIRECT_SELECTION": 'eager'
}


Expand Down Expand Up @@ -96,15 +97,14 @@ def _get_context():
def set_from_args(args, user_config):
global STRICT_MODE, FULL_REFRESH, WARN_ERROR, \
USE_EXPERIMENTAL_PARSER, STATIC_PARSER, WRITE_JSON, PARTIAL_PARSE, \
USE_COLORS, STORE_FAILURES, PROFILES_DIR, DEBUG, LOG_FORMAT, EAGER_INDIRECT_SELECTION, \
USE_COLORS, STORE_FAILURES, PROFILES_DIR, DEBUG, LOG_FORMAT, INDIRECT_SELECTION, \
VERSION_CHECK, FAIL_FAST, SEND_ANONYMOUS_USAGE_STATS, PRINTER_WIDTH, \
WHICH

STRICT_MODE = False # backwards compatibility
# cli args without user_config or env var option
FULL_REFRESH = getattr(args, 'full_refresh', FULL_REFRESH)
STORE_FAILURES = getattr(args, 'store_failures', STORE_FAILURES)
EAGER_INDIRECT_SELECTION = getattr(args, 'indirect_selection', 'eager') != 'cautious'
WHICH = getattr(args, 'which', WHICH)

# global cli flags with env var and user_config alternatives
Expand All @@ -121,6 +121,7 @@ def set_from_args(args, user_config):
FAIL_FAST = get_flag_value('FAIL_FAST', args, user_config)
SEND_ANONYMOUS_USAGE_STATS = get_flag_value('SEND_ANONYMOUS_USAGE_STATS', args, user_config)
PRINTER_WIDTH = get_flag_value('PRINTER_WIDTH', args, user_config)
INDIRECT_SELECTION = get_flag_value('INDIRECT_SELECTION', args, user_config)


def get_flag_value(flag, args, user_config):
Expand All @@ -133,7 +134,7 @@ def get_flag_value(flag, args, user_config):
if env_value is not None and env_value != '':
env_value = env_value.lower()
# non Boolean values
if flag in ['LOG_FORMAT', 'PRINTER_WIDTH', 'PROFILES_DIR']:
if flag in ['LOG_FORMAT', 'PRINTER_WIDTH', 'PROFILES_DIR', 'INDIRECT_SELECTION']:
flag_value = env_value
else:
flag_value = env_set_bool(env_value)
Expand Down Expand Up @@ -164,4 +165,5 @@ def get_flag_dict():
"fail_fast": FAIL_FAST,
"send_anonymous_usage_stats": SEND_ANONYMOUS_USAGE_STATS,
"printer_width": PRINTER_WIDTH,
"indirect_selection": INDIRECT_SELECTION
}
27 changes: 20 additions & 7 deletions core/dbt/graph/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
SelectionIntersection,
SelectionDifference,
SelectionCriteria,
IndirectSelection
)

INTERSECTION_DELIMITER = ','
Expand All @@ -25,7 +26,8 @@


def parse_union(
components: List[str], expect_exists: bool, eagerly_expand: bool = True
components: List[str], expect_exists: bool,
indirect_selection: IndirectSelection = IndirectSelection.Eager
) -> SelectionUnion:
# turn ['a b', 'c'] -> ['a', 'b', 'c']
raw_specs = itertools.chain.from_iterable(
Expand All @@ -36,7 +38,7 @@ def parse_union(
# ['a', 'b', 'c,d'] -> union('a', 'b', intersection('c', 'd'))
for raw_spec in raw_specs:
intersection_components: List[SelectionSpec] = [
SelectionCriteria.from_single_spec(part, eagerly_expand=eagerly_expand)
SelectionCriteria.from_single_spec(part, indirect_selection=indirect_selection)
for part in raw_spec.split(INTERSECTION_DELIMITER)
]
union_components.append(SelectionIntersection(
Expand All @@ -52,25 +54,36 @@ def parse_union(


def parse_union_from_default(
raw: Optional[List[str]], default: List[str], eagerly_expand: bool = True
raw: Optional[List[str]], default: List[str],
indirect_selection: IndirectSelection = IndirectSelection.Eager
) -> SelectionUnion:
components: List[str]
expect_exists: bool
if raw is None:
return parse_union(components=default, expect_exists=False, eagerly_expand=eagerly_expand)
return parse_union(
components=default,
expect_exists=False,
indirect_selection=indirect_selection)
else:
return parse_union(components=raw, expect_exists=True, eagerly_expand=eagerly_expand)
return parse_union(
components=raw,
expect_exists=True,
indirect_selection=indirect_selection)


def parse_difference(
include: Optional[List[str]], exclude: Optional[List[str]]
) -> SelectionDifference:

included = parse_union_from_default(
include,
DEFAULT_INCLUDES,
eagerly_expand=flags.EAGER_INDIRECT_SELECTION
indirect_selection=IndirectSelection(flags.INDIRECT_SELECTION)
)
excluded = parse_union_from_default(exclude, DEFAULT_EXCLUDES, eagerly_expand=True)
excluded = parse_union_from_default(
exclude,
DEFAULT_EXCLUDES,
indirect_selection=IndirectSelection.Eager)
return SelectionDifference(components=[included, excluded])


Expand Down
14 changes: 9 additions & 5 deletions core/dbt/graph/selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from .graph import Graph, UniqueId
from .queue import GraphQueue
from .selector_methods import MethodManager
from .selector_spec import SelectionCriteria, SelectionSpec
from .selector_spec import SelectionCriteria, SelectionSpec, IndirectSelection

from dbt.events.functions import fire_event
from dbt.events.types import SelectorReportInvalidSelector
Expand Down Expand Up @@ -96,7 +96,7 @@ def get_nodes_from_criteria(
neighbors = self.collect_specified_neighbors(spec, collected)
direct_nodes, indirect_nodes = self.expand_selection(
selected=(collected | neighbors),
eagerly_expand=spec.eagerly_expand
indirect_selection=spec.indirect_selection
)
return direct_nodes, indirect_nodes

Expand Down Expand Up @@ -204,7 +204,8 @@ def filter_selection(self, selected: Set[UniqueId]) -> Set[UniqueId]:
}

def expand_selection(
self, selected: Set[UniqueId], eagerly_expand: bool = True
self, selected: Set[UniqueId],
indirect_selection: IndirectSelection = IndirectSelection.Eager
) -> Tuple[Set[UniqueId], Set[UniqueId]]:
# Test selection by default expands to include an implicitly/indirectly selected tests.
# `dbt test -m model_a` also includes tests that directly depend on `model_a`.
Expand All @@ -217,7 +218,7 @@ def expand_selection(
# - If ANY parent is missing, return it separately. We'll keep it around
# for later and see if its other parents show up.
# Users can opt out of inclusive EAGER mode by passing --indirect-selection cautious
# CLI argument or by specifying `eagerly_expand: true` in a yaml selector
# CLI argument or by specifying `indirect_selection: true` in a yaml selector

direct_nodes = set(selected)
indirect_nodes = set()
Expand All @@ -227,7 +228,10 @@ def expand_selection(
node = self.manifest.nodes[unique_id]
if can_select_indirectly(node):
# should we add it in directly?
if eagerly_expand or set(node.depends_on.nodes) <= set(selected):
if (
indirect_selection == IndirectSelection.Eager or
set(node.depends_on.nodes) <= set(selected)
):
direct_nodes.add(unique_id)
# if not:
else:
Expand Down
31 changes: 18 additions & 13 deletions core/dbt/graph/selector_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import re
from abc import ABCMeta, abstractmethod
from dataclasses import dataclass
from dbt.dataclass_schema import StrEnum

from typing import (
Set, Iterator, List, Optional, Dict, Union, Any, Iterable, Tuple
Expand All @@ -22,6 +23,11 @@
SELECTOR_METHOD_SEPARATOR = '.'


class IndirectSelection(StrEnum):
Eager = 'eager'
Cautious = 'cautious'


def _probably_path(value: str):
"""Decide if value is probably a path. Windows has two path separators, so
we should check both sep ('\\') and altsep ('/') there.
Expand Down Expand Up @@ -66,7 +72,7 @@ class SelectionCriteria:
parents_depth: Optional[int]
children: bool
children_depth: Optional[int]
eagerly_expand: bool = True
indirect_selection: IndirectSelection = IndirectSelection.Eager

def __post_init__(self):
if self.children and self.childrens_parents:
Expand Down Expand Up @@ -104,7 +110,8 @@ def parse_method(

@classmethod
def selection_criteria_from_dict(
cls, raw: Any, dct: Dict[str, Any], eagerly_expand: bool = True
cls, raw: Any, dct: Dict[str, Any],
indirect_selection: IndirectSelection = IndirectSelection.Eager
) -> 'SelectionCriteria':
if 'value' not in dct:
raise RuntimeException(
Expand All @@ -116,14 +123,9 @@ def selection_criteria_from_dict(
children_depth = _match_to_int(dct, 'children_depth')

# If defined field in selector, override CLI flag
indirect_selection = dct.get('indirect_selection', None)
if indirect_selection:
if indirect_selection in ['eager', 'cautious']:
eagerly_expand = indirect_selection != 'cautious'
else:
raise RuntimeException(
f'indirect_selection value "{indirect_selection}" is not valid!'
)
indirect_selection = IndirectSelection(
dct.get('indirect_selection', None) or indirect_selection
)

return cls(
raw=raw,
Expand All @@ -135,7 +137,7 @@ def selection_criteria_from_dict(
parents_depth=parents_depth,
children=bool(dct.get('children')),
children_depth=children_depth,
eagerly_expand=eagerly_expand
indirect_selection=indirect_selection
)

@classmethod
Expand All @@ -159,7 +161,10 @@ def dict_from_single_spec(cls, raw: str):
return dct

@classmethod
def from_single_spec(cls, raw: str, eagerly_expand: bool = True) -> 'SelectionCriteria':
def from_single_spec(
cls, raw: str,
indirect_selection: IndirectSelection = IndirectSelection.Eager
) -> 'SelectionCriteria':
result = RAW_SELECTOR_PATTERN.match(raw)
if result is None:
# bad spec!
Expand All @@ -168,7 +173,7 @@ def from_single_spec(cls, raw: str, eagerly_expand: bool = True) -> 'SelectionCr
return cls.selection_criteria_from_dict(
raw,
result.groupdict(),
eagerly_expand=eagerly_expand
indirect_selection=indirect_selection
)


Expand Down
3 changes: 3 additions & 0 deletions core/dbt/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ def _build_build_subparser(subparsers, base_subparser):
sub.add_argument(
'--indirect-selection',
choices=['eager', 'cautious'],
default='eager',
dest='indirect_selection',
help='''
Select all tests that are adjacent to selected resources,
Expand Down Expand Up @@ -738,6 +739,7 @@ def _build_test_subparser(subparsers, base_subparser):
sub.add_argument(
'--indirect-selection',
choices=['eager', 'cautious'],
default='eager',
dest='indirect_selection',
help='''
Select all tests that are adjacent to selected resources,
Expand Down Expand Up @@ -842,6 +844,7 @@ def _build_list_subparser(subparsers, base_subparser):
sub.add_argument(
'--indirect-selection',
choices=['eager', 'cautious'],
default='eager',
dest='indirect_selection',
help='''
Select all tests that are adjacent to selected resources,
Expand Down
22 changes: 22 additions & 0 deletions test/unit/test_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from dbt.contracts.project import UserConfig
from dbt.config.profile import DEFAULT_PROFILES_DIR

from core.dbt.graph.selector_spec import IndirectSelection

class TestFlags(TestCase):

Expand Down Expand Up @@ -195,3 +196,24 @@ def test__flags(self):
os.environ.pop('DBT_PRINTER_WIDTH')
delattr(self.args, 'printer_width')
self.user_config.printer_width = None

# indirect_selection
self.user_config.indirect_selection = 'eager'
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Eager)
self.user_config.indirect_selection = 'cautious'
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Cautious)
self.user_config.indirect_selection = None
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Eager)
os.environ['DBT_INDIRECT_SELECTION'] = 'cautious'
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Cautious)
setattr(self.args, 'indirect_selection', 'cautious')
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Cautious)
# cleanup
os.environ.pop('DBT_INDIRECT_SELECTION')
delattr(self.args, 'indirect_selection')
self.user_config.indirect_selection = None

0 comments on commit 5d1b104

Please sign in to comment.