Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds 'buildable' selection mode #6366

Merged
merged 1 commit into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20230102-091335.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Adds buildable selection mode
time: 2023-01-02T09:13:35.663627-05:00
custom:
Author: agpapa
Issue: "6365"
9 changes: 7 additions & 2 deletions core/dbt/graph/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ def parse_union(
components=intersection_components,
expect_exists=expect_exists,
raw=raw_spec,
indirect_selection=IndirectSelection(flags.INDIRECT_SELECTION),
)
)
return SelectionUnion(
components=union_components,
expect_exists=False,
raw=components,
indirect_selection=IndirectSelection(flags.INDIRECT_SELECTION),
)


Expand Down Expand Up @@ -78,9 +80,12 @@ def parse_difference(
include, DEFAULT_INCLUDES, indirect_selection=IndirectSelection(flags.INDIRECT_SELECTION)
)
excluded = parse_union_from_default(
exclude, DEFAULT_EXCLUDES, indirect_selection=IndirectSelection.Eager
exclude, DEFAULT_EXCLUDES, indirect_selection=IndirectSelection(flags.INDIRECT_SELECTION)
)
return SelectionDifference(
components=[included, excluded],
indirect_selection=IndirectSelection(flags.INDIRECT_SELECTION),
)
return SelectionDifference(components=[included, excluded])


RawDefinition = Union[str, Dict[str, Any]]
Expand Down
45 changes: 36 additions & 9 deletions core/dbt/graph/selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ def select_nodes_recursively(self, spec: SelectionSpec) -> Tuple[Set[UniqueId],
initial_direct = spec.combined(direct_sets)
indirect_nodes = spec.combined(indirect_sets)

direct_nodes = self.incorporate_indirect_nodes(initial_direct, indirect_nodes)
direct_nodes = self.incorporate_indirect_nodes(
initial_direct, indirect_nodes, spec.indirect_selection
)

if spec.expect_exists and len(direct_nodes) == 0:
warn_or_error(NoNodesForSelectionCriteria(spec_raw=str(spec.raw)))
Expand Down Expand Up @@ -197,19 +199,30 @@ def expand_selection(
) -> 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`.
# Expansion has two modes, EAGER and CAUTIOUS.
# Expansion has three modes, EAGER, CAUTIOUS and BUILDABLE.
#
# EAGER mode: If ANY parent is selected, select the test.
#
# CAUTIOUS mode:
# - If ALL parents are selected, select the test.
# - If ANY parent is missing, return it separately. We'll keep it around
# for later and see if its other parents show up.
#
# BUILDABLE mode:
# - If ALL parents are selected, or the parents of the test are themselves parents of the selected, select the test.
# - 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 `indirect_selection: true` in a yaml selector

direct_nodes = set(selected)
indirect_nodes = set()
selected_and_parents = set()
if indirect_selection == IndirectSelection.Buildable:
selected_and_parents = selected.union(self.graph.select_parents(selected)).union(
self.manifest.sources
)

for unique_id in self.graph.select_successors(selected):
if unique_id in self.manifest.nodes:
Expand All @@ -220,14 +233,20 @@ def expand_selection(
node.depends_on_nodes
) <= set(selected):
direct_nodes.add(unique_id)
# if not:
elif indirect_selection == IndirectSelection.Buildable and set(
node.depends_on_nodes
) <= set(selected_and_parents):
direct_nodes.add(unique_id)
else:
indirect_nodes.add(unique_id)

return direct_nodes, indirect_nodes

def incorporate_indirect_nodes(
self, direct_nodes: Set[UniqueId], indirect_nodes: Set[UniqueId] = set()
self,
direct_nodes: Set[UniqueId],
indirect_nodes: Set[UniqueId] = set(),
indirect_selection: IndirectSelection = IndirectSelection.Eager,
) -> Set[UniqueId]:
# Check tests previously selected indirectly to see if ALL their
# parents are now present.
Expand All @@ -238,11 +257,19 @@ def incorporate_indirect_nodes(

selected = set(direct_nodes)

for unique_id in indirect_nodes:
if unique_id in self.manifest.nodes:
node = self.manifest.nodes[unique_id]
if set(node.depends_on_nodes) <= set(selected):
selected.add(unique_id)
if indirect_selection == IndirectSelection.Cautious:
for unique_id in indirect_nodes:
if unique_id in self.manifest.nodes:
node = self.manifest.nodes[unique_id]
if set(node.depends_on_nodes) <= set(selected):
selected.add(unique_id)
elif indirect_selection == IndirectSelection.Buildable:
selected_and_parents = selected.union(self.graph.select_parents(selected))
for unique_id in indirect_nodes:
if unique_id in self.manifest.nodes:
node = self.manifest.nodes[unique_id]
if set(node.depends_on_nodes) <= set(selected_and_parents):
selected.add(unique_id)

return selected

Expand Down
3 changes: 3 additions & 0 deletions core/dbt/graph/selector_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
class IndirectSelection(StrEnum):
Eager = "eager"
Cautious = "cautious"
Buildable = "buildable"


def _probably_path(value: str):
Expand Down Expand Up @@ -173,12 +174,14 @@ class BaseSelectionGroup(dbtClassMixin, Iterable[SelectionSpec], metaclass=ABCMe
def __init__(
self,
components: Iterable[SelectionSpec],
indirect_selection: IndirectSelection = IndirectSelection.Eager,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case this is confusing - in selector.py on line 138 we need to get the indirect selection method from a composite spec (a union, difference, or intersection). This change here, and the changes in cli.py, add the indirect selection method to these composite specs through the BaseSelectionGroup so we can get them at that line in selector.py. Previously the indirect selection method was only on the SelectionCriteria object.

It's possible I misunderstood how something works here, but I couldn't find an easier way to get the indirect selection method at that point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this note, really helpful!

expect_exists: bool = False,
raw: Any = None,
):
self.components: List[SelectionSpec] = list(components)
self.expect_exists = expect_exists
self.raw = raw
self.indirect_selection = indirect_selection

def __iter__(self) -> Iterator[SelectionSpec]:
for component in self.components:
Expand Down
6 changes: 3 additions & 3 deletions core/dbt/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ def _build_build_subparser(subparsers, base_subparser):
)
sub.add_argument(
"--indirect-selection",
choices=["eager", "cautious"],
choices=["eager", "cautious", "buildable"],
default="eager",
dest="indirect_selection",
help="""
Expand Down Expand Up @@ -763,7 +763,7 @@ def _build_test_subparser(subparsers, base_subparser):
)
sub.add_argument(
"--indirect-selection",
choices=["eager", "cautious"],
choices=["eager", "cautious", "buildable"],
default="eager",
dest="indirect_selection",
help="""
Expand Down Expand Up @@ -869,7 +869,7 @@ def _build_list_subparser(subparsers, base_subparser):
)
sub.add_argument(
"--indirect-selection",
choices=["eager", "cautious"],
choices=["eager", "cautious", "buildable"],
default="eager",
dest="indirect_selection",
help="""
Expand Down
3 changes: 3 additions & 0 deletions test/unit/test_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ def test__flags(self):
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 = 'buildable'
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Buildable)
self.user_config.indirect_selection = None
flags.set_from_args(self.args, self.user_config)
self.assertEqual(flags.INDIRECT_SELECTION, IndirectSelection.Eager)
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_selection/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
tags = ['a_or_b']
) }}

select 1 as fun
select * FROM {{ref('model_b')}}
"""


Expand Down
93 changes: 85 additions & 8 deletions tests/functional/test_selection/test_selection_expansion.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,18 @@ def test_model_a_exclude_specific_test_cautious(
self.list_tests_and_assert(select, exclude, expected, indirect_selection)
self.run_tests_and_assert(select, exclude, expected, indirect_selection)

def test_model_a_exclude_specific_test_buildable(
self,
project,
):
select = "model_a"
exclude = "unique_model_a_fun"
expected = ["just_a", "cf_a_b", "cf_a_src", "relationships_model_a_fun__fun__ref_model_b_", "relationships_model_a_fun__fun__source_my_src_my_tbl_"]
indirect_selection = "buildable"

self.list_tests_and_assert(select, exclude, expected, indirect_selection)
self.run_tests_and_assert(select, exclude, expected, indirect_selection)

def test_only_generic(
self,
project,
Expand Down Expand Up @@ -374,6 +386,40 @@ def test_model_a_indirect_selection_eager(
self.list_tests_and_assert(select, exclude, expected, indirect_selection)
self.run_tests_and_assert(select, exclude, expected, indirect_selection)

def test_model_a_indirect_selection_cautious(
self,
project,
):
select = "model_a"
exclude = None
expected = [
"just_a",
"unique_model_a_fun",
]
indirect_selection = "cautious"

self.list_tests_and_assert(select, exclude, expected, indirect_selection)
self.run_tests_and_assert(select, exclude, expected, indirect_selection)

def test_model_a_indirect_selection_buildable(
self,
project,
):
select = "model_a"
exclude = None
expected = [
"cf_a_b",
"cf_a_src",
"just_a",
"relationships_model_a_fun__fun__ref_model_b_",
"relationships_model_a_fun__fun__source_my_src_my_tbl_",
"unique_model_a_fun",
]
indirect_selection = "buildable"

self.list_tests_and_assert(select, exclude, expected, indirect_selection)
self.run_tests_and_assert(select, exclude, expected, indirect_selection)

def test_model_a_indirect_selection_exclude_unique_tests(
self,
project,
Expand Down Expand Up @@ -402,16 +448,21 @@ def selectors(self):
definition:
method: fqn
value: model_a
- name: model_a_no_indirect_selection
- name: model_a_cautious_indirect_selection
definition:
method: fqn
value: model_a
indirect_selection: "cautious"
- name: model_a_yes_indirect_selection
- name: model_a_eager_indirect_selection
definition:
method: fqn
value: model_a
indirect_selection: "eager"
- name: model_a_buildable_indirect_selection
definition:
method: fqn
value: model_a
indirect_selection: "buildable"
"""

def test_selector_model_a_unset_indirect_selection(
Expand Down Expand Up @@ -440,7 +491,7 @@ def test_selector_model_a_unset_indirect_selection(
selector_name="model_a_unset_indirect_selection",
)

def test_selector_model_a_no_indirect_selection(
def test_selector_model_a_cautious_indirect_selection(
self,
project,
):
Expand All @@ -450,16 +501,42 @@ def test_selector_model_a_no_indirect_selection(
include=None,
exclude=None,
expected_tests=expected,
selector_name="model_a_no_indirect_selection",
selector_name="model_a_cautious_indirect_selection",
)
self.run_tests_and_assert(
include=None,
exclude=None,
expected_tests=expected,
selector_name="model_a_cautious_indirect_selection",
)

def test_selector_model_a_eager_indirect_selection(
self,
project,
):
expected = [
"cf_a_b",
"cf_a_src",
"just_a",
"relationships_model_a_fun__fun__ref_model_b_",
"relationships_model_a_fun__fun__source_my_src_my_tbl_",
"unique_model_a_fun",
]

self.list_tests_and_assert(
include=None,
exclude=None,
expected_tests=expected,
selector_name="model_a_eager_indirect_selection",
)
self.run_tests_and_assert(
include=None,
exclude=None,
expected_tests=expected,
selector_name="model_a_no_indirect_selection",
selector_name="model_a_eager_indirect_selection",
)

def test_selector_model_a_yes_indirect_selection(
def test_selector_model_a_buildable_indirect_selection(
self,
project,
):
Expand All @@ -476,11 +553,11 @@ def test_selector_model_a_yes_indirect_selection(
include=None,
exclude=None,
expected_tests=expected,
selector_name="model_a_yes_indirect_selection",
selector_name="model_a_buildable_indirect_selection",
)
self.run_tests_and_assert(
include=None,
exclude=None,
expected_tests=expected,
selector_name="model_a_yes_indirect_selection",
selector_name="model_a_buildable_indirect_selection",
)