From 00e279f4249e66433a630c800de23e38173f475d Mon Sep 17 00:00:00 2001 From: Peter Allen Webb Date: Fri, 20 Sep 2024 11:05:28 -0400 Subject: [PATCH 01/10] Allow snapshots to be defined in YAML. --- .../unreleased/Features-20240920-110447.yaml | 6 +++ core/dbt/parser/schemas.py | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 .changes/unreleased/Features-20240920-110447.yaml diff --git a/.changes/unreleased/Features-20240920-110447.yaml b/.changes/unreleased/Features-20240920-110447.yaml new file mode 100644 index 00000000000..ed6d9cb09de --- /dev/null +++ b/.changes/unreleased/Features-20240920-110447.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Allow snapshots to be defined in YAML. +time: 2024-09-20T11:04:47.703117-04:00 +custom: + Author: peterallenwebb + Issue: "10246" diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 3a06756e355..e0623074d7d 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -205,6 +205,7 @@ def parse_file(self, block: FileBlock, dct: Optional[Dict] = None) -> None: # PatchParser.parse() if "snapshots" in dct: + self._add_yaml_snapshot_nodes_to_manifest(dct["snapshots"], block) snapshot_parse_result = TestablePatchParser(self, yaml_block, "snapshots").parse() for test_block in snapshot_parse_result.test_blocks: self.generic_test_parser.parse_tests(test_block) @@ -265,6 +266,54 @@ def parse_file(self, block: FileBlock, dct: Optional[Dict] = None) -> None: saved_query_parser = SavedQueryParser(self, yaml_block) saved_query_parser.parse() + def _add_yaml_snapshot_nodes_to_manifest( + self, snapshots: List[Dict[str, Any]], block: FileBlock + ) -> None: + """We support the creation of simple snapshots in yaml, without an + accompanying SQL definition. For such snapshots, the user must supply + a 'relation' property to indicate the target of the snapshot. This + function looks for such snapshots and adds a node to manifest for each + one we find, since they were not added during SQL parsing.""" + + rebuild_refs = False + for snapshot in snapshots: + if "relation" in snapshot: + from dbt.parser import SnapshotParser + + if "name" not in snapshot: + raise ParsingError("A snapshot must define the 'name' property. ") + + # Reuse the logic of SnapshotParser as far as possible to create + # a new node we can add to the manifest. + parser = SnapshotParser(self.project, self.manifest, self.root_project) + fqn = [self.project.project_name, "snapshots", snapshot["name"]] + snapshot_node = parser._create_parsetime_node( + block, + self.get_compiled_path(block), + parser.initial_config(fqn), + fqn, + snapshot["name"], + ) + + # Parse the expected ref() or source() expression given by + # 'relation' so that we know what we are snapshotting. + source_or_ref = statically_parse_ref_or_source(snapshot["relation"]) + if isinstance(source_or_ref, RefArgs): + snapshot_node.refs.append(source_or_ref) + else: + snapshot_node.sources.append(source_or_ref) + + # Implement the snapshot SQL as a simple select * + snapshot_node.raw_code = "select * from {{ " + snapshot["relation"] + " }}" + + # Add our new node to the manifest, and note that ref lookup collections + # will need to be rebuilt. + self.manifest.add_node_nofile(snapshot_node) + rebuild_refs = True + + if rebuild_refs: + self.manifest.rebuild_ref_lookup() + Parsed = TypeVar("Parsed", UnpatchedSourceDefinition, ParsedNodePatch, ParsedMacroPatch) NodeTarget = TypeVar("NodeTarget", UnparsedNodeUpdate, UnparsedAnalysisUpdate, UnparsedModelUpdate) From 59682a87f9664840b7262243dd0c89a07b11002d Mon Sep 17 00:00:00 2001 From: Mike Alfare <13974384+mikealfare@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:09:23 -0400 Subject: [PATCH 02/10] Add Snowplow tracking for behavior flags (#10721) * add behavior deprecation snowplow callback * update tests for new callback * update test input with the new required field --- .../Under the Hood-20240911-162730.yaml | 6 ++ core/dbt/events/logging.py | 3 + core/dbt/tracking.py | 18 ++++- tests/unit/events/test_logging.py | 2 +- tests/unit/test_behavior_flags.py | 65 +++++++++++++++++++ 5 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 .changes/unreleased/Under the Hood-20240911-162730.yaml create mode 100644 tests/unit/test_behavior_flags.py diff --git a/.changes/unreleased/Under the Hood-20240911-162730.yaml b/.changes/unreleased/Under the Hood-20240911-162730.yaml new file mode 100644 index 00000000000..0d35aeb5262 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20240911-162730.yaml @@ -0,0 +1,6 @@ +kind: Under the Hood +body: Add Snowplow tracking for behavior flag deprecations +time: 2024-09-11T16:27:30.293832-04:00 +custom: + Author: mikealfare + Issue: "10552" diff --git a/core/dbt/events/logging.py b/core/dbt/events/logging.py index f0bef3ae497..b0db1003e72 100644 --- a/core/dbt/events/logging.py +++ b/core/dbt/events/logging.py @@ -2,8 +2,10 @@ from functools import partial from typing import Callable, List +from dbt.tracking import track_behavior_change_warn from dbt_common.events.base_types import EventLevel, EventMsg from dbt_common.events.event_manager_client import ( + add_callback_to_manager, add_logger_to_manager, cleanup_event_logger, get_event_manager, @@ -68,6 +70,7 @@ def setup_event_logger(flags, callbacks: List[Callable[[EventMsg], None]] = []) make_log_dir_if_missing(flags.LOG_PATH) event_manager = get_event_manager() event_manager.callbacks = callbacks.copy() + add_callback_to_manager(track_behavior_change_warn) if flags.LOG_LEVEL != "none": line_format = _line_format_from_str(flags.LOG_FORMAT, LineFormat.PlainText) diff --git a/core/dbt/tracking.py b/core/dbt/tracking.py index 880243e4d6e..0358b29f020 100644 --- a/core/dbt/tracking.py +++ b/core/dbt/tracking.py @@ -25,7 +25,8 @@ SendingEvent, TrackingInitializeFailure, ) -from dbt_common.events.functions import fire_event, get_invocation_id +from dbt_common.events.base_types import EventMsg +from dbt_common.events.functions import fire_event, get_invocation_id, msg_to_dict from dbt_common.exceptions import NotImplementedError sp_logger.setLevel(100) @@ -36,6 +37,7 @@ ADAPTER_INFO_SPEC = "iglu:com.dbt/adapter_info/jsonschema/1-0-1" DEPRECATION_WARN_SPEC = "iglu:com.dbt/deprecation_warn/jsonschema/1-0-0" +BEHAVIOR_CHANGE_WARN_SPEC = "iglu:com.dbt/behavior_change_warn/jsonschema/1-0-0" EXPERIMENTAL_PARSER = "iglu:com.dbt/experimental_parser/jsonschema/1-0-0" INVOCATION_ENV_SPEC = "iglu:com.dbt/invocation_env/jsonschema/1-0-0" INVOCATION_SPEC = "iglu:com.dbt/invocation/jsonschema/1-0-2" @@ -364,6 +366,20 @@ def track_deprecation_warn(options): ) +def track_behavior_change_warn(msg: EventMsg) -> None: + if msg.info.name != "BehaviorChangeEvent" or active_user is None: + return + + context = [SelfDescribingJson(BEHAVIOR_CHANGE_WARN_SPEC, msg_to_dict(msg))] + track( + active_user, + category="dbt", + action=msg.info.name, + label=get_invocation_id(), + context=context, + ) + + def track_invocation_end(invocation_context, result_type=None): data = {"progress": "end", "result_type": result_type, "result": None} data.update(invocation_context) diff --git a/tests/unit/events/test_logging.py b/tests/unit/events/test_logging.py index 00284ecab78..b2077b64793 100644 --- a/tests/unit/events/test_logging.py +++ b/tests/unit/events/test_logging.py @@ -23,7 +23,7 @@ def test_clears_preexisting_event_manager_state(self) -> None: setup_event_logger(get_flags()) assert len(manager.loggers) == 0 - assert len(manager.callbacks) == 0 + assert len(manager.callbacks) == 1 # snowplow tracker for behavior flags def test_specify_max_bytes( self, diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py new file mode 100644 index 00000000000..6f4746c438c --- /dev/null +++ b/tests/unit/test_behavior_flags.py @@ -0,0 +1,65 @@ +import pytest + +from dbt.tracking import ( + disable_tracking, + initialize_from_flags, + track_behavior_change_warn, +) +from dbt_common.behavior_flags import Behavior +from dbt_common.events.event_manager_client import ( + add_callback_to_manager, + cleanup_event_logger, +) + + +@pytest.fixture +def snowplow_tracker(mocker): + # initialize `active_user` without writing the cookie to disk + initialize_from_flags(True, "") + mocker.patch("dbt.tracking.User.set_cookie").return_value = {"id": 42} + + # add the relevant callback to the event manager + add_callback_to_manager(track_behavior_change_warn) + + # don't make a call, catch the request + snowplow_tracker = mocker.patch("dbt.tracking.tracker.track_struct_event") + + yield snowplow_tracker + + # teardown + cleanup_event_logger() + disable_tracking() + + +def test_false_evaluation_triggers_snowplow_tracking(snowplow_tracker): + behavior = Behavior( + [{"name": "my_flag", "default": False, "description": "This is a false flag."}], {} + ) + if behavior.my_flag: + # trigger a False evaluation + assert False, "This flag should evaluate to false and skip this line" + assert snowplow_tracker.called + + +def test_true_evaluation_does_not_trigger_snowplow_tracking(snowplow_tracker): + behavior = Behavior( + [{"name": "my_flag", "default": True, "description": "This is a true flag."}], {} + ) + if behavior.my_flag: + pass + else: + # trigger a True evaluation + assert False, "This flag should evaluate to false and skip this line" + assert not snowplow_tracker.called + + +def test_false_evaluation_does_not_trigger_snowplow_tracking_when_disabled(snowplow_tracker): + disable_tracking() + + behavior = Behavior( + [{"name": "my_flag", "default": False, "description": "This is a false flag."}], {} + ) + if behavior.my_flag: + # trigger a False evaluation + assert False, "This flag should evaluate to false and skip this line" + assert not snowplow_tracker.called From d18d946328605d2b04d1561950e9fab44c9b7a3e Mon Sep 17 00:00:00 2001 From: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Date: Wed, 18 Sep 2024 14:13:50 -0600 Subject: [PATCH 03/10] Replace `TestSelector` with `ResourceTypeSelector` (#10718) * [tidy first] Replace `TestSelector` with `ResourceTypeSelector` * Changelog entry * Fully preserve current behavior * Revert "Fully preserve current behavior" This reverts commit ceecfec96da00efeaca57922b4a3ed7eec639105. --- .../Under the Hood-20240916-102201.yaml | 6 ++++++ core/dbt/task/build.py | 3 +-- core/dbt/task/list.py | 3 +-- core/dbt/task/test.py | 17 +++-------------- 4 files changed, 11 insertions(+), 18 deletions(-) create mode 100644 .changes/unreleased/Under the Hood-20240916-102201.yaml diff --git a/.changes/unreleased/Under the Hood-20240916-102201.yaml b/.changes/unreleased/Under the Hood-20240916-102201.yaml new file mode 100644 index 00000000000..48485d44f48 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20240916-102201.yaml @@ -0,0 +1,6 @@ +kind: Under the Hood +body: Replace `TestSelector` with `ResourceTypeSelector` +time: 2024-09-16T10:22:01.339462-06:00 +custom: + Author: dbeatty10 + Issue: "10718" diff --git a/core/dbt/task/build.py b/core/dbt/task/build.py index 62d2ed28c7c..832f7f0fe47 100644 --- a/core/dbt/task/build.py +++ b/core/dbt/task/build.py @@ -11,7 +11,6 @@ from dbt.graph import Graph, GraphQueue, ResourceTypeSelector from dbt.node_types import NodeType from dbt.task.base import BaseRunner, resource_types_from_args -from dbt.task.test import TestSelector from dbt_common.events.functions import fire_event from .run import ModelRunner as run_model_runner @@ -198,7 +197,7 @@ def get_node_selector(self, no_unit_tests=False) -> ResourceTypeSelector: resource_types = self.resource_types(no_unit_tests) if resource_types == [NodeType.Test]: - return TestSelector( + return ResourceTypeSelector( graph=self.graph, manifest=self.manifest, previous_state=self.previous_state, diff --git a/core/dbt/task/list.py b/core/dbt/task/list.py index 53fc0d80a13..180cfc71693 100644 --- a/core/dbt/task/list.py +++ b/core/dbt/task/list.py @@ -17,7 +17,6 @@ from dbt.node_types import NodeType from dbt.task.base import resource_types_from_args from dbt.task.runnable import GraphRunnableTask -from dbt.task.test import TestSelector from dbt_common.events.contextvars import task_contextvars from dbt_common.events.functions import fire_event, warn_or_error from dbt_common.events.types import PrintEvent @@ -201,7 +200,7 @@ def get_node_selector(self): if self.manifest is None or self.graph is None: raise DbtInternalError("manifest and graph must be set to get perform node selection") if self.resource_types == [NodeType.Test]: - return TestSelector( + return ResourceTypeSelector( graph=self.graph, manifest=self.manifest, previous_state=self.previous_state, diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index 0b1b56f8b22..d1958d4ef4b 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -375,18 +375,6 @@ def _render_daff_diff(self, daff_diff: daff.TableDiff) -> str: return rendered -class TestSelector(ResourceTypeSelector): - def __init__( - self, graph, manifest, previous_state, resource_types=[NodeType.Test, NodeType.Unit] - ) -> None: - super().__init__( - graph=graph, - manifest=manifest, - previous_state=previous_state, - resource_types=resource_types, - ) - - class TestTask(RunTask): """ Testing: @@ -399,13 +387,14 @@ class TestTask(RunTask): def raise_on_first_error(self) -> bool: return False - def get_node_selector(self) -> TestSelector: + def get_node_selector(self) -> ResourceTypeSelector: if self.manifest is None or self.graph is None: raise DbtInternalError("manifest and graph must be set to get perform node selection") - return TestSelector( + return ResourceTypeSelector( graph=self.graph, manifest=self.manifest, previous_state=self.previous_state, + resource_types=[NodeType.Test, NodeType.Unit], ) def get_runner_type(self, _) -> Optional[Type[BaseRunner]]: From 8a7001c940d2b4faa3779cc11e734cc414c550cd Mon Sep 17 00:00:00 2001 From: Mike Alfare <13974384+mikealfare@users.noreply.github.com> Date: Wed, 18 Sep 2024 16:29:18 -0400 Subject: [PATCH 04/10] snowplow is deprecating track_struct_event; use StructuredEvent instead (#10736) --- core/dbt/tracking.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/core/dbt/tracking.py b/core/dbt/tracking.py index 0358b29f020..7d648d86e03 100644 --- a/core/dbt/tracking.py +++ b/core/dbt/tracking.py @@ -12,6 +12,7 @@ from snowplow_tracker import Emitter, SelfDescribingJson, Subject, Tracker from snowplow_tracker import __version__ as snowplow_version # type: ignore from snowplow_tracker import logger as sp_logger +from snowplow_tracker.events import StructuredEvent from dbt import version as dbt_version from dbt.adapters.exceptions import FailedToConnectError @@ -217,12 +218,12 @@ def get_dbt_env_context(): def track(user, *args, **kwargs): if user.do_not_track: return - else: - fire_event(SendingEvent(kwargs=str(kwargs))) - try: - tracker.track_struct_event(*args, **kwargs) - except Exception: - fire_event(SendEventFailure()) + + fire_event(SendingEvent(kwargs=str(kwargs))) + try: + tracker.track(StructuredEvent(*args, **kwargs)) + except Exception: + fire_event(SendEventFailure()) def track_project_id(options): From 10464c1a22bc7941dc4e6c0576d42bf87ec027be Mon Sep 17 00:00:00 2001 From: Mike Alfare <13974384+mikealfare@users.noreply.github.com> Date: Wed, 18 Sep 2024 18:45:11 -0400 Subject: [PATCH 05/10] fix mock in unit test after removing deprecated snowplow method (#10738) --- tests/unit/test_behavior_flags.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py index 6f4746c438c..d899a83f283 100644 --- a/tests/unit/test_behavior_flags.py +++ b/tests/unit/test_behavior_flags.py @@ -22,7 +22,8 @@ def snowplow_tracker(mocker): add_callback_to_manager(track_behavior_change_warn) # don't make a call, catch the request - snowplow_tracker = mocker.patch("dbt.tracking.tracker.track_struct_event") + # to avoid confusion, this is snowplow_tracker's track, not our wrapper that's also named track + snowplow_tracker = mocker.patch("dbt.tracking.tracker.track") yield snowplow_tracker From a8542725f7dc5c19968449c5b60c74357ee0090e Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Wed, 18 Sep 2024 22:13:50 -0500 Subject: [PATCH 06/10] Split out model vs microbatch execution (#10737) --- core/dbt/task/run.py | 76 +++++++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 22 deletions(-) diff --git a/core/dbt/task/run.py b/core/dbt/task/run.py index d2a7f8d0c0e..e6e380b4063 100644 --- a/core/dbt/task/run.py +++ b/core/dbt/task/run.py @@ -350,6 +350,48 @@ def _materialization_relations(self, result: Any, model) -> List[BaseRelation]: ) raise CompilationError(msg, node=model) + def _execute_model( + self, + hook_ctx: Any, + context_config: Any, + model: ModelNode, + context: Dict[str, Any], + materialization_macro: MacroProtocol, + ) -> RunResult: + try: + result = MacroGenerator( + materialization_macro, context, stack=context["context_macro_stack"] + )() + finally: + self.adapter.post_model_hook(context_config, hook_ctx) + + for relation in self._materialization_relations(result, model): + self.adapter.cache_added(relation.incorporate(dbt_created=True)) + + return self._build_run_model_result(model, context) + + def _execute_microbatch_model( + self, + hook_ctx: Any, + context_config: Any, + model: ModelNode, + manifest: Manifest, + context: Dict[str, Any], + materialization_macro: MacroProtocol, + ) -> RunResult: + batch_results = None + try: + batch_results = self._execute_microbatch_materialization( + model, manifest, context, materialization_macro + ) + finally: + self.adapter.post_model_hook(context_config, hook_ctx) + + if batch_results is not None: + return self._build_run_microbatch_model_result(model, batch_results) + else: + return self._build_run_model_result(model, context) + def execute(self, model, manifest): context = generate_runtime_model_context(model, self.config, manifest) @@ -378,29 +420,19 @@ def execute(self, model, manifest): ) hook_ctx = self.adapter.pre_model_hook(context_config) - batch_results = None - try: - if ( - os.environ.get("DBT_EXPERIMENTAL_MICROBATCH") - and model.config.materialized == "incremental" - and model.config.incremental_strategy == "microbatch" - ): - batch_results = self._execute_microbatch_materialization( - model, manifest, context, materialization_macro - ) - else: - result = MacroGenerator( - materialization_macro, context, stack=context["context_macro_stack"] - )() - for relation in self._materialization_relations(result, model): - self.adapter.cache_added(relation.incorporate(dbt_created=True)) - finally: - self.adapter.post_model_hook(context_config, hook_ctx) - - if batch_results: - return self._build_run_microbatch_model_result(model, batch_results) - return self._build_run_model_result(model, context) + if ( + os.environ.get("DBT_EXPERIMENTAL_MICROBATCH") + and model.config.materialized == "incremental" + and model.config.incremental_strategy == "microbatch" + ): + return self._execute_microbatch_model( + hook_ctx, context_config, model, manifest, context, materialization_macro + ) + else: + return self._execute_model( + hook_ctx, context_config, model, context, materialization_macro + ) def _execute_microbatch_materialization( self, From 574e2d0e71c37ac18caa4f424faf901c333faa61 Mon Sep 17 00:00:00 2001 From: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:52:59 -0600 Subject: [PATCH 07/10] Enable `--resource-type` and `--exclude-resource-type` CLI flags and environment variables for `dbt test` (#10706) * Adding logic to TestSelector to remove unit tests if they are in excluded_resource_types * Adding change log * Respect `--resource-type` and `--exclude-resource-type` CLI flags and associated environment variables * Test CLI flag for excluding unit tests for the `dbt test` command * Satisy isort pre-commit hook * Fix mypy for positional argument "resource_types" in call to "TestSelector" * Replace `TestSelector` with `ResourceTypeSelector` * Add co-author * Update changelog description * Add functional tests for new feature * Compare the actual results, not just the count * Remove test case covered elsewhere * Test for `DBT_EXCLUDE_RESOURCE_TYPES` environment variable for `dbt test` * Update per pre-commit hook * Restore to original form (until we refactor extraneous `ResourceTypeSelector` references later) --------- Co-authored-by: Matthew Cooper --- .../unreleased/Features-20240903-132428.yaml | 6 ++++ core/dbt/cli/main.py | 2 ++ core/dbt/node_types.py | 5 ++++ core/dbt/task/test.py | 28 ++++++++++++++++--- .../unit_testing/test_unit_testing.py | 10 ++++++- .../unit_testing/test_ut_resource_types.py | 20 ++++++++++++- 6 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 .changes/unreleased/Features-20240903-132428.yaml diff --git a/.changes/unreleased/Features-20240903-132428.yaml b/.changes/unreleased/Features-20240903-132428.yaml new file mode 100644 index 00000000000..08df6958990 --- /dev/null +++ b/.changes/unreleased/Features-20240903-132428.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Enable `--resource-type` and `--exclude-resource-type` CLI flags and environment variables for `dbt test` +time: 2024-09-03T13:24:28.592837+01:00 +custom: + Author: TowardOliver dbeatty10 + Issue: "10656" diff --git a/core/dbt/cli/main.py b/core/dbt/cli/main.py index e042888ef4b..ca79d5eb073 100644 --- a/core/dbt/cli/main.py +++ b/core/dbt/cli/main.py @@ -785,6 +785,8 @@ def freshness(ctx, **kwargs): @click.pass_context @global_flags @p.exclude +@p.resource_type +@p.exclude_resource_type @p.profiles_dir @p.project_dir @p.select diff --git a/core/dbt/node_types.py b/core/dbt/node_types.py index 52503f46ba2..71ef90594a2 100644 --- a/core/dbt/node_types.py +++ b/core/dbt/node_types.py @@ -26,6 +26,11 @@ NodeType.Snapshot, ] +TEST_NODE_TYPES: List["NodeType"] = [ + NodeType.Test, + NodeType.Unit, +] + VERSIONED_NODE_TYPES: List["NodeType"] = [ NodeType.Model, ] diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index d1958d4ef4b..356328a4263 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -3,7 +3,17 @@ import re import threading from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type, Union +from typing import ( + TYPE_CHECKING, + Any, + Collection, + Dict, + List, + Optional, + Tuple, + Type, + Union, +) import daff @@ -25,9 +35,9 @@ from dbt.exceptions import BooleanError, DbtInternalError from dbt.flags import get_flags from dbt.graph import ResourceTypeSelector -from dbt.node_types import NodeType +from dbt.node_types import TEST_NODE_TYPES, NodeType from dbt.parser.unit_tests import UnitTestManifestLoader -from dbt.task.base import BaseRunner +from dbt.task.base import BaseRunner, resource_types_from_args from dbt.utils import _coerce_decimal, strtobool from dbt_common.dataclass_schema import dbtClassMixin from dbt_common.events.format import pluralize @@ -387,6 +397,16 @@ class TestTask(RunTask): def raise_on_first_error(self) -> bool: return False + @property + def resource_types(self) -> List[NodeType]: + resource_types: Collection[NodeType] = resource_types_from_args( + self.args, set(TEST_NODE_TYPES), set(TEST_NODE_TYPES) + ) + + # filter out any non-test node types + resource_types = [rt for rt in resource_types if rt in TEST_NODE_TYPES] + return list(resource_types) + def get_node_selector(self) -> ResourceTypeSelector: if self.manifest is None or self.graph is None: raise DbtInternalError("manifest and graph must be set to get perform node selection") @@ -394,7 +414,7 @@ def get_node_selector(self) -> ResourceTypeSelector: graph=self.graph, manifest=self.manifest, previous_state=self.previous_state, - resource_types=[NodeType.Test, NodeType.Unit], + resource_types=self.resource_types, ) def get_runner_type(self, _) -> Optional[Type[BaseRunner]]: diff --git a/tests/functional/unit_testing/test_unit_testing.py b/tests/functional/unit_testing/test_unit_testing.py index 7332ddccb64..53cfc84f4bf 100644 --- a/tests/functional/unit_testing/test_unit_testing.py +++ b/tests/functional/unit_testing/test_unit_testing.py @@ -76,11 +76,19 @@ def test_basic(self, project): ) assert len(results) == 1 - # Exclude unit tests with environment variable + # Exclude unit tests with environment variable for build command os.environ["DBT_EXCLUDE_RESOURCE_TYPES"] = "unit_test" results = run_dbt(["build", "--select", "my_model"], expect_pass=True) assert len(results) == 1 + # Exclude unit tests with environment variable for test command + results = run_dbt(["test", "--select", "my_model"], expect_pass=True) + assert len(results) == 0 + + # Exclude unit tests with environment variable for list command + results = run_dbt(["list", "--select", "my_model"], expect_pass=True) + assert len(results) == 1 + del os.environ["DBT_EXCLUDE_RESOURCE_TYPES"] # Test select by test name diff --git a/tests/functional/unit_testing/test_ut_resource_types.py b/tests/functional/unit_testing/test_ut_resource_types.py index 85d198bb60a..09f64bdd061 100644 --- a/tests/functional/unit_testing/test_ut_resource_types.py +++ b/tests/functional/unit_testing/test_ut_resource_types.py @@ -46,6 +46,12 @@ def test_unit_test_list(self, project): results = run_dbt(["list", "--exclude-resource-types", "model", "test"]) assert sorted(results) == EXPECTED_UNIT_TESTS + results = run_dbt(["test", "--resource-type", "unit_test"]) + assert len(results) == len(EXPECTED_UNIT_TESTS) + + results = run_dbt(["test", "--exclude-resource-types", "model", "test"]) + assert len(results) == len(EXPECTED_UNIT_TESTS) + # data tests results = run_dbt(["list", "--resource-type", "test"]) assert sorted(results) == EXPECTED_DATA_TESTS @@ -53,6 +59,12 @@ def test_unit_test_list(self, project): results = run_dbt(["list", "--exclude-resource-types", "unit_test", "model"]) assert sorted(results) == EXPECTED_DATA_TESTS + results = run_dbt(["test", "--resource-type", "test"]) + assert len(results) == len(EXPECTED_DATA_TESTS) + + results = run_dbt(["test", "--exclude-resource-types", "unit_test", "model"]) + assert len(results) == len(EXPECTED_DATA_TESTS) + results = run_dbt(["build", "--resource-type", "test"]) assert len(results) == len(EXPECTED_DATA_TESTS) @@ -61,11 +73,17 @@ def test_unit_test_list(self, project): # models results = run_dbt(["list", "--resource-type", "model"]) - assert len(results) == len(EXPECTED_MODELS) + assert sorted(results) == EXPECTED_MODELS results = run_dbt(["list", "--exclude-resource-type", "unit_test", "test"]) assert sorted(results) == EXPECTED_MODELS + results = run_dbt(["test", "--resource-type", "model"]) + assert len(results) == 0 + + results = run_dbt(["test", "--exclude-resource-types", "unit_test", "test"]) + assert len(results) == 0 + results = run_dbt(["build", "--resource-type", "model"]) assert len(results) == len(EXPECTED_MODELS) From 5c0d7ce5bdba73d698d2533f882d635efe5a7d0e Mon Sep 17 00:00:00 2001 From: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:53:16 -0600 Subject: [PATCH 08/10] Standardize returning `ResourceTypeSelector` instances in `dbt list` and `dbt build` (#10739) * Remove duplicated constructor for `ResourceTypeSelector` * Add type annotation for `ResourceTypeSelector` * Standardize on constructor for `ResourceTypeSelector` where `include_empty_nodes=True` * Changelog entry --- .../Under the Hood-20240918-170325.yaml | 7 ++++++ core/dbt/task/build.py | 7 ------ core/dbt/task/list.py | 24 +++++++------------ 3 files changed, 15 insertions(+), 23 deletions(-) create mode 100644 .changes/unreleased/Under the Hood-20240918-170325.yaml diff --git a/.changes/unreleased/Under the Hood-20240918-170325.yaml b/.changes/unreleased/Under the Hood-20240918-170325.yaml new file mode 100644 index 00000000000..3f265a36eda --- /dev/null +++ b/.changes/unreleased/Under the Hood-20240918-170325.yaml @@ -0,0 +1,7 @@ +kind: Under the Hood +body: Standardize returning `ResourceTypeSelector` instances in `dbt list` and `dbt + build` +time: 2024-09-18T17:03:25.639516-06:00 +custom: + Author: dbeatty10 + Issue: "10739" diff --git a/core/dbt/task/build.py b/core/dbt/task/build.py index 832f7f0fe47..625a4498d6a 100644 --- a/core/dbt/task/build.py +++ b/core/dbt/task/build.py @@ -196,13 +196,6 @@ def get_node_selector(self, no_unit_tests=False) -> ResourceTypeSelector: resource_types = self.resource_types(no_unit_tests) - if resource_types == [NodeType.Test]: - return ResourceTypeSelector( - graph=self.graph, - manifest=self.manifest, - previous_state=self.previous_state, - resource_types=resource_types, - ) return ResourceTypeSelector( graph=self.graph, manifest=self.manifest, diff --git a/core/dbt/task/list.py b/core/dbt/task/list.py index 180cfc71693..2638920976a 100644 --- a/core/dbt/task/list.py +++ b/core/dbt/task/list.py @@ -196,24 +196,16 @@ def selection_arg(self): else: return self.args.select - def get_node_selector(self): + def get_node_selector(self) -> ResourceTypeSelector: if self.manifest is None or self.graph is None: raise DbtInternalError("manifest and graph must be set to get perform node selection") - if self.resource_types == [NodeType.Test]: - return ResourceTypeSelector( - graph=self.graph, - manifest=self.manifest, - previous_state=self.previous_state, - resource_types=self.resource_types, - ) - else: - return ResourceTypeSelector( - graph=self.graph, - manifest=self.manifest, - previous_state=self.previous_state, - resource_types=self.resource_types, - include_empty_nodes=True, - ) + return ResourceTypeSelector( + graph=self.graph, + manifest=self.manifest, + previous_state=self.previous_state, + resource_types=self.resource_types, + include_empty_nodes=True, + ) def interpret_results(self, results): # list command should always return 0 as exit code From 5743edc73f6985e6301758bf6279edb51afc359b Mon Sep 17 00:00:00 2001 From: Peter Allen Webb Date: Fri, 20 Sep 2024 18:09:50 -0400 Subject: [PATCH 09/10] Add tests for YAML-defined snapshots. --- tests/functional/snapshots/fixtures.py | 14 ++++++++++++ .../snapshots/test_basic_snapshot.py | 22 +++++++++++++++++++ .../snapshots/test_yaml_defined_snapshot.py | 0 3 files changed, 36 insertions(+) create mode 100644 tests/functional/snapshots/test_yaml_defined_snapshot.py diff --git a/tests/functional/snapshots/fixtures.py b/tests/functional/snapshots/fixtures.py index a94f0c04875..661ebe9fe2e 100644 --- a/tests/functional/snapshots/fixtures.py +++ b/tests/functional/snapshots/fixtures.py @@ -291,6 +291,20 @@ {% endsnapshot %} """ +snapshots_pg__snapshot_yml = """ +version: 2 + +snapshots: + - name: snapshot_actual + relation: "ref('seed')" + config: + unique_key: "id || '-' || first_name" + strategy: timestamp + updated_at: updated_at + meta: + owner: 'a_owner' +""" + snapshots_pg__snapshot_no_target_schema_sql = """ {% snapshot snapshot_actual %} diff --git a/tests/functional/snapshots/test_basic_snapshot.py b/tests/functional/snapshots/test_basic_snapshot.py index ac6c3831642..b5a508b04a9 100644 --- a/tests/functional/snapshots/test_basic_snapshot.py +++ b/tests/functional/snapshots/test_basic_snapshot.py @@ -20,6 +20,7 @@ seeds__seed_newcol_csv, snapshots_pg__snapshot_no_target_schema_sql, snapshots_pg__snapshot_sql, + snapshots_pg__snapshot_yml, snapshots_pg_custom__snapshot_sql, snapshots_pg_custom_namespaced__snapshot_sql, ) @@ -372,3 +373,24 @@ def test_updated_at_snapshot(self, project): class TestRefUpdatedAtCheckCols(UpdatedAtCheckCols): def test_updated_at_ref(self, project): ref_setup(project, num_snapshot_models=2) + + +class BasicYaml(Basic): + @pytest.fixture(scope="class") + def snapshots(self): + """Overrides the same function in Basic to use the YAML method of + defining a snapshot.""" + return {"snapshot.yml": snapshots_pg__snapshot_yml} + + @pytest.fixture(scope="class") + def models(self): + """Overrides the same function in Basic to use a modified version of + schema.yml without snapshot config.""" + return { + "ref_snapshot.sql": models__ref_snapshot_sql, + } + + +class TestBasicSnapshotYaml(BasicYaml): + def test_basic_snapshot_yaml(self, project): + snapshot_setup(project, num_snapshot_models=1) diff --git a/tests/functional/snapshots/test_yaml_defined_snapshot.py b/tests/functional/snapshots/test_yaml_defined_snapshot.py new file mode 100644 index 00000000000..e69de29bb2d From 86cf015cd88697e9b124d8ab0efcfab5053d7435 Mon Sep 17 00:00:00 2001 From: Peter Allen Webb Date: Mon, 23 Sep 2024 12:10:14 -0400 Subject: [PATCH 10/10] Modify scheme used for forming snapshot fqn to better match other resources. --- core/dbt/parser/schemas.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index e0623074d7d..c2a8b798c5a 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -286,7 +286,8 @@ def _add_yaml_snapshot_nodes_to_manifest( # Reuse the logic of SnapshotParser as far as possible to create # a new node we can add to the manifest. parser = SnapshotParser(self.project, self.manifest, self.root_project) - fqn = [self.project.project_name, "snapshots", snapshot["name"]] + fqn = parser.get_fqn_prefix(block.path.relative_path) + fqn.append(snapshot["name"]) snapshot_node = parser._create_parsetime_node( block, self.get_compiled_path(block),