From ebc22fa26c9cb26b6deef86cc0a7ceb1ee3fb642 Mon Sep 17 00:00:00 2001 From: Chenyu Li Date: Thu, 4 Apr 2024 14:34:31 -0700 Subject: [PATCH 01/10] [BUG] Exclude password-like fields for considering reparse (#9844) --- .../unreleased/Fixes-20240402-135556.yaml | 6 ++++ core/dbt/parser/manifest.py | 30 +++++++------------ .../partial_parsing/test_partial_parsing.py | 29 ++++++++++++++++++ .../partial_parsing/test_pp_vars.py | 21 +++++-------- tests/unit/test_parse_manifest.py | 23 ++++++++++++++ 5 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240402-135556.yaml diff --git a/.changes/unreleased/Fixes-20240402-135556.yaml b/.changes/unreleased/Fixes-20240402-135556.yaml new file mode 100644 index 00000000000..b6ba62fc0f7 --- /dev/null +++ b/.changes/unreleased/Fixes-20240402-135556.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Exclude password-like fields for considering reparse +time: 2024-04-02T13:55:56.169953-07:00 +custom: + Author: ChenyuLInx + Issue: "9795" diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 0dd3ec08423..1681c6f4851 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -835,13 +835,6 @@ def is_partial_parsable(self, manifest: Manifest) -> Tuple[bool, Optional[str]]: ) valid = False reparse_reason = ReparseReason.proj_env_vars_changed - if ( - self.manifest.state_check.profile_env_vars_hash - != manifest.state_check.profile_env_vars_hash - ): - fire_event(UnableToPartialParse(reason="env vars used in profiles.yml have changed")) - valid = False - reparse_reason = ReparseReason.prof_env_vars_changed missing_keys = { k @@ -980,18 +973,18 @@ def build_manifest_state_check(self): env_var_str += f"{key}:{config.project_env_vars[key]}|" project_env_vars_hash = FileHash.from_contents(env_var_str) - # Create a FileHash of the env_vars in the project - key_list = list(config.profile_env_vars.keys()) - key_list.sort() - env_var_str = "" - for key in key_list: - env_var_str += f"{key}:{config.profile_env_vars[key]}|" - profile_env_vars_hash = FileHash.from_contents(env_var_str) + # Create a hash of the connection_info, which user has access to in + # jinja context. Thus attributes here may affect the parsing result. + # Ideally we should not expose all of the connection info to the jinja. - # Create a FileHash of the profile file - profile_path = os.path.join(get_flags().PROFILES_DIR, "profiles.yml") - with open(profile_path) as fp: - profile_hash = FileHash.from_contents(fp.read()) + # Renaming this variable mean that we will have to do a whole lot more + # change to make sure the previous manifest can be loaded correctly. + # This is an example of naming should be chosen based on the functionality + # rather than the implementation details. + connection_keys = list(config.credentials.connection_info()) + # avoid reparsing because of ordering issues + connection_keys.sort() + profile_hash = FileHash.from_contents(pprint.pformat(connection_keys)) # Create a FileHashes for dbt_project for all dependencies project_hashes = {} @@ -1003,7 +996,6 @@ def build_manifest_state_check(self): # Create the ManifestStateCheck object state_check = ManifestStateCheck( project_env_vars_hash=project_env_vars_hash, - profile_env_vars_hash=profile_env_vars_hash, vars_hash=vars_hash, profile_hash=profile_hash, project_hashes=project_hashes, diff --git a/tests/functional/partial_parsing/test_partial_parsing.py b/tests/functional/partial_parsing/test_partial_parsing.py index aa38944e678..6b5ba8895cd 100644 --- a/tests/functional/partial_parsing/test_partial_parsing.py +++ b/tests/functional/partial_parsing/test_partial_parsing.py @@ -11,6 +11,7 @@ run_dbt_and_capture, rename_dir, ) +import yaml from tests.functional.utils import up_one from dbt.tests.fixtures.project import write_project_files from tests.functional.partial_parsing.fixtures import ( @@ -824,3 +825,31 @@ def test_pp_renamed_project_dir_changed_project_contents(self, project): run_dbt(["deps"]) len(run_dbt(["--partial-parse", "seed"])) == 1 len(run_dbt(["--partial-parse", "run"])) == 3 + + +class TestProfileChanges: + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": "select 1 as id", + } + + def test_profile_change(self, project, dbt_profile_data): + # Fist run not partial parsing + _, stdout = run_dbt_and_capture(["parse"]) + assert "Unable to do partial parsing because saved manifest not found" in stdout + + _, stdout = run_dbt_and_capture(["parse"]) + assert "Unable to do partial parsing" not in stdout + + # change dbname which is included in the connection_info + dbt_profile_data["test"]["outputs"]["default"]["dbname"] = "dbt2" + write_file(yaml.safe_dump(dbt_profile_data), project.profiles_dir, "profiles.yml") + _, stdout = run_dbt_and_capture(["parse"]) + assert "Unable to do partial parsing because profile has changed" in stdout + + # Change the password which is not included in the connection_info + dbt_profile_data["test"]["outputs"]["default"]["pass"] = "another_password" + write_file(yaml.safe_dump(dbt_profile_data), project.profiles_dir, "profiles.yml") + _, stdout = run_dbt_and_capture(["parse"]) + assert "Unable to do partial parsing" not in stdout diff --git a/tests/functional/partial_parsing/test_pp_vars.py b/tests/functional/partial_parsing/test_pp_vars.py index a01e78c6458..aa93f1131dd 100644 --- a/tests/functional/partial_parsing/test_pp_vars.py +++ b/tests/functional/partial_parsing/test_pp_vars.py @@ -314,40 +314,33 @@ def dbt_profile_target(self): # calls 'load_config' before the tests are run. # Note: only the specified profile is rendered, so there's no # point it setting env_vars in non-used profiles. - os.environ["ENV_VAR_USER"] = "root" - os.environ["ENV_VAR_PASS"] = "password" + os.environ["ENV_VAR_HOST"] = "localhost" return { "type": "postgres", "threads": 4, - "host": "localhost", + "host": "{{ env_var('ENV_VAR_HOST') }}", "port": 5432, - "user": "{{ env_var('ENV_VAR_USER') }}", - "pass": "{{ env_var('ENV_VAR_PASS') }}", + "user": "root", + "pass": "password", "dbname": "dbt", } def test_profile_env_vars(self, project, logs_dir): # Initial run - os.environ["ENV_VAR_USER"] = "root" - os.environ["ENV_VAR_PASS"] = "password" + os.environ["ENV_VAR_HOST"] = "localhost" run_dbt(["run"]) - manifest = get_manifest(project.project_root) - env_vars_checksum = manifest.state_check.profile_env_vars_hash.checksum # Change env_vars, the user doesn't exist, this should fail - os.environ["ENV_VAR_USER"] = "fake_user" + os.environ["ENV_VAR_HOST"] = "wrong_host" # N.B. run_dbt_and_capture won't work here because FailedToConnectError ends the test entirely with pytest.raises(FailedToConnectError): run_dbt(["run"], expect_pass=False) log_output = Path(logs_dir, "dbt.log").read_text() - assert "env vars used in profiles.yml have changed" in log_output - - manifest = get_manifest(project.project_root) - assert env_vars_checksum != manifest.state_check.profile_env_vars_hash.checksum + assert "Unable to do partial parsing because profile has changed" in log_output class TestProfileSecretEnvVars: diff --git a/tests/unit/test_parse_manifest.py b/tests/unit/test_parse_manifest.py index a3113b78de0..23d6d51446d 100644 --- a/tests/unit/test_parse_manifest.py +++ b/tests/unit/test_parse_manifest.py @@ -106,6 +106,18 @@ def _new_file(self, searched, name, match): class TestPartialParse(unittest.TestCase): + def setUp(self) -> None: + mock_project = MagicMock(RuntimeConfig) + mock_project.cli_vars = "" + mock_project.args = MagicMock() + mock_project.args.profile = "test" + mock_project.args.target = "test" + mock_project.project_env_vars = {} + mock_project.profile_env_vars = {} + mock_project.project_target_path = "mock_target_path" + mock_project.credentials = MagicMock() + self.mock_project = mock_project + @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") @patch("dbt.parser.manifest.os.path.exists") @patch("dbt.parser.manifest.open") @@ -122,6 +134,17 @@ def test_partial_parse_file_path(self, patched_open, patched_os_exist, patched_s # if specified in flags, we use the specified path patched_open.assert_called_with("specified_partial_parse_path", "rb") + def test_profile_hash_change(self): + # This test validate that the profile_hash is updated when the connection keys change + profile_hash = "750bc99c1d64ca518536ead26b28465a224be5ffc918bf2a490102faa5a1bcf5" + self.mock_project.credentials.connection_info.return_value = "test" + set_from_args(Namespace(), {}) + manifest = ManifestLoader(self.mock_project, {}) + assert manifest.manifest.state_check.profile_hash.checksum == profile_hash + self.mock_project.credentials.connection_info.return_value = "test1" + manifest = ManifestLoader(self.mock_project, {}) + assert manifest.manifest.state_check.profile_hash.checksum != profile_hash + class TestFailedPartialParse(unittest.TestCase): @patch("dbt.tracking.track_partial_parser") From b945d177d38218ada0a634c25c75d17a1d716e79 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 5 Apr 2024 13:38:55 -0400 Subject: [PATCH 02/10] [Tidy first] Add "Flags" to args parameters in tasks (#9856) * Add "Flags" to args parameter in tasks * Add comment --- core/dbt/cli/flags.py | 5 +++++ core/dbt/task/base.py | 13 +++++++------ core/dbt/task/build.py | 3 ++- core/dbt/task/debug.py | 3 ++- core/dbt/task/list.py | 3 ++- core/dbt/task/retry.py | 2 +- core/dbt/task/run.py | 3 ++- core/dbt/task/runnable.py | 3 ++- 8 files changed, 23 insertions(+), 12 deletions(-) diff --git a/core/dbt/cli/flags.py b/core/dbt/cli/flags.py index 92a6cbc5a28..d87c1aa96eb 100644 --- a/core/dbt/cli/flags.py +++ b/core/dbt/cli/flags.py @@ -352,6 +352,11 @@ def set_common_global_flags(self): if getattr(self, "MACRO_DEBUGGING", None) is not None: jinja.MACRO_DEBUGGING = getattr(self, "MACRO_DEBUGGING") + # This is here to prevent mypy from complaining about all of the + # attributes which we added dynamically. + def __getattr__(self, name: str) -> Any: + return super().__get_attribute__(name) # type: ignore + CommandParams = List[str] diff --git a/core/dbt/task/base.py b/core/dbt/task/base.py index 2ca6cb2e978..b000c70755e 100644 --- a/core/dbt/task/base.py +++ b/core/dbt/task/base.py @@ -12,6 +12,7 @@ import dbt_common.exceptions.base import dbt.exceptions from dbt import tracking +from dbt.cli.flags import Flags from dbt.config import RuntimeConfig, Project from dbt.config.profile import read_profile from dbt.constants import DBT_PROJECT_FILE_NAME @@ -51,7 +52,7 @@ class NoneConfig: @classmethod - def from_args(cls, args): + def from_args(cls, args: Flags): return None @@ -73,13 +74,13 @@ def read_profiles(profiles_dir=None): class BaseTask(metaclass=ABCMeta): ConfigType: Union[Type[NoneConfig], Type[Project]] = NoneConfig - def __init__(self, args, config, project=None) -> None: + def __init__(self, args: Flags, config, project=None) -> None: self.args = args self.config = config self.project = config if isinstance(config, Project) else project @classmethod - def pre_init_hook(cls, args): + def pre_init_hook(cls, args: Flags): """A hook called before the task is initialized.""" if args.log_format == "json": log_manager.format_json() @@ -155,7 +156,7 @@ def move_to_nearest_project_dir(project_dir: Optional[str]) -> Path: class ConfiguredTask(BaseTask): ConfigType = RuntimeConfig - def __init__(self, args, config, manifest: Optional[Manifest] = None) -> None: + def __init__(self, args: Flags, config, manifest: Optional[Manifest] = None) -> None: super().__init__(args, config) self.graph: Optional[Graph] = None self.manifest = manifest @@ -174,7 +175,7 @@ def compile_manifest(self): dbt.tracking.track_runnable_timing({"graph_compilation_elapsed": compile_time}) @classmethod - def from_args(cls, args, *pargs, **kwargs): + def from_args(cls, args: Flags, *pargs, **kwargs): move_to_nearest_project_dir(args.project_dir) return super().from_args(args, *pargs, **kwargs) @@ -487,7 +488,7 @@ def do_skip(self, cause=None): def resource_types_from_args( - args, all_resource_values: Set[NodeType], default_resource_values: Set[NodeType] + args: Flags, all_resource_values: Set[NodeType], default_resource_values: Set[NodeType] ) -> Set[NodeType]: if not args.resource_types: diff --git a/core/dbt/task/build.py b/core/dbt/task/build.py index 5d3a42b3b9f..6c6426ceefd 100644 --- a/core/dbt/task/build.py +++ b/core/dbt/task/build.py @@ -8,6 +8,7 @@ from dbt.artifacts.schemas.results import NodeStatus, RunStatus from dbt.artifacts.schemas.run import RunResult +from dbt.cli.flags import Flags from dbt.graph import ResourceTypeSelector, GraphQueue, Graph from dbt.node_types import NodeType from dbt.task.test import TestSelector @@ -74,7 +75,7 @@ class BuildTask(RunTask): } ALL_RESOURCE_VALUES = frozenset({x for x in RUNNER_MAP.keys()}) - def __init__(self, args, config, manifest) -> None: + def __init__(self, args: Flags, config, manifest) -> None: super().__init__(args, config, manifest) self.selected_unit_tests: Set = set() self.model_to_unit_test_map: Dict[str, List] = {} diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index ea0f636bd6c..51eabaea13e 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -19,6 +19,7 @@ import dbt.exceptions import dbt_common.exceptions from dbt.adapters.factory import get_adapter, register_adapter +from dbt.cli.flags import Flags from dbt.config import PartialProject, Project, Profile from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer from dbt.artifacts.schemas.results import RunStatus @@ -77,7 +78,7 @@ class DebugRunStatus(Flag): class DebugTask(BaseTask): - def __init__(self, args, config) -> None: + def __init__(self, args: Flags, config) -> None: super().__init__(args, config) self.profiles_dir = args.PROFILES_DIR self.profile_path = os.path.join(self.profiles_dir, "profiles.yml") diff --git a/core/dbt/task/list.py b/core/dbt/task/list.py index ff6fea3447e..3ea4b1e2d27 100644 --- a/core/dbt/task/list.py +++ b/core/dbt/task/list.py @@ -8,6 +8,7 @@ SemanticModel, UnitTestDefinition, ) +from dbt.cli.flags import Flags from dbt.flags import get_flags from dbt.graph import ResourceTypeSelector from dbt.task.base import resource_types_from_args @@ -54,7 +55,7 @@ class ListTask(GraphRunnableTask): ) ) - def __init__(self, args, config, manifest) -> None: + def __init__(self, args: Flags, config, manifest) -> None: super().__init__(args, config, manifest) if self.args.models: if self.args.select: diff --git a/core/dbt/task/retry.py b/core/dbt/task/retry.py index 9aadf9ead97..70dea9756f2 100644 --- a/core/dbt/task/retry.py +++ b/core/dbt/task/retry.py @@ -63,7 +63,7 @@ class RetryTask(ConfiguredTask): - def __init__(self, args, config) -> None: + def __init__(self, args: Flags, config) -> None: # load previous run results state_path = args.state or config.target_path self.previous_results = load_result_state( diff --git a/core/dbt/task/run.py b/core/dbt/task/run.py index 2a6031ad45c..4fc6ebc64cf 100644 --- a/core/dbt/task/run.py +++ b/core/dbt/task/run.py @@ -15,6 +15,7 @@ from dbt import tracking from dbt import utils from dbt.adapters.base import BaseRelation +from dbt.cli.flags import Flags from dbt.clients.jinja import MacroGenerator from dbt.context.providers import generate_runtime_model_context from dbt.contracts.graph.nodes import HookNode, ResultNode @@ -305,7 +306,7 @@ def execute(self, model, manifest): class RunTask(CompileTask): - def __init__(self, args, config, manifest) -> None: + def __init__(self, args: Flags, config, manifest) -> None: super().__init__(args, config, manifest) self.ran_hooks: List[HookNode] = [] self._total_executed = 0 diff --git a/core/dbt/task/runnable.py b/core/dbt/task/runnable.py index cff69e23e80..746c08bf656 100644 --- a/core/dbt/task/runnable.py +++ b/core/dbt/task/runnable.py @@ -15,6 +15,7 @@ import dbt.utils from dbt.adapters.base import BaseRelation from dbt.adapters.factory import get_adapter +from dbt.cli.flags import Flags from dbt.contracts.graph.manifest import Manifest from dbt.contracts.graph.nodes import ResultNode from dbt.artifacts.schemas.results import NodeStatus, RunningStatus, RunStatus, BaseResult @@ -65,7 +66,7 @@ class GraphRunnableTask(ConfiguredTask): MARK_DEPENDENT_ERRORS_STATUSES = [NodeStatus.Error] - def __init__(self, args, config, manifest) -> None: + def __init__(self, args: Flags, config, manifest) -> None: super().__init__(args, config, manifest) self._flattened_nodes: Optional[List[ResultNode]] = None self._raise_next_tick: Optional[DbtRuntimeError] = None From 96f54264b4a4a185706051a58c801c9ea3b27385 Mon Sep 17 00:00:00 2001 From: niteshy Date: Fri, 5 Apr 2024 11:19:39 -0700 Subject: [PATCH 03/10] Removing unwanted dev dependency (#9838) * Removing unwanted dev dependency * updating change log * Remove duplicate dependency of protobuf in dev-requirements to resolve #9830 --------- Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> --- .changes/unreleased/Dependencies-20240331-103917.yaml | 6 ++++++ dev-requirements.txt | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/Dependencies-20240331-103917.yaml diff --git a/.changes/unreleased/Dependencies-20240331-103917.yaml b/.changes/unreleased/Dependencies-20240331-103917.yaml new file mode 100644 index 00000000000..c4cb75dd449 --- /dev/null +++ b/.changes/unreleased/Dependencies-20240331-103917.yaml @@ -0,0 +1,6 @@ +kind: Dependencies +body: Remove duplicate dependency of protobuf in dev-requirements +time: 2024-03-31T10:39:17.432017-07:00 +custom: + Author: niteshy + Issue: "9830" diff --git a/dev-requirements.txt b/dev-requirements.txt index 0c706dddbbe..f0c5413def7 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -14,7 +14,6 @@ ipdb mypy==1.4.1 pip-tools pre-commit -protobuf>=4.0.0,<5.0.0 pytest>=7.4,<8.0 pytest-cov pytest-csv>=3.0,<4.0 From e81f7fdbd5b248d7cc24847da1ac6eee453f9d1a Mon Sep 17 00:00:00 2001 From: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Date: Fri, 5 Apr 2024 15:26:52 -0600 Subject: [PATCH 04/10] Only create the `packages-install-path` / `dbt_packages` folder during `dbt deps` (#9810) * Changelog entry * The packages-install-path should not be created by `dbt list` * Only create packages-install-path during `dbt deps` --- .changes/unreleased/Fixes-20240323-124558.yaml | 6 ++++++ core/dbt/compilation.py | 1 - tests/functional/list/test_list.py | 7 +++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/Fixes-20240323-124558.yaml diff --git a/.changes/unreleased/Fixes-20240323-124558.yaml b/.changes/unreleased/Fixes-20240323-124558.yaml new file mode 100644 index 00000000000..b36173325ba --- /dev/null +++ b/.changes/unreleased/Fixes-20240323-124558.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Only create the packages-install-path / dbt_packages folder during dbt deps +time: 2024-03-23T12:45:58.159017-06:00 +custom: + Author: dbeatty10 + Issue: 6985 9584 diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index 388713842ed..09695fc59d7 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -274,7 +274,6 @@ def __init__(self, config) -> None: def initialize(self): make_directory(self.config.project_target_path) - make_directory(self.config.packages_install_path) # creates a ModelContext which is converted to # a dict for jinja rendering of SQL diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index b636bf20d39..4eb8cb8e2f9 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -37,6 +37,13 @@ def project_config_update(self): }, } + def test_packages_install_path_does_not_exist(self, project): + run_dbt(["list"]) + packages_install_path = "dbt_packages" + + # the packages-install-path should not be created by `dbt list` + assert not os.path.exists(packages_install_path) + def run_dbt_ls(self, args=None, expect_pass=True): log_manager.stdout_console() full_args = ["ls"] From cf08b8411a3f847a1c2973a867b1744a6d19bb3b Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Mon, 8 Apr 2024 12:51:20 -0700 Subject: [PATCH 05/10] Remove try/except logic for importing `Empty` from `queue` library (#9877) Following [PEP8](https://peps.python.org/pep-0008/#package-and-module-names) the Python 2 library `Queue` was renamed to `queue` in [Python 3](https://peps.python.org/pep-3108/#pep-8-violations-done). Our try/except logic was to ensure the `Empty` class was imported without error when running Python 2. Python 2 went EOL January 1st, 2020 and we haven't supported Python 2 in a very long time. As such, it seems past time to remove this relic. --- tests/unit/test_graph.py | 7 +------ tests/unit/test_linker.py | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/unit/test_graph.py b/tests/unit/test_graph.py index d4282bcff95..96e058796b8 100644 --- a/tests/unit/test_graph.py +++ b/tests/unit/test_graph.py @@ -20,12 +20,7 @@ from dbt.graph import NodeSelector, parse_difference from dbt.events.logging import setup_event_logger from dbt.mp_context import get_mp_context - -try: - from queue import Empty -except ImportError: - from Queue import Empty - +from queue import Empty from .utils import config_from_parts_or_dicts, generate_name_macros, inject_plugin diff --git a/tests/unit/test_linker.py b/tests/unit/test_linker.py index 9c36ae19674..973a41c062b 100644 --- a/tests/unit/test_linker.py +++ b/tests/unit/test_linker.py @@ -4,14 +4,9 @@ from unittest import mock from dbt import compilation - -try: - from queue import Empty -except ImportError: - from Queue import Empty - from dbt.graph.selector import NodeSelector from dbt.graph.cli import parse_difference +from queue import Empty def _mock_manifest(nodes): From 1e4e15c023926cfb1571fe0e192b9877c496bc57 Mon Sep 17 00:00:00 2001 From: Samuel Favarin Date: Tue, 9 Apr 2024 14:23:12 +0200 Subject: [PATCH 06/10] Better error message when trying to select a disabled model (#9863) * feat: update log message * chore: add change log * Body for changelog entry --------- Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> --- .changes/unreleased/Features-20240405-175733.yaml | 6 ++++++ core/dbt/events/types.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/Features-20240405-175733.yaml diff --git a/.changes/unreleased/Features-20240405-175733.yaml b/.changes/unreleased/Features-20240405-175733.yaml new file mode 100644 index 00000000000..0346361fc15 --- /dev/null +++ b/.changes/unreleased/Features-20240405-175733.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Better error message when trying to select a disabled model +time: 2024-04-05T17:57:33.047963+02:00 +custom: + Author: SamuelBFavarin + Issue: "9747" diff --git a/core/dbt/events/types.py b/core/dbt/events/types.py index b8ce80aa5de..391d0c22002 100644 --- a/core/dbt/events/types.py +++ b/core/dbt/events/types.py @@ -1099,7 +1099,7 @@ def code(self) -> str: return "M030" def message(self) -> str: - return f"The selection criterion '{self.spec_raw}' does not match any nodes" + return f"The selection criterion '{self.spec_raw}' does not match any enabled nodes" class DepsLockUpdating(InfoLevel): From fb41ce93d63ff028513f22d9f130fb2710262942 Mon Sep 17 00:00:00 2001 From: "Reza J. Bavaghoush" Date: Wed, 10 Apr 2024 03:15:21 +0200 Subject: [PATCH 07/10] fix(core): relax pathspec upper bound version limitation (#9373) * fix(core): relax pathspec upper bound version limitation * add changelog entry PR #9373 --- .changes/unreleased/Dependencies-20240117-100818.yaml | 6 ++++++ core/setup.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/Dependencies-20240117-100818.yaml diff --git a/.changes/unreleased/Dependencies-20240117-100818.yaml b/.changes/unreleased/Dependencies-20240117-100818.yaml new file mode 100644 index 00000000000..ca318fe1e57 --- /dev/null +++ b/.changes/unreleased/Dependencies-20240117-100818.yaml @@ -0,0 +1,6 @@ +kind: Dependencies +body: Relax pathspec upper bound version restriction +time: 2024-01-17T10:08:18.009949641+01:00 +custom: + Author: rzjfr + PR: "9373" diff --git a/core/setup.py b/core/setup.py index f1e1856ca73..f8b96f2fcaa 100644 --- a/core/setup.py +++ b/core/setup.py @@ -66,7 +66,7 @@ # ---- # These packages are major-version-0. Keep upper bounds on upcoming minor versions (which could have breaking changes) # and check compatibility / bump in each new minor version of dbt-core. - "pathspec>=0.9,<0.12", + "pathspec>=0.9,<0.13", "sqlparse>=0.2.3,<0.5", # ---- # These are major-version-0 packages also maintained by dbt-labs. From ebacedd89df86bc36e1c81205c5f5883c3f3e645 Mon Sep 17 00:00:00 2001 From: Jaejun <63435794+jx2lee@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:19:37 +0900 Subject: [PATCH 08/10] Fix configuration of turning test warnings into failures (#9347) --- .../unreleased/Fixes-20240108-232035.yaml | 6 ++++ core/dbt/task/test.py | 7 ++-- tests/functional/test_singular_tests.py | 34 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240108-232035.yaml create mode 100644 tests/functional/test_singular_tests.py diff --git a/.changes/unreleased/Fixes-20240108-232035.yaml b/.changes/unreleased/Fixes-20240108-232035.yaml new file mode 100644 index 00000000000..227332f7af1 --- /dev/null +++ b/.changes/unreleased/Fixes-20240108-232035.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: fix configuration of turning test warnings into failures with WARN_ERROR_OPTIONS +time: 2024-01-08T23:20:35.339102+09:00 +custom: + Author: jx2lee + Issue: "7761" diff --git a/core/dbt/task/test.py b/core/dbt/task/test.py index 7f5ed77f7e1..8a82c7f1243 100644 --- a/core/dbt/task/test.py +++ b/core/dbt/task/test.py @@ -87,6 +87,7 @@ class UnitTestResultData(dbtClassMixin): class TestRunner(CompileRunner): _ANSI_ESCAPE = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") + _LOG_TEST_RESULT_EVENTS = LogTestResult def describe_node_name(self): if self.node.resource_type == NodeType.Unit: @@ -102,7 +103,7 @@ def print_result_line(self, result): model = result.node fire_event( - LogTestResult( + self._LOG_TEST_RESULT_EVENTS( name=self.describe_node_name(), status=str(result.status), index=self.node_index, @@ -280,7 +281,9 @@ def build_test_run_result(self, test: TestNode, result: TestResultData) -> RunRe message = f"Got {num_errors}, configured to fail if {test.config.error_if}" failures = result.failures elif result.should_warn: - if get_flags().WARN_ERROR: + if get_flags().WARN_ERROR or get_flags().WARN_ERROR_OPTIONS.includes( + self._LOG_TEST_RESULT_EVENTS.__name__ + ): status = TestStatus.Fail message = f"Got {num_errors}, configured to fail if {test.config.warn_if}" else: diff --git a/tests/functional/test_singular_tests.py b/tests/functional/test_singular_tests.py new file mode 100644 index 00000000000..a4b9d05b510 --- /dev/null +++ b/tests/functional/test_singular_tests.py @@ -0,0 +1,34 @@ +import pytest + +from dbt.tests.util import run_dbt + +single_test_sql = """ +{{ config(warn_if = '>0', error_if ="> 10") }} + +select 1 as issue +""" + + +class TestSingularTestWarnError: + @pytest.fixture(scope="class") + def tests(self): + return {"single_test.sql": single_test_sql} + + def test_singular_test_warn_error(self, project): + results = run_dbt(["--warn-error", "test"], expect_pass=False) + assert results.results[0].status == "fail" + + def test_singular_test_warn_error_options(self, project): + results = run_dbt( + ["--warn-error-options", "{'include': 'all'}", "test"], expect_pass=False + ) + assert results.results[0].status == "fail" + + def test_singular_test_equals_warn_error(self, project): + results = run_dbt(["--warn-error", "test"], expect_pass=False) + warn_error_result = results.results[0].status + + results = run_dbt( + ["--warn-error-options", "{'include': 'all'}", "test"], expect_pass=False + ) + assert warn_error_result == results.results[0].status From d03292e8b99f86a40b8a720d45137ad915768ce7 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 9 Apr 2024 23:51:15 -0700 Subject: [PATCH 09/10] Delete empty files that have been floating around with no future plans (#9878) * Delete empty `test_compiler.py` file Over a year ago in [#7008](https://github.com/dbt-labs/dbt-core/pull/7008/files#diff-c10753a61a1d8f973ec3206206d72b3c5200e41673ab2c6b256f333103eadb8f) the tests in `test_compiler.py` got moved into other testing files. Since then this file has been sitting around empty. It is probably a reasonable time to say goodbye to it. * Delete empty `searcher.py` file The file `searcher.py` was added, as an empty file, four years ago in [#2312](https://github.com/dbt-labs/dbt-core/pull/2312/files#diff-c8e9e62cf63356f44fe1a2b0272b239d7a650c57911477ed4549b15ee3de16fa). Since then it has never been touched or added to. I'm not sure of the initial purpose of the file as it never contained anything and I have not found any documentation in reference to it. It's safe to say, it can go away. --- core/dbt/contracts/graph/searcher.py | 0 tests/unit/test_compiler.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 core/dbt/contracts/graph/searcher.py delete mode 100644 tests/unit/test_compiler.py diff --git a/core/dbt/contracts/graph/searcher.py b/core/dbt/contracts/graph/searcher.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/unit/test_compiler.py b/tests/unit/test_compiler.py deleted file mode 100644 index e69de29bb2d..00000000000 From a1f005789dd477ad165fcb0c9bdf0db1cec431c0 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Wed, 10 Apr 2024 09:30:53 -0400 Subject: [PATCH 10/10] [Tidy first] Task config type (#9874) --- core/dbt/cli/main.py | 3 +-- core/dbt/deps/resolver.py | 15 +++++------ core/dbt/task/base.py | 57 +++++++++++++++------------------------ core/dbt/task/build.py | 4 ++- core/dbt/task/clean.py | 7 +++++ core/dbt/task/debug.py | 11 ++------ core/dbt/task/deps.py | 7 ++--- core/dbt/task/list.py | 4 ++- core/dbt/task/retry.py | 2 +- core/dbt/task/run.py | 4 ++- core/dbt/task/runnable.py | 12 ++++++--- 11 files changed, 61 insertions(+), 65 deletions(-) diff --git a/core/dbt/cli/main.py b/core/dbt/cli/main.py index deff7d8d341..07a9de861a7 100644 --- a/core/dbt/cli/main.py +++ b/core/dbt/cli/main.py @@ -399,7 +399,6 @@ def debug(ctx, **kwargs): task = DebugTask( ctx.obj["flags"], - None, ) results = task.run() @@ -464,7 +463,7 @@ def init(ctx, **kwargs): """Initialize a new dbt project.""" from dbt.task.init import InitTask - task = InitTask(ctx.obj["flags"], None) + task = InitTask(ctx.obj["flags"]) results = task.run() success = task.interpret_results(results) diff --git a/core/dbt/deps/resolver.py b/core/dbt/deps/resolver.py index 3d74bac4980..5f890109b0e 100644 --- a/core/dbt/deps/resolver.py +++ b/core/dbt/deps/resolver.py @@ -1,5 +1,5 @@ from dataclasses import dataclass, field -from typing import Dict, List, NoReturn, Union, Type, Iterator, Set, Any +from typing import Dict, List, NoReturn, Type, Iterator, Set, Any from dbt.exceptions import ( DuplicateDependencyToRootError, @@ -17,14 +17,13 @@ from dbt.deps.registry import RegistryUnpinnedPackage from dbt.contracts.project import ( + PackageSpec, LocalPackage, TarballPackage, GitPackage, RegistryPackage, ) -PackageContract = Union[LocalPackage, TarballPackage, GitPackage, RegistryPackage] - @dataclass class PackageListing: @@ -68,7 +67,7 @@ def incorporate(self, package: UnpinnedPackage): else: self.packages[key] = package - def update_from(self, src: List[PackageContract]) -> None: + def update_from(self, src: List[PackageSpec]) -> None: pkg: UnpinnedPackage for contract in src: if isinstance(contract, LocalPackage): @@ -84,9 +83,7 @@ def update_from(self, src: List[PackageContract]) -> None: self.incorporate(pkg) @classmethod - def from_contracts( - cls: Type["PackageListing"], src: List[PackageContract] - ) -> "PackageListing": + def from_contracts(cls: Type["PackageListing"], src: List[PackageSpec]) -> "PackageListing": self = cls({}) self.update_from(src) return self @@ -114,7 +111,7 @@ def _check_for_duplicate_project_names( def resolve_packages( - packages: List[PackageContract], + packages: List[PackageSpec], project: Project, cli_vars: Dict[str, Any], ) -> List[PinnedPackage]: @@ -137,7 +134,7 @@ def resolve_packages( return resolved -def resolve_lock_packages(packages: List[PackageContract]) -> List[PinnedPackage]: +def resolve_lock_packages(packages: List[PackageSpec]) -> List[PinnedPackage]: lock_packages = PackageListing.from_contracts(packages) final = PackageListing() diff --git a/core/dbt/task/base.py b/core/dbt/task/base.py index b000c70755e..d4c206b023c 100644 --- a/core/dbt/task/base.py +++ b/core/dbt/task/base.py @@ -6,14 +6,14 @@ from contextlib import nullcontext from datetime import datetime from pathlib import Path -from typing import Any, Dict, List, Optional, Type, Union, Set +from typing import Any, Dict, List, Optional, Set from dbt.compilation import Compiler import dbt_common.exceptions.base import dbt.exceptions from dbt import tracking from dbt.cli.flags import Flags -from dbt.config import RuntimeConfig, Project +from dbt.config import RuntimeConfig from dbt.config.profile import read_profile from dbt.constants import DBT_PROJECT_FILE_NAME from dbt.contracts.graph.manifest import Manifest @@ -50,12 +50,6 @@ from dbt.task.printer import print_run_result_error -class NoneConfig: - @classmethod - def from_args(cls, args: Flags): - return None - - def read_profiles(profiles_dir=None): """This is only used for some error handling""" if profiles_dir is None: @@ -72,12 +66,8 @@ def read_profiles(profiles_dir=None): class BaseTask(metaclass=ABCMeta): - ConfigType: Union[Type[NoneConfig], Type[Project]] = NoneConfig - - def __init__(self, args: Flags, config, project=None) -> None: + def __init__(self, args: Flags) -> None: self.args = args - self.config = config - self.project = config if isinstance(config, Project) else project @classmethod def pre_init_hook(cls, args: Flags): @@ -94,23 +84,6 @@ def set_log_format(cls): else: log_manager.format_text() - @classmethod - def from_args(cls, args, *pargs, **kwargs): - try: - # This is usually RuntimeConfig - config = cls.ConfigType.from_args(args) - except dbt.exceptions.DbtProjectError as exc: - fire_event(LogDbtProjectError(exc=str(exc))) - - tracking.track_invalid_invocation(args=args, result_type=exc.result_type) - raise dbt_common.exceptions.DbtRuntimeError("Could not run dbt") from exc - except dbt.exceptions.DbtProfileError as exc: - all_profile_names = list(read_profiles(get_flags().PROFILES_DIR).keys()) - fire_event(LogDbtProfileError(exc=str(exc), profiles=all_profile_names)) - tracking.track_invalid_invocation(args=args, result_type=exc.result_type) - raise dbt_common.exceptions.DbtRuntimeError("Could not run dbt") from exc - return cls(args, config, *pargs, **kwargs) - @abstractmethod def run(self): raise dbt_common.exceptions.base.NotImplementedError("Not Implemented") @@ -154,10 +127,11 @@ def move_to_nearest_project_dir(project_dir: Optional[str]) -> Path: # produce the same behavior. currently this class only contains manifest compilation, # holding a manifest, and moving direcories. class ConfiguredTask(BaseTask): - ConfigType = RuntimeConfig - - def __init__(self, args: Flags, config, manifest: Optional[Manifest] = None) -> None: - super().__init__(args, config) + def __init__( + self, args: Flags, config: RuntimeConfig, manifest: Optional[Manifest] = None + ) -> None: + super().__init__(args) + self.config = config self.graph: Optional[Graph] = None self.manifest = manifest self.compiler = Compiler(self.config) @@ -177,7 +151,20 @@ def compile_manifest(self): @classmethod def from_args(cls, args: Flags, *pargs, **kwargs): move_to_nearest_project_dir(args.project_dir) - return super().from_args(args, *pargs, **kwargs) + try: + # This is usually RuntimeConfig + config = RuntimeConfig.from_args(args) + except dbt.exceptions.DbtProjectError as exc: + fire_event(LogDbtProjectError(exc=str(exc))) + + tracking.track_invalid_invocation(args=args, result_type=exc.result_type) + raise dbt_common.exceptions.DbtRuntimeError("Could not run dbt") from exc + except dbt.exceptions.DbtProfileError as exc: + all_profile_names = list(read_profiles(get_flags().PROFILES_DIR).keys()) + fire_event(LogDbtProfileError(exc=str(exc), profiles=all_profile_names)) + tracking.track_invalid_invocation(args=args, result_type=exc.result_type) + raise dbt_common.exceptions.DbtRuntimeError("Could not run dbt") from exc + return cls(args, config, *pargs, **kwargs) class ExecutionContext: diff --git a/core/dbt/task/build.py b/core/dbt/task/build.py index 6c6426ceefd..57f11c71bd5 100644 --- a/core/dbt/task/build.py +++ b/core/dbt/task/build.py @@ -9,6 +9,8 @@ from dbt.artifacts.schemas.results import NodeStatus, RunStatus from dbt.artifacts.schemas.run import RunResult from dbt.cli.flags import Flags +from dbt.config.runtime import RuntimeConfig +from dbt.contracts.graph.manifest import Manifest from dbt.graph import ResourceTypeSelector, GraphQueue, Graph from dbt.node_types import NodeType from dbt.task.test import TestSelector @@ -75,7 +77,7 @@ class BuildTask(RunTask): } ALL_RESOURCE_VALUES = frozenset({x for x in RUNNER_MAP.keys()}) - def __init__(self, args: Flags, config, manifest) -> None: + def __init__(self, args: Flags, config: RuntimeConfig, manifest: Manifest) -> None: super().__init__(args, config, manifest) self.selected_unit_tests: Set = set() self.model_to_unit_test_map: Dict[str, List] = {} diff --git a/core/dbt/task/clean.py b/core/dbt/task/clean.py index efae26bf6e1..c4e98f5db2b 100644 --- a/core/dbt/task/clean.py +++ b/core/dbt/task/clean.py @@ -9,6 +9,8 @@ FinishedCleanPaths, ) from dbt_common.exceptions import DbtRuntimeError +from dbt.cli.flags import Flags +from dbt.config.project import Project from dbt.task.base import ( BaseTask, move_to_nearest_project_dir, @@ -16,6 +18,11 @@ class CleanTask(BaseTask): + def __init__(self, args: Flags, config: Project): + super().__init__(args) + self.config = config + self.project = config + def run(self): """ This function takes all the paths in the target file diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 51eabaea13e..b388e4336ba 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -78,8 +78,8 @@ class DebugRunStatus(Flag): class DebugTask(BaseTask): - def __init__(self, args: Flags, config) -> None: - super().__init__(args, config) + def __init__(self, args: Flags) -> None: + super().__init__(args) self.profiles_dir = args.PROFILES_DIR self.profile_path = os.path.join(self.profiles_dir, "profiles.yml") try: @@ -98,13 +98,6 @@ def __init__(self, args: Flags, config) -> None: self.profile: Optional[Profile] = None self.raw_profile_data: Optional[Dict[str, Any]] = None self.profile_name: Optional[str] = None - self.project: Optional[Project] = None - - @property - def project_profile(self): - if self.project is None: - return None - return self.project.profile_name def run(self) -> bool: # WARN: this is a legacy workflow that is not compatible with other runtime flags diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 85788fe440f..0f8e45f073f 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -13,7 +13,7 @@ from dbt.deps.base import downloads_directory from dbt.deps.resolver import resolve_lock_packages, resolve_packages from dbt.deps.registry import RegistryPinnedPackage -from dbt.contracts.project import Package +from dbt.contracts.project import PackageSpec from dbt_common.events.functions import fire_event @@ -44,7 +44,7 @@ def increase_indent(self, flow=False, indentless=False): return super(dbtPackageDumper, self).increase_indent(flow, False) -def _create_sha1_hash(packages: List[Package]) -> str: +def _create_sha1_hash(packages: List[PackageSpec]) -> str: """Create a SHA1 hash of the packages list, this is used to determine if the packages for current execution matches the previous lock. @@ -94,14 +94,15 @@ def _create_packages_yml_entry(package: str, version: Optional[str], source: str class DepsTask(BaseTask): def __init__(self, args: Any, project: Project) -> None: + super().__init__(args=args) # N.B. This is a temporary fix for a bug when using relative paths via # --project-dir with deps. A larger overhaul of our path handling methods # is needed to fix this the "right" way. # See GH-7615 project.project_root = str(Path(project.project_root).resolve()) + self.project = project move_to_nearest_project_dir(project.project_root) - super().__init__(args=args, config=None, project=project) self.cli_vars = args.vars def track_package_install( diff --git a/core/dbt/task/list.py b/core/dbt/task/list.py index 3ea4b1e2d27..e345bc78d94 100644 --- a/core/dbt/task/list.py +++ b/core/dbt/task/list.py @@ -9,6 +9,8 @@ UnitTestDefinition, ) from dbt.cli.flags import Flags +from dbt.config.runtime import RuntimeConfig +from dbt.contracts.graph.manifest import Manifest from dbt.flags import get_flags from dbt.graph import ResourceTypeSelector from dbt.task.base import resource_types_from_args @@ -55,7 +57,7 @@ class ListTask(GraphRunnableTask): ) ) - def __init__(self, args: Flags, config, manifest) -> None: + def __init__(self, args: Flags, config: RuntimeConfig, manifest: Manifest) -> None: super().__init__(args, config, manifest) if self.args.models: if self.args.select: diff --git a/core/dbt/task/retry.py b/core/dbt/task/retry.py index 70dea9756f2..57724f455e0 100644 --- a/core/dbt/task/retry.py +++ b/core/dbt/task/retry.py @@ -63,7 +63,7 @@ class RetryTask(ConfiguredTask): - def __init__(self, args: Flags, config) -> None: + def __init__(self, args: Flags, config: RuntimeConfig) -> None: # load previous run results state_path = args.state or config.target_path self.previous_results = load_result_state( diff --git a/core/dbt/task/run.py b/core/dbt/task/run.py index 4fc6ebc64cf..b57d39c785b 100644 --- a/core/dbt/task/run.py +++ b/core/dbt/task/run.py @@ -17,8 +17,10 @@ from dbt.adapters.base import BaseRelation from dbt.cli.flags import Flags from dbt.clients.jinja import MacroGenerator +from dbt.config.runtime import RuntimeConfig from dbt.context.providers import generate_runtime_model_context from dbt.contracts.graph.nodes import HookNode, ResultNode +from dbt.contracts.graph.manifest import Manifest from dbt.artifacts.schemas.results import NodeStatus, RunStatus, RunningStatus, BaseResult from dbt.artifacts.schemas.run import RunResult from dbt.artifacts.resources import Hook @@ -306,7 +308,7 @@ def execute(self, model, manifest): class RunTask(CompileTask): - def __init__(self, args: Flags, config, manifest) -> None: + def __init__(self, args: Flags, config: RuntimeConfig, manifest: Manifest) -> None: super().__init__(args, config, manifest) self.ran_hooks: List[HookNode] = [] self._total_executed = 0 diff --git a/core/dbt/task/runnable.py b/core/dbt/task/runnable.py index 746c08bf656..6593053c285 100644 --- a/core/dbt/task/runnable.py +++ b/core/dbt/task/runnable.py @@ -16,6 +16,7 @@ from dbt.adapters.base import BaseRelation from dbt.adapters.factory import get_adapter from dbt.cli.flags import Flags +from dbt.config.runtime import RuntimeConfig from dbt.contracts.graph.manifest import Manifest from dbt.contracts.graph.nodes import ResultNode from dbt.artifacts.schemas.results import NodeStatus, RunningStatus, RunStatus, BaseResult @@ -66,8 +67,9 @@ class GraphRunnableTask(ConfiguredTask): MARK_DEPENDENT_ERRORS_STATUSES = [NodeStatus.Error] - def __init__(self, args: Flags, config, manifest) -> None: + def __init__(self, args: Flags, config: RuntimeConfig, manifest: Manifest) -> None: super().__init__(args, config, manifest) + self.config = config self._flattened_nodes: Optional[List[ResultNode]] = None self._raise_next_tick: Optional[DbtRuntimeError] = None self._skipped_children: Dict[str, Optional[RunResult]] = {} @@ -124,7 +126,9 @@ def get_selection_spec(self) -> SelectionSpec: # This is what's used with no default selector and no selection # use --select and --exclude args spec = parse_difference(self.selection_arg, self.exclusion_arg, indirect_selection) - return spec + # mypy complains because the return values of get_selector and parse_difference + # are different + return spec # type: ignore @abstractmethod def get_node_selector(self) -> NodeSelector: @@ -624,7 +628,9 @@ def create_schema(relation: BaseRelation) -> None: list_futures = [] create_futures = [] - with dbt_common.utils.executor(self.config) as tpe: + # TODO: following has a mypy issue because profile and project config + # defines threads as int and HasThreadingConfig defines it as Optional[int] + with dbt_common.utils.executor(self.config) as tpe: # type: ignore for req in required_databases: if req.database is None: name = "list_schemas"