From 1659c7b2b7052a2fcc857b48d2e5b441422270ab Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 25 Jul 2024 21:01:02 -0400 Subject: [PATCH 01/23] persist schema yml unrendered_config before rendering --- core/dbt/context/providers.py | 3 + core/dbt/contracts/graph/manifest.py | 1 + core/dbt/parser/base.py | 8 ++ core/dbt/parser/schemas.py | 16 ++- tests/functional/defer_state/fixtures.py | 27 +++++ .../test_modified_state_environment_aware.py | 109 ++++++++++++++++++ 6 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 tests/functional/defer_state/test_modified_state_environment_aware.py diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index bbb5f269c93..1962deafa13 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -371,6 +371,9 @@ def __call__(self, *args, **kwargs): # not call it! if self.context_config is None: raise DbtRuntimeError("At parse time, did not receive a context config") + + # TODO: consider skipping this for sql state:modified configs. args rendered so this will produced a rendered config unless + # statically extracted self.context_config.add_config_call(opts) return "" diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index 21c5571b74b..ab3a4e5b9c6 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -827,6 +827,7 @@ class Manifest(MacroMethods, dbtClassMixin): unit_tests: MutableMapping[str, UnitTestDefinition] = field(default_factory=dict) saved_queries: MutableMapping[str, SavedQuery] = field(default_factory=dict) fixtures: MutableMapping[str, UnitTestFileFixture] = field(default_factory=dict) + unrendered_patch_configs: MutableMapping[str, Dict[str, Any]] = field(default_factory=dict) _doc_lookup: Optional[DocLookup] = field( default=None, metadata={"serialize": lambda x: None, "deserialize": lambda x: None} diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index a68f7384c0b..b619de73aae 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -308,6 +308,7 @@ def update_parsed_node_config( config: ContextConfig, context=None, patch_config_dict=None, + patch_original_file_path=None, ) -> None: """Given the ContextConfig used for parsing and the parsed node, generate and set the true values to use, overriding the temporary parse @@ -371,6 +372,13 @@ def update_parsed_node_config( # unrendered_config is used to compare the original database/schema/alias # values and to handle 'same_config' and 'same_contents' calls + if patch_original_file_path: + # Use the unrendered_patch_configs if available, provided patch_config_dict may actuallly already be rendered + if unrendered_patch_config_dict := self.manifest.unrendered_patch_configs.get( + patch_original_file_path + ): + patch_config_dict = unrendered_patch_config_dict + parsed_node.unrendered_config = config.build_config_dict( rendered=False, patch_config_dict=patch_config_dict ) diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 5e269fd385c..70f2087fbc6 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -339,9 +339,18 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: if "name" not in entry and "model" not in entry: raise ParsingError("Entry did not contain a name") + unrendered_config = None + if "config" in entry: + unrendered_config = entry["config"] + # Render the data (except for tests, data_tests and descriptions). # See the SchemaYamlRenderer entry = self.render_entry(entry) + + # TODO: consider not storing if rendered config == unrendered_config + if unrendered_config: + self.schema_parser.manifest.unrendered_patch_configs[path] = unrendered_config + if self.schema_yaml_vars.env_vars: self.schema_parser.manifest.env_vars.update(self.schema_yaml_vars.env_vars) schema_file = self.yaml.file @@ -608,7 +617,12 @@ def patch_node_config(self, node, patch) -> None: ) # We need to re-apply the config_call_dict after the patch config config._config_call_dict = node.config_call_dict - self.schema_parser.update_parsed_node_config(node, config, patch_config_dict=patch.config) + self.schema_parser.update_parsed_node_config( + node, + config, + patch_config_dict=patch.config, + patch_original_file_path=patch.original_file_path, + ) # Subclasses of NodePatchParser: TestablePatchParser, ModelPatchParser, AnalysisPatchParser, diff --git a/tests/functional/defer_state/fixtures.py b/tests/functional/defer_state/fixtures.py index 832bf258f56..a2b721c05c1 100644 --- a/tests/functional/defer_state/fixtures.py +++ b/tests/functional/defer_state/fixtures.py @@ -540,3 +540,30 @@ metricflow_time_spine_sql = """ SELECT to_date('02/20/2023', 'mm/dd/yyyy') as date_day """ + + +model_with_env_var_in_config_sql = """ +{{ config(materialized=env_var('DBT_TEST_STATE_MODIFIED')) }} +select 1 as id +""" + +model_with_no_in_config_sql = """ +select 1 as id +""" + + +schema_model_with_env_var_in_config_yml = """ +models: + - name: model + config: + materialized: "{{ env_var('DBT_TEST_STATE_MODIFIED') }}" + +""" + +schema_source_with_env_var_as_property_yml = """ +sources: + - name: jaffle_shop + database: "{{ env_var('DBT_TEST_STATE_MODIFIED') }}" + tables: + - name: customers +""" diff --git a/tests/functional/defer_state/test_modified_state_environment_aware.py b/tests/functional/defer_state/test_modified_state_environment_aware.py new file mode 100644 index 00000000000..9fc2168d5d6 --- /dev/null +++ b/tests/functional/defer_state/test_modified_state_environment_aware.py @@ -0,0 +1,109 @@ +import os + +import pytest + +from dbt.tests.util import run_dbt +from tests.functional.defer_state.fixtures import ( # model_with_env_var_in_config_sql,; schema_source_with_env_var_as_property_yml, + model_with_no_in_config_sql, + schema_model_with_env_var_in_config_yml, +) +from tests.functional.defer_state.test_modified_state import BaseModifiedState + + +class BaseNodeWithEnvVarConfig(BaseModifiedState): + @pytest.fixture(scope="class") + def seeds(self): + return {} + + @pytest.fixture(scope="class", autouse=True) + def setup(self): + os.environ["DBT_TEST_STATE_MODIFIED"] = "table" + yield + del os.environ["DBT_TEST_STATE_MODIFIED"] + + def test_change_env_var(self, project): + # Generate ./state without changing environment variable value + run_dbt(["run"]) + self.copy_state() + + # Assert no false positive + results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert len(results) == 0 + + # Change environment variable and assert no + # Environment variables do not have an effect on state:modified + os.environ["DBT_TEST_STATE_MODIFIED"] = "view" + results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert len(results) == 0 + + +# class TestModelNodeWithEnvVarConfigInSqlFile(BaseNodeWithEnvVarConfig): +# @pytest.fixture(scope="class") +# def models(self): +# return { +# "model.sql": model_with_env_var_in_config_sql, +# } + + +class TestModelNodeWithEnvVarConfigInSchemaYml(BaseNodeWithEnvVarConfig): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_no_in_config_sql, + "schema.yml": schema_model_with_env_var_in_config_yml, + } + + +class TestModelNodeWithEnvVarConfigInProjectYml(BaseNodeWithEnvVarConfig): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_no_in_config_sql, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "models": { + "test": { + "+materialized": "{{ env_var('DBT_TEST_STATE_MODIFIED') }}", + } + } + } + + +class TestModelNodeWithEnvVarConfigInProjectYmlAndSchemaYml(BaseNodeWithEnvVarConfig): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_no_in_config_sql, + "schema.yml": schema_model_with_env_var_in_config_yml, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "models": { + "test": { + "+materialized": "{{ env_var('DBT_TEST_STATE_MODIFIED') }}", + } + } + } + + +# class TestModelNodeWithEnvVarConfigInSqlAndSchemaYml(BaseNodeWithEnvVarConfig): +# @pytest.fixture(scope="class") +# def models(self): +# return { +# "model.sql": model_with_env_var_in_config_sql, +# "schema.yml": schema_model_with_env_var_in_config_yml +# } + + +# class TestSourceNodeWithEnvVarConfigInSchema(BaseNodeWithEnvVarConfig): +# @pytest.fixture(scope="class") +# def models(self): +# return { +# "schema.yml": schema_source_with_env_var_as_property_yml, +# "model.sql": "select * from {{ source('jaffle_shop', 'customers') }}" +# } From 8ad3c0395d953c23b71d84d0730e53c23a0597bd Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 25 Jul 2024 21:48:18 -0400 Subject: [PATCH 02/23] persist unrendered config on schema file instead of manifest --- core/dbt/contracts/files.py | 7 +++++++ core/dbt/contracts/graph/manifest.py | 1 - core/dbt/parser/base.py | 18 +++++++++++------- core/dbt/parser/schemas.py | 8 ++++++-- .../test_modified_state_environment_aware.py | 4 ++-- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index 793451c0b00..b79a09efe8c 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -212,6 +212,7 @@ class SchemaSourceFile(BaseSourceFile): # created too, but those are in 'sources' sop: List[SourceKey] = field(default_factory=list) env_vars: Dict[str, Any] = field(default_factory=dict) + unrendered_configs: Dict[str, Any] = field(default_factory=dict) pp_dict: Optional[Dict[str, Any]] = None pp_test_index: Optional[Dict[str, Any]] = None @@ -317,6 +318,12 @@ def get_all_test_ids(self): test_ids.extend(self.data_tests[key][name]) return test_ids + def add_unrendered_config(self, unrendered_config, yaml_key, name): + if yaml_key not in self.unrendered_configs: + self.unrendered_configs[yaml_key] = {} + if name not in self.unrendered_configs[yaml_key]: + self.unrendered_configs[yaml_key][name] = unrendered_config + def add_env_var(self, var, yaml_key, name): if yaml_key not in self.env_vars: self.env_vars[yaml_key] = {} diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index ab3a4e5b9c6..21c5571b74b 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -827,7 +827,6 @@ class Manifest(MacroMethods, dbtClassMixin): unit_tests: MutableMapping[str, UnitTestDefinition] = field(default_factory=dict) saved_queries: MutableMapping[str, SavedQuery] = field(default_factory=dict) fixtures: MutableMapping[str, UnitTestFileFixture] = field(default_factory=dict) - unrendered_patch_configs: MutableMapping[str, Dict[str, Any]] = field(default_factory=dict) _doc_lookup: Optional[DocLookup] = field( default=None, metadata={"serialize": lambda x: None, "deserialize": lambda x: None} diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index b619de73aae..7ace0e416b5 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -13,6 +13,7 @@ generate_generate_name_macro_context, generate_parser_model_context, ) +from dbt.contracts.files import SchemaSourceFile from dbt.contracts.graph.manifest import Manifest from dbt.contracts.graph.nodes import BaseNode, ManifestNode from dbt.contracts.graph.unparsed import Docs, UnparsedNode @@ -308,7 +309,7 @@ def update_parsed_node_config( config: ContextConfig, context=None, patch_config_dict=None, - patch_original_file_path=None, + patch_file_id=None, ) -> None: """Given the ContextConfig used for parsing and the parsed node, generate and set the true values to use, overriding the temporary parse @@ -372,12 +373,15 @@ def update_parsed_node_config( # unrendered_config is used to compare the original database/schema/alias # values and to handle 'same_config' and 'same_contents' calls - if patch_original_file_path: - # Use the unrendered_patch_configs if available, provided patch_config_dict may actuallly already be rendered - if unrendered_patch_config_dict := self.manifest.unrendered_patch_configs.get( - patch_original_file_path - ): - patch_config_dict = unrendered_patch_config_dict + if patch_file_id: + # Use the patch_file.unrendered_configs if available, as provided patch_config_dict may actuallly already be rendered + if patch_file := self.manifest.files.get(patch_file_id, None): + if isinstance(patch_file, SchemaSourceFile): + # TODO: do not hardcode "models" + if unrendered_patch_config := patch_file.unrendered_configs.get( + "models", {} + ).get(parsed_node.name): + patch_config_dict = unrendered_patch_config parsed_node.unrendered_config = config.build_config_dict( rendered=False, patch_config_dict=patch_config_dict diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 70f2087fbc6..9d4d8d1012a 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -349,7 +349,9 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: # TODO: consider not storing if rendered config == unrendered_config if unrendered_config: - self.schema_parser.manifest.unrendered_patch_configs[path] = unrendered_config + schema_file = self.yaml.file + assert isinstance(schema_file, SchemaSourceFile) + schema_file.add_unrendered_config(unrendered_config, self.key, entry["name"]) if self.schema_yaml_vars.env_vars: self.schema_parser.manifest.env_vars.update(self.schema_yaml_vars.env_vars) @@ -412,6 +414,8 @@ def parse(self) -> ParseResult: self.manifest.source_patches[key] = patch source_file.source_patches.append(key) else: + # TODO: add unrendered_database from manifest.unrendered_source_patch + # self.yaml.path.original_file_path source = self._target_from_dict(UnparsedSourceDefinition, data) self.add_source_definitions(source) return ParseResult() @@ -621,7 +625,7 @@ def patch_node_config(self, node, patch) -> None: node, config, patch_config_dict=patch.config, - patch_original_file_path=patch.original_file_path, + patch_file_id=patch.file_id, ) diff --git a/tests/functional/defer_state/test_modified_state_environment_aware.py b/tests/functional/defer_state/test_modified_state_environment_aware.py index 9fc2168d5d6..c038e8ad65e 100644 --- a/tests/functional/defer_state/test_modified_state_environment_aware.py +++ b/tests/functional/defer_state/test_modified_state_environment_aware.py @@ -3,7 +3,7 @@ import pytest from dbt.tests.util import run_dbt -from tests.functional.defer_state.fixtures import ( # model_with_env_var_in_config_sql,; schema_source_with_env_var_as_property_yml, +from tests.functional.defer_state.fixtures import ( # model_with_env_var_in_config_sql,;; schema_source_with_env_var_as_property_yml, model_with_no_in_config_sql, schema_model_with_env_var_in_config_yml, ) @@ -105,5 +105,5 @@ def project_config_update(self): # def models(self): # return { # "schema.yml": schema_source_with_env_var_as_property_yml, -# "model.sql": "select * from {{ source('jaffle_shop', 'customers') }}" +# # "model.sql": "select * from {{ source('jaffle_shop', 'customers') }}" # } From 4f96f5ccc5e93b93f35c963e9f3f7042c6553091 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Fri, 26 Jul 2024 10:25:40 -0400 Subject: [PATCH 03/23] update test_configs_in_schema_files.py --- tests/functional/configs/test_configs_in_schema_files.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/configs/test_configs_in_schema_files.py b/tests/functional/configs/test_configs_in_schema_files.py index e0b85686895..bfed83e2705 100644 --- a/tests/functional/configs/test_configs_in_schema_files.py +++ b/tests/functional/configs/test_configs_in_schema_files.py @@ -221,7 +221,7 @@ def test_config_layering( assert model_node.config.materialized == "view" model_unrendered_config = { "materialized": "view", - "meta": {"my_attr": "TESTING", "owner": "Julie Smith"}, + "meta": {"my_attr": "{{ var('my_var') }}", "owner": "Julie Smith"}, "tags": ["tag_1_in_model", "tag_2_in_model"], } assert model_node.unrendered_config == model_unrendered_config From acd23b0fc097351bde9cdac8c82094ec309f7c0d Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Fri, 26 Jul 2024 19:51:11 -0400 Subject: [PATCH 04/23] support versioning in SchemaSourceFile.add_unrendered_config --- core/dbt/contracts/files.py | 18 ++++++++++++++++-- core/dbt/parser/base.py | 9 +++++---- core/dbt/parser/schemas.py | 20 +++++++++++++++----- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index b79a09efe8c..789b0384ac2 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -318,11 +318,25 @@ def get_all_test_ids(self): test_ids.extend(self.data_tests[key][name]) return test_ids - def add_unrendered_config(self, unrendered_config, yaml_key, name): + def add_unrendered_config(self, unrendered_config, yaml_key, name, version=None): if yaml_key not in self.unrendered_configs: self.unrendered_configs[yaml_key] = {} + if name not in self.unrendered_configs[yaml_key]: - self.unrendered_configs[yaml_key][name] = unrendered_config + self.unrendered_configs[yaml_key][name] = {} + + if version not in self.unrendered_configs[yaml_key][name]: + self.unrendered_configs[yaml_key][name][version] = unrendered_config + + def get_unrendered_config(self, yaml_key, name, version=None) -> Optional[Dict[str, Any]]: + if yaml_key not in self.unrendered_configs: + return None + if name not in self.unrendered_configs[yaml_key]: + return None + + unrendered_config = self.unrendered_configs[yaml_key][name].get(version) + + return unrendered_config def add_env_var(self, var, yaml_key, name): if yaml_key not in self.env_vars: diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 7ace0e416b5..661d77835d8 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -26,6 +26,7 @@ from dbt.node_types import AccessType, ModelLanguage, NodeType from dbt.parser.search import FileBlock from dbt_common.dataclass_schema import ValidationError +from dbt_common.utils import deep_merge # internally, the parser may store a less-restrictive type that will be # transformed into the final type. But it will have to be derived from @@ -378,10 +379,10 @@ def update_parsed_node_config( if patch_file := self.manifest.files.get(patch_file_id, None): if isinstance(patch_file, SchemaSourceFile): # TODO: do not hardcode "models" - if unrendered_patch_config := patch_file.unrendered_configs.get( - "models", {} - ).get(parsed_node.name): - patch_config_dict = unrendered_patch_config + if unrendered_patch_config := patch_file.get_unrendered_config( + "models", parsed_node.name, getattr(parsed_node, "version", None) + ): + patch_config_dict = deep_merge(patch_config_dict, unrendered_patch_config) parsed_node.unrendered_config = config.build_config_dict( rendered=False, patch_config_dict=patch_config_dict diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 9d4d8d1012a..64c6c00988f 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -339,24 +339,34 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: if "name" not in entry and "model" not in entry: raise ParsingError("Entry did not contain a name") - unrendered_config = None + unrendered_config = {} if "config" in entry: unrendered_config = entry["config"] + unrendered_version_configs = {} + if "versions" in entry: + for version in entry["versions"]: + if "v" in version: + unrendered_version_configs[version["v"]] = version.get("config", {}) + # Render the data (except for tests, data_tests and descriptions). # See the SchemaYamlRenderer entry = self.render_entry(entry) # TODO: consider not storing if rendered config == unrendered_config + schema_file = self.yaml.file + assert isinstance(schema_file, SchemaSourceFile) + if unrendered_config: - schema_file = self.yaml.file - assert isinstance(schema_file, SchemaSourceFile) schema_file.add_unrendered_config(unrendered_config, self.key, entry["name"]) + for version, unrendered_version_config in unrendered_version_configs.items(): + schema_file.add_unrendered_config( + unrendered_version_config, self.key, entry["name"], version + ) + if self.schema_yaml_vars.env_vars: self.schema_parser.manifest.env_vars.update(self.schema_yaml_vars.env_vars) - schema_file = self.yaml.file - assert isinstance(schema_file, SchemaSourceFile) for var in self.schema_yaml_vars.env_vars.keys(): schema_file.add_env_var(var, self.key, entry["name"]) self.schema_yaml_vars.env_vars = {} From fe9b57206f55a5e2001946244e5438535ba85358 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Sun, 11 Aug 2024 18:24:03 -0700 Subject: [PATCH 05/23] fix: do not use None for version key in unrendered_configs --- core/dbt/contracts/files.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index 789b0384ac2..38f858b58d6 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -319,24 +319,23 @@ def get_all_test_ids(self): return test_ids def add_unrendered_config(self, unrendered_config, yaml_key, name, version=None): + versioned_name = f"{name}_v{version}" if version is not None else name + if yaml_key not in self.unrendered_configs: self.unrendered_configs[yaml_key] = {} - if name not in self.unrendered_configs[yaml_key]: - self.unrendered_configs[yaml_key][name] = {} - - if version not in self.unrendered_configs[yaml_key][name]: - self.unrendered_configs[yaml_key][name][version] = unrendered_config + if versioned_name not in self.unrendered_configs[yaml_key]: + self.unrendered_configs[yaml_key][versioned_name] = unrendered_config def get_unrendered_config(self, yaml_key, name, version=None) -> Optional[Dict[str, Any]]: + versioned_name = f"{name}_v{version}" if version is not None else name + if yaml_key not in self.unrendered_configs: return None - if name not in self.unrendered_configs[yaml_key]: + if versioned_name not in self.unrendered_configs[yaml_key]: return None - unrendered_config = self.unrendered_configs[yaml_key][name].get(version) - - return unrendered_config + return self.unrendered_configs[yaml_key][versioned_name] def add_env_var(self, var, yaml_key, name): if yaml_key not in self.env_vars: From f368cdf50349e13a335302cc7d2b0653a7993d5d Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 12 Aug 2024 00:25:29 -0700 Subject: [PATCH 06/23] support reliable unrendered_config in .sql files --- core/dbt/clients/jinja_static.py | 87 +++++++++++++++++++ core/dbt/context/context_config.py | 11 ++- core/dbt/context/providers.py | 9 +- core/dbt/parser/base.py | 1 + ...> test_modified_state_environment_vars.py} | 45 +++++----- tests/unit/clients/test_jinja_static.py | 26 ++++++ 6 files changed, 152 insertions(+), 27 deletions(-) rename tests/functional/defer_state/{test_modified_state_environment_aware.py => test_modified_state_environment_vars.py} (69%) diff --git a/core/dbt/clients/jinja_static.py b/core/dbt/clients/jinja_static.py index d8746a7607d..1dc2c7d48e2 100644 --- a/core/dbt/clients/jinja_static.py +++ b/core/dbt/clients/jinja_static.py @@ -191,3 +191,90 @@ def statically_parse_ref_or_source(expression: str) -> Union[RefArgs, List[str]] raise ParsingError(f"Invalid ref or source expression: {expression}") return ref_or_source + + +def statically_parse_unrendered_config(string: str) -> Optional[Dict[str, Any]]: + """ + Given a string with jinja, extract an unrendered config call. + If no config call is present, returns None. + + For example, given: + "{{ config(materialized=env_var('DBT_TEST_STATE_MODIFIED')) }}\nselect 1 as id" + returns: {"materialized" : "env_var('DBT_TEST_STATE_MODIFIED')"} + + No config call: + "select 1 as id" + returns: None + """ + # set 'capture_macros' to capture undefined + env = get_environment(None, capture_macros=True) + + global _TESTING_MACRO_CACHE + if test_caching_enabled() and _TESTING_MACRO_CACHE and string in _TESTING_MACRO_CACHE: + parsed = _TESTING_MACRO_CACHE.get(string, None) + func_calls = getattr(parsed, "_dbt_cached_calls") + else: + parsed = env.parse(string) + func_calls = tuple(parsed.find_all(jinja2.nodes.Call)) + + config_func_calls = list( + filter( + lambda f: hasattr(f, "node") and hasattr(f.node, "name") and f.node.name == "config", + func_calls, + ) + ) + # There should only be one {{ config(...) }} call per input + config_func_call = config_func_calls[0] if config_func_calls else None + + if not config_func_call: + return None + + unrendered_config = {} + for kwarg in config_func_call.kwargs: + unrendered_config[kwarg.key] = construct_static_kwarg_value(kwarg) + + return unrendered_config + + +def construct_static_kwarg_value(kwarg): + static_kwarg_value = _construct_static_kwarg_value(kwarg) + if isinstance(static_kwarg_value, str): + return static_kwarg_value.strip("'") + else: + return static_kwarg_value + + +def _construct_static_kwarg_value(kwarg) -> str: + kwarg_type = type(kwarg.value).__name__ + if kwarg_type == "Const": + return ( + f"'{kwarg.value.value}'" if isinstance(kwarg.value.value, str) else kwarg.value.value + ) + + elif kwarg_type == "Call": + unrendered_args = [] + for arg in kwarg.value.args: + arg_type = type(arg).__name__ + if arg_type == "Const": + unrendered_args.append( + f"'{arg.value}'" if isinstance(arg.value, str) else arg.value + ) + + unrendered_kwargs = {} + for call_kwarg in kwarg.value.kwargs: + kwarg_value = _construct_static_kwarg_value(call_kwarg) + + unrendered_kwargs[call_kwarg.key] = kwarg_value + + formatted_unrendered_kwargs = [ + f"{key}={value}" for key, value in unrendered_kwargs.items() + ] + + formatted_all_args = ", ".join( + str(x) for x in (unrendered_args + formatted_unrendered_kwargs) + ) + + return f"{kwarg.value.node.name}({formatted_all_args})" + + else: + return "" diff --git a/core/dbt/context/context_config.py b/core/dbt/context/context_config.py index 51222ceba10..60c5d32c6da 100644 --- a/core/dbt/context/context_config.py +++ b/core/dbt/context/context_config.py @@ -286,6 +286,7 @@ def __init__( project_name: str, ) -> None: self._config_call_dict: Dict[str, Any] = {} + self._unrendered_config_call_dict: Dict[str, Any] = {} self._active_project = active_project self._fqn = fqn self._resource_type = resource_type @@ -295,6 +296,10 @@ def add_config_call(self, opts: Dict[str, Any]) -> None: dct = self._config_call_dict self._add_config_call(dct, opts) + def add_unrendered_config_call(self, opts: Dict[str, Any]) -> None: + # Cannot perform complex merge behaviours on unrendered configs as they may not be appropriate types. + self._unrendered_config_call_dict.update(opts) + @classmethod def _add_config_call(cls, config_call_dict, opts: Dict[str, Any]) -> None: # config_call_dict is already encountered configs, opts is new @@ -353,12 +358,16 @@ def build_config_dict( if rendered: # TODO CT-211 src = ContextConfigGenerator(self._active_project) # type: ignore[var-annotated] + config_call_dict = self._config_call_dict else: # TODO CT-211 src = UnrenderedConfigGenerator(self._active_project) # type: ignore[assignment] + # TODO: behaviour flag can route behaviour here. + # TODO: consider using self._config_call_dict for python models as _unrendered_config_call_dict is unreliable + config_call_dict = self._unrendered_config_call_dict return src.calculate_node_config_dict( - config_call_dict=self._config_call_dict, + config_call_dict=config_call_dict, fqn=self._fqn, resource_type=self._resource_type, project_name=self._project_name, diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index c68732f83ad..1f495da20eb 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -34,6 +34,7 @@ UnitTestMacroGenerator, get_rendered, ) +from dbt.clients.jinja_static import statically_parse_unrendered_config from dbt.config import IsFQNResource, Project, RuntimeConfig from dbt.constants import DEFAULT_ENV_PLACEHOLDER from dbt.context.base import Var, contextmember, contextproperty @@ -372,8 +373,12 @@ def __call__(self, *args, **kwargs): if self.context_config is None: raise DbtRuntimeError("At parse time, did not receive a context config") - # TODO: consider skipping this for sql state:modified configs. args rendered so this will produced a rendered config unless - # statically extracted + # Track unrendered opts to build parsed node unrendered_config later on + unrendered_config = statically_parse_unrendered_config(self.model.raw_code) + if unrendered_config: + self.context_config.add_unrendered_config_call(unrendered_config) + + # Use rendered opts to populate context_config self.context_config.add_config_call(opts) return "" diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 661d77835d8..ffdb4de7ea8 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -374,6 +374,7 @@ def update_parsed_node_config( # unrendered_config is used to compare the original database/schema/alias # values and to handle 'same_config' and 'same_contents' calls + # TODO: Behaviour flag can route here if patch_file_id: # Use the patch_file.unrendered_configs if available, as provided patch_config_dict may actuallly already be rendered if patch_file := self.manifest.files.get(patch_file_id, None): diff --git a/tests/functional/defer_state/test_modified_state_environment_aware.py b/tests/functional/defer_state/test_modified_state_environment_vars.py similarity index 69% rename from tests/functional/defer_state/test_modified_state_environment_aware.py rename to tests/functional/defer_state/test_modified_state_environment_vars.py index c038e8ad65e..bf1bef91c42 100644 --- a/tests/functional/defer_state/test_modified_state_environment_aware.py +++ b/tests/functional/defer_state/test_modified_state_environment_vars.py @@ -3,18 +3,15 @@ import pytest from dbt.tests.util import run_dbt -from tests.functional.defer_state.fixtures import ( # model_with_env_var_in_config_sql,;; schema_source_with_env_var_as_property_yml, +from tests.functional.defer_state.fixtures import ( # schema_source_with_env_var_as_property_yml + model_with_env_var_in_config_sql, model_with_no_in_config_sql, schema_model_with_env_var_in_config_yml, ) from tests.functional.defer_state.test_modified_state import BaseModifiedState -class BaseNodeWithEnvVarConfig(BaseModifiedState): - @pytest.fixture(scope="class") - def seeds(self): - return {} - +class BaseTestStateSelectionEnvVarConfig(BaseModifiedState): @pytest.fixture(scope="class", autouse=True) def setup(self): os.environ["DBT_TEST_STATE_MODIFIED"] = "table" @@ -30,22 +27,22 @@ def test_change_env_var(self, project): results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) assert len(results) == 0 - # Change environment variable and assert no + # Change environment variable and assert no false positive # Environment variables do not have an effect on state:modified os.environ["DBT_TEST_STATE_MODIFIED"] = "view" results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) assert len(results) == 0 -# class TestModelNodeWithEnvVarConfigInSqlFile(BaseNodeWithEnvVarConfig): -# @pytest.fixture(scope="class") -# def models(self): -# return { -# "model.sql": model_with_env_var_in_config_sql, -# } +class TestModelNodeWithEnvVarConfigInSqlFile(BaseTestStateSelectionEnvVarConfig): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_env_var_in_config_sql, + } -class TestModelNodeWithEnvVarConfigInSchemaYml(BaseNodeWithEnvVarConfig): +class TestModelNodeWithEnvVarConfigInSchemaYml(BaseTestStateSelectionEnvVarConfig): @pytest.fixture(scope="class") def models(self): return { @@ -54,7 +51,7 @@ def models(self): } -class TestModelNodeWithEnvVarConfigInProjectYml(BaseNodeWithEnvVarConfig): +class TestModelNodeWithEnvVarConfigInProjectYml(BaseTestStateSelectionEnvVarConfig): @pytest.fixture(scope="class") def models(self): return { @@ -72,7 +69,7 @@ def project_config_update(self): } -class TestModelNodeWithEnvVarConfigInProjectYmlAndSchemaYml(BaseNodeWithEnvVarConfig): +class TestModelNodeWithEnvVarConfigInProjectYmlAndSchemaYml(BaseTestStateSelectionEnvVarConfig): @pytest.fixture(scope="class") def models(self): return { @@ -91,16 +88,16 @@ def project_config_update(self): } -# class TestModelNodeWithEnvVarConfigInSqlAndSchemaYml(BaseNodeWithEnvVarConfig): -# @pytest.fixture(scope="class") -# def models(self): -# return { -# "model.sql": model_with_env_var_in_config_sql, -# "schema.yml": schema_model_with_env_var_in_config_yml -# } +class TestModelNodeWithEnvVarConfigInSqlAndSchemaYml(BaseTestStateSelectionEnvVarConfig): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_env_var_in_config_sql, + "schema.yml": schema_model_with_env_var_in_config_yml, + } -# class TestSourceNodeWithEnvVarConfigInSchema(BaseNodeWithEnvVarConfig): +# class TestSourceNodeWithEnvVarConfigInSchema(BaseTestStateSelectionEnvVarConfig): # @pytest.fixture(scope="class") # def models(self): # return { diff --git a/tests/unit/clients/test_jinja_static.py b/tests/unit/clients/test_jinja_static.py index 171976a6b50..cb6888a20f6 100644 --- a/tests/unit/clients/test_jinja_static.py +++ b/tests/unit/clients/test_jinja_static.py @@ -4,6 +4,7 @@ from dbt.clients.jinja_static import ( statically_extract_macro_calls, statically_parse_ref_or_source, + statically_parse_unrendered_config, ) from dbt.context.base import generate_base_context from dbt.exceptions import ParsingError @@ -77,3 +78,28 @@ def test_invalid_expression(self): def test_valid_ref_expression(self, expression, expected_ref_or_source): ref_or_source = statically_parse_ref_or_source(expression) assert ref_or_source == expected_ref_or_source + + +class TestStaticallyParseUnrenderedConfig: + @pytest.mark.parametrize( + "expression,expected_unrendered_config", + [ + ("{{ config(materialized='view') }}", {"materialized": "view"}), + ( + "{{ config(materialized='view', enabled=True) }}", + {"materialized": "view", "enabled": True}, + ), + ("{{ config(materialized=env_var('test')) }}", {"materialized": "env_var('test')"}), + ( + "{{ config(materialized=env_var('test', default='default')) }}", + {"materialized": "env_var('test', default='default')"}, + ), + ( + "{{ config(materialized=env_var('test', default=env_var('default'))) }}", + {"materialized": "env_var('test', default=env_var('default'))"}, + ), + ], + ) + def test_statically_parse_unrendered_config(self, expression, expected_unrendered_config): + unrendered_config = statically_parse_unrendered_config(expression) + assert unrendered_config == expected_unrendered_config From 57db0bf529ac1d02fb20ece113ef41a0a7cf11a2 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 19 Aug 2024 19:33:32 -0400 Subject: [PATCH 07/23] more reliably set unrendered_config_call_dict + prefer config_call_dict if unavailable --- core/dbt/context/context_config.py | 8 ++++++-- core/dbt/parser/schemas.py | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/core/dbt/context/context_config.py b/core/dbt/context/context_config.py index 60c5d32c6da..e8b2e8a6b09 100644 --- a/core/dbt/context/context_config.py +++ b/core/dbt/context/context_config.py @@ -363,8 +363,12 @@ def build_config_dict( # TODO CT-211 src = UnrenderedConfigGenerator(self._active_project) # type: ignore[assignment] # TODO: behaviour flag can route behaviour here. - # TODO: consider using self._config_call_dict for python models as _unrendered_config_call_dict is unreliable - config_call_dict = self._unrendered_config_call_dict + # Prefer _config_call_dict if it is available and _unrendered_config_call_dict is not, + # as _unrendered_config_call_dict is unreliable for non-sql nodes (e.g. no jinja config block rendered for python models, test nodes, etc) + if self._config_call_dict and not self._unrendered_config_call_dict: + config_call_dict = self._config_call_dict + else: + config_call_dict = self._unrendered_config_call_dict return src.calculate_node_config_dict( config_call_dict=config_call_dict, diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 6756b661055..1a055dfb944 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -634,6 +634,7 @@ def patch_node_config(self, node, patch) -> None: ) # We need to re-apply the config_call_dict after the patch config config._config_call_dict = node.config_call_dict + config._unrendered_config_call_dict = node.unrendered_config self.schema_parser.update_parsed_node_config( node, config, From 1ce11ec11e203dfeaee7afb600d608612e74baf4 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Tue, 20 Aug 2024 10:17:35 -0400 Subject: [PATCH 08/23] store + reset unrendered_config_call_dict on node --- core/dbt/artifacts/resources/v1/components.py | 3 +++ core/dbt/parser/base.py | 1 + core/dbt/parser/models.py | 1 + core/dbt/parser/schemas.py | 2 +- 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/core/dbt/artifacts/resources/v1/components.py b/core/dbt/artifacts/resources/v1/components.py index fc6f44a38f0..8eb43f35d8e 100644 --- a/core/dbt/artifacts/resources/v1/components.py +++ b/core/dbt/artifacts/resources/v1/components.py @@ -194,6 +194,7 @@ class ParsedResource(ParsedResourceMandatory): unrendered_config: Dict[str, Any] = field(default_factory=dict) created_at: float = field(default_factory=lambda: time.time()) config_call_dict: Dict[str, Any] = field(default_factory=dict) + unrendered_config_call_dict: Dict[str, Any] = field(default_factory=dict) relation_name: Optional[str] = None raw_code: str = "" @@ -201,6 +202,8 @@ def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None): dct = super().__post_serialize__(dct, context) if context and context.get("artifact") and "config_call_dict" in dct: del dct["config_call_dict"] + if context and context.get("artifact") and "unrendered_config_call_dict" in dct: + del dct["unrendered_config_call_dict"] return dct diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index ffdb4de7ea8..d17be8f2ebe 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -390,6 +390,7 @@ def update_parsed_node_config( ) parsed_node.config_call_dict = config._config_call_dict + parsed_node.unrendered_config_call_dict = config._unrendered_config_call_dict # do this once before we parse the node database/schema/alias, so # parsed_node.config is what it would be if they did nothing diff --git a/core/dbt/parser/models.py b/core/dbt/parser/models.py index dd56d06868c..f00d78fba04 100644 --- a/core/dbt/parser/models.py +++ b/core/dbt/parser/models.py @@ -301,6 +301,7 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None: statically_parsed = self.run_experimental_parser(node) # run the stable static parser unless it is explicitly turned off else: + # TODO: consider running new parser here? statically_parsed = self.run_static_parser(node) # if the static parser succeeded, extract some data in easy-to-compare formats diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 1a055dfb944..73270ee7f57 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -634,7 +634,7 @@ def patch_node_config(self, node, patch) -> None: ) # We need to re-apply the config_call_dict after the patch config config._config_call_dict = node.config_call_dict - config._unrendered_config_call_dict = node.unrendered_config + config._unrendered_config_call_dict = node.unrendered_config_call_dict self.schema_parser.update_parsed_node_config( node, config, From bad6b920e9a096acd7fa7212d9aa3fe0bc8e9bbf Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 4 Sep 2024 09:43:22 -0400 Subject: [PATCH 09/23] first pass: put behind legacy flag --- core/dbt/context/context_config.py | 15 ++-- core/dbt/contracts/project.py | 4 +- core/dbt/parser/base.py | 16 +++-- .../configs/test_configs_in_schema_files.py | 70 +++++++++++++++++-- .../test_modified_state_environment_vars.py | 13 +++- tests/unit/contracts/graph/test_manifest.py | 1 + tests/unit/contracts/graph/test_nodes.py | 4 ++ .../unit/contracts/graph/test_nodes_parsed.py | 11 +++ tests/unit/parser/test_parser.py | 13 ++-- 9 files changed, 124 insertions(+), 23 deletions(-) diff --git a/core/dbt/context/context_config.py b/core/dbt/context/context_config.py index e8b2e8a6b09..b23efd37cdd 100644 --- a/core/dbt/context/context_config.py +++ b/core/dbt/context/context_config.py @@ -6,6 +6,7 @@ from dbt.adapters.factory import get_config_class_by_name from dbt.config import IsFQNResource, Project, RuntimeConfig from dbt.contracts.graph.model_config import get_config_for +from dbt.flags import get_flags from dbt.node_types import NodeType from dbt.utils import fqn_search from dbt_common.contracts.config.base import BaseConfig, _listify @@ -362,13 +363,17 @@ def build_config_dict( else: # TODO CT-211 src = UnrenderedConfigGenerator(self._active_project) # type: ignore[assignment] - # TODO: behaviour flag can route behaviour here. - # Prefer _config_call_dict if it is available and _unrendered_config_call_dict is not, - # as _unrendered_config_call_dict is unreliable for non-sql nodes (e.g. no jinja config block rendered for python models, test nodes, etc) - if self._config_call_dict and not self._unrendered_config_call_dict: + + # preserve legacy behaviour - using unreliable (potentially rendered) _config_call_dict + if get_flags().require_config_jinja_insensitivity_for_state_modified is False: config_call_dict = self._config_call_dict else: - config_call_dict = self._unrendered_config_call_dict + # Prefer _config_call_dict if it is available and _unrendered_config_call_dict is not, + # as _unrendered_config_call_dict is unreliable for non-sql nodes (e.g. no jinja config block rendered for python models, etc) + if self._config_call_dict and not self._unrendered_config_call_dict: + config_call_dict = self._config_call_dict + else: + config_call_dict = self._unrendered_config_call_dict return src.calculate_node_config_dict( config_call_dict=config_call_dict, diff --git a/core/dbt/contracts/project.py b/core/dbt/contracts/project.py index b0b7179f333..d05092b28ab 100644 --- a/core/dbt/contracts/project.py +++ b/core/dbt/contracts/project.py @@ -341,7 +341,8 @@ class ProjectFlags(ExtensibleDbtClassMixin): warn_error_options: Optional[Dict[str, Union[str, List[str]]]] = None write_json: Optional[bool] = None - # legacy behaviors + # legacy behaviors - https://github.com/dbt-labs/dbt-core/blob/main/docs/guides/behavior-change-flags.md + require_config_jinja_insensitivity_for_state_modified: bool = False require_explicit_package_overrides_for_builtin_materializations: bool = True require_resource_names_without_spaces: bool = False source_freshness_run_project_hooks: bool = False @@ -349,6 +350,7 @@ class ProjectFlags(ExtensibleDbtClassMixin): @property def project_only_flags(self) -> Dict[str, Any]: return { + "require_config_jinja_insensitivity_for_state_modified": self.require_config_jinja_insensitivity_for_state_modified, "require_explicit_package_overrides_for_builtin_materializations": self.require_explicit_package_overrides_for_builtin_materializations, "require_resource_names_without_spaces": self.require_resource_names_without_spaces, "source_freshness_run_project_hooks": self.source_freshness_run_project_hooks, diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index d17be8f2ebe..6d54da28c90 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -23,6 +23,7 @@ DictParseError, InvalidAccessTypeError, ) +from dbt.flags import get_flags from dbt.node_types import AccessType, ModelLanguage, NodeType from dbt.parser.search import FileBlock from dbt_common.dataclass_schema import ValidationError @@ -372,19 +373,20 @@ def update_parsed_node_config( if hasattr(parsed_node, "contract"): parsed_node.contract = Contract.from_dict(contract_dct) - # unrendered_config is used to compare the original database/schema/alias - # values and to handle 'same_config' and 'same_contents' calls - # TODO: Behaviour flag can route here - if patch_file_id: - # Use the patch_file.unrendered_configs if available, as provided patch_config_dict may actuallly already be rendered - if patch_file := self.manifest.files.get(patch_file_id, None): - if isinstance(patch_file, SchemaSourceFile): + if get_flags().require_config_jinja_insensitivity_for_state_modified: + # Use the patch_file.unrendered_configs if available to update patch_dict_config, + # as provided patch_config_dict may actuallly already be rendered and thus sensitive to jinja evaluations + if patch_file_id: + patch_file = self.manifest.files.get(patch_file_id, None) + if patch_file and isinstance(patch_file, SchemaSourceFile): # TODO: do not hardcode "models" if unrendered_patch_config := patch_file.get_unrendered_config( "models", parsed_node.name, getattr(parsed_node, "version", None) ): patch_config_dict = deep_merge(patch_config_dict, unrendered_patch_config) + # unrendered_config is used to compare the original database/schema/alias + # values and to handle 'same_config' and 'same_contents' calls parsed_node.unrendered_config = config.build_config_dict( rendered=False, patch_config_dict=patch_config_dict ) diff --git a/tests/functional/configs/test_configs_in_schema_files.py b/tests/functional/configs/test_configs_in_schema_files.py index bfed83e2705..d5e0e68ec51 100644 --- a/tests/functional/configs/test_configs_in_schema_files.py +++ b/tests/functional/configs/test_configs_in_schema_files.py @@ -110,6 +110,15 @@ class TestSchemaFileConfigs: + @pytest.fixture(scope="class") + def expected_unrendered_config(self): + # my_attr is unrendered when require_config_jinja_insensitivity_for_state_modified: True + return { + "materialized": "view", + "meta": {"my_attr": "{{ var('my_var') }}", "owner": "Julie Smith"}, + "tags": ["tag_1_in_model", "tag_2_in_model"], + } + @pytest.fixture(scope="class") def models(self): return { @@ -125,6 +134,9 @@ def seeds(self): @pytest.fixture(scope="class") def project_config_update(self): return { + "flags": { + "require_config_jinja_insensitivity_for_state_modified": True, + }, "models": { "+meta": { "company": "NuMade", @@ -160,6 +172,7 @@ def project_config_update(self): def test_config_layering( self, project, + expected_unrendered_config, ): # run seed @@ -219,11 +232,7 @@ def test_config_layering( model_node = manifest.nodes[model_id] assert model_node.config.materialized == "view" - model_unrendered_config = { - "materialized": "view", - "meta": {"my_attr": "{{ var('my_var') }}", "owner": "Julie Smith"}, - "tags": ["tag_1_in_model", "tag_2_in_model"], - } + model_unrendered_config = expected_unrendered_config assert model_node.unrendered_config == model_unrendered_config # look for test meta @@ -251,6 +260,57 @@ def test_config_layering( run_dbt(["run"]) +class TestLegacySchemaFileConfigs(TestSchemaFileConfigs): + @pytest.fixture(scope="class") + def expected_unrendered_config(self): + # my_attr is rendered ("TESTING") when require_config_jinja_insensitivity_for_state_modified: False + return { + "materialized": "view", + "meta": {"my_attr": "TESTING", "owner": "Julie Smith"}, + "tags": ["tag_1_in_model", "tag_2_in_model"], + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + # The uncommented below lines can be removed once the default behaviour is flipped. + # require_config_jinja_insensitivity_for_state_modified defaults to false currently + # "flags": { + # "require_config_jinja_insensitivity_for_state_modified": False, + # }, + "models": { + "+meta": { + "company": "NuMade", + }, + "test": { + "+meta": { + "project": "test", + }, + "tagged": { + "+meta": { + "team": "Core Team", + }, + "tags": ["tag_in_project"], + "model": { + "materialized": "table", + "+meta": { + "owner": "Julie Dent", + }, + }, + }, + }, + }, + "vars": { + "test": { + "my_var": "TESTING", + } + }, + "seeds": { + "quote_columns": False, + }, + } + + list_schema_yml = """ - name: my_name - name: alt_name diff --git a/tests/functional/defer_state/test_modified_state_environment_vars.py b/tests/functional/defer_state/test_modified_state_environment_vars.py index bf1bef91c42..7a80a154741 100644 --- a/tests/functional/defer_state/test_modified_state_environment_vars.py +++ b/tests/functional/defer_state/test_modified_state_environment_vars.py @@ -18,6 +18,14 @@ def setup(self): yield del os.environ["DBT_TEST_STATE_MODIFIED"] + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": { + "require_config_jinja_insensitivity_for_state_modified": True, + } + } + def test_change_env_var(self, project): # Generate ./state without changing environment variable value run_dbt(["run"]) @@ -80,11 +88,14 @@ def models(self): @pytest.fixture(scope="class") def project_config_update(self): return { + "flags": { + "require_config_jinja_insensitivity_for_state_modified": True, + }, "models": { "test": { "+materialized": "{{ env_var('DBT_TEST_STATE_MODIFIED') }}", } - } + }, } diff --git a/tests/unit/contracts/graph/test_manifest.py b/tests/unit/contracts/graph/test_manifest.py index d8d1df0d900..0f3a80e5039 100644 --- a/tests/unit/contracts/graph/test_manifest.py +++ b/tests/unit/contracts/graph/test_manifest.py @@ -84,6 +84,7 @@ "docs", "checksum", "unrendered_config", + "unrendered_config_call_dict", "created_at", "config_call_dict", "relation_name", diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index a498b99dcbc..6493c6eb520 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -139,6 +139,7 @@ def basic_uncompiled_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -197,6 +198,7 @@ def basic_compiled_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, "access": "protected", "constraints": [], @@ -458,6 +460,7 @@ def basic_uncompiled_schema_test_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -515,6 +518,7 @@ def basic_compiled_schema_test_dict(): "unrendered_config": { "severity": "warn", }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index ebbe2443771..772e97100f7 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -197,6 +197,7 @@ def base_parsed_model_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, "access": AccessType.Protected.value, "constraints": [], @@ -318,6 +319,7 @@ def complex_parsed_model_dict(): "materialized": "ephemeral", "post_hook": ['insert into blah(a, b) select "1", 1'], }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, "access": AccessType.Protected.value, "constraints": [], @@ -526,6 +528,7 @@ def basic_parsed_seed_dict(): "meta": {}, "checksum": {"name": "path", "checksum": "seeds/seed.csv"}, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -630,6 +633,7 @@ def complex_parsed_seed_dict(): "unrendered_config": { "persist_docs": {"relation": True, "columns": True}, }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -828,6 +832,7 @@ def base_parsed_hook_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -920,6 +925,7 @@ def complex_parsed_hook_dict(): "column_types": {"a": "text"}, "materialized": "table", }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -1013,6 +1019,7 @@ def minimal_parsed_schema_test_dict(): "name": "sha256", "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -1063,6 +1070,7 @@ def basic_parsed_schema_test_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -1151,6 +1159,7 @@ def complex_parsed_schema_test_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {"materialized": "table", "severity": "WARN"}, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -1535,6 +1544,7 @@ def basic_timestamp_snapshot_dict(): "target_database": "some_snapshot_db", "target_schema": "some_snapshot_schema", }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } @@ -1637,6 +1647,7 @@ def basic_check_snapshot_dict(): "strategy": "check", "check_cols": "all", }, + "unrendered_config_call_dict": {}, "config_call_dict": {}, } diff --git a/tests/unit/parser/test_parser.py b/tests/unit/parser/test_parser.py index 20a2c9e8c83..d1ab174243d 100644 --- a/tests/unit/parser/test_parser.py +++ b/tests/unit/parser/test_parser.py @@ -6,8 +6,6 @@ import yaml -import dbt.flags -import dbt.parser from dbt import tracking from dbt.artifacts.resources import ModelConfig, RefArgs from dbt.context.context_config import ContextConfig @@ -60,7 +58,9 @@ normalize, ) -set_from_args(Namespace(WARN_ERROR=False), None) +set_from_args( + Namespace(warn_error=False, require_config_jinja_insensitivity_for_state_modified=False), None +) def get_abs_os_path(unix_path): @@ -94,7 +94,12 @@ def _generate_macros(self): yield pm def setUp(self): - dbt.flags.WARN_ERROR = True + set_from_args( + Namespace( + warn_error=True, require_config_jinja_insensitivity_for_state_modified=False + ), + None, + ) # HACK: this is needed since tracking events can # be sent when using the model parser tracking.do_not_track() From d8bf0e6dc64cc6828f557c129aa7b781394eb362 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 5 Sep 2024 19:57:49 -0400 Subject: [PATCH 10/23] fix SnapshotParserTest unit test --- tests/unit/parser/test_parser.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/parser/test_parser.py b/tests/unit/parser/test_parser.py index d1ab174243d..1b1cbe6bf2e 100644 --- a/tests/unit/parser/test_parser.py +++ b/tests/unit/parser/test_parser.py @@ -1476,6 +1476,13 @@ def test_single_block(self): "unique_key": "id", "updated_at": "last_update", }, + unrendered_config_call_dict={ + "strategy": "timestamp", + "target_database": "dbt", + "target_schema": "analytics", + "unique_key": "id", + "updated_at": "last_update", + }, ) assertEqualNodes(expected, node) file_id = "snowplow://" + normalize("snapshots/nested/snap_1.sql") @@ -1546,6 +1553,13 @@ def test_multi_block(self): "unique_key": "id", "updated_at": "last_update", }, + unrendered_config_call_dict={ + "strategy": "timestamp", + "target_database": "dbt", + "target_schema": "analytics", + "unique_key": "id", + "updated_at": "last_update", + }, ) expect_bar = SnapshotNode( alias="bar", @@ -1583,6 +1597,13 @@ def test_multi_block(self): "unique_key": "id", "updated_at": "last_update", }, + unrendered_config_call_dict={ + "strategy": "timestamp", + "target_database": "dbt", + "target_schema": "analytics", + "unique_key": "id", + "updated_at": "last_update", + }, ) assertEqualNodes(nodes[0], expect_bar) assertEqualNodes(nodes[1], expect_foo) From fad129564938c4caed213c74b615073ac4492483 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 5 Sep 2024 20:13:54 -0400 Subject: [PATCH 11/23] Return early to avoid creating jinja environemt if no config call in input string --- core/dbt/clients/jinja_static.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/dbt/clients/jinja_static.py b/core/dbt/clients/jinja_static.py index 1dc2c7d48e2..d1738a7e26f 100644 --- a/core/dbt/clients/jinja_static.py +++ b/core/dbt/clients/jinja_static.py @@ -206,6 +206,10 @@ def statically_parse_unrendered_config(string: str) -> Optional[Dict[str, Any]]: "select 1 as id" returns: None """ + # Return early to avoid creating jinja environemt if no config call in input string + if "config(" not in string: + return None + # set 'capture_macros' to capture undefined env = get_environment(None, capture_macros=True) From 47b18b13af629af25153900d5aa97b719176ef05 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 5 Sep 2024 20:14:25 -0400 Subject: [PATCH 12/23] test var sensitivity with require_config_jinja_insensitivity_for_state_modified --- tests/functional/defer_state/fixtures.py | 6 + .../defer_state/test_modified_state_vars.py | 103 ++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 tests/functional/defer_state/test_modified_state_vars.py diff --git a/tests/functional/defer_state/fixtures.py b/tests/functional/defer_state/fixtures.py index a2b721c05c1..4d830cca562 100644 --- a/tests/functional/defer_state/fixtures.py +++ b/tests/functional/defer_state/fixtures.py @@ -567,3 +567,9 @@ tables: - name: customers """ + + +model_with_var_in_config_sql = """ +{{ config(materialized=var('DBT_TEST_STATE_MODIFIED')) }} +select 1 as id +""" diff --git a/tests/functional/defer_state/test_modified_state_vars.py b/tests/functional/defer_state/test_modified_state_vars.py new file mode 100644 index 00000000000..353e9cd92cd --- /dev/null +++ b/tests/functional/defer_state/test_modified_state_vars.py @@ -0,0 +1,103 @@ +import pytest + +from dbt.tests.util import run_dbt +from tests.functional.defer_state.fixtures import model_with_var_in_config_sql +from tests.functional.defer_state.test_modified_state import BaseModifiedState + + +class TestStateSelectionVarConfigLegacy(BaseModifiedState): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_var_in_config_sql, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": { + "require_config_jinja_insensitivity_for_state_modified": False, + } + } + + def test_change_var(self, project): + # Generate ./state without changing variable value + run_dbt(["run", "--vars", "DBT_TEST_STATE_MODIFIED: view"]) + self.copy_state() + + # Assert no false positive + results = run_dbt( + [ + "list", + "-s", + "state:modified", + "--state", + "./state", + "--vars", + "DBT_TEST_STATE_MODIFIED: view", + ] + ) + assert len(results) == 0 + + # Change var and assert no false negative - legacy behaviour + results = run_dbt( + [ + "list", + "-s", + "state:modified", + "--state", + "./state", + "--vars", + "DBT_TEST_STATE_MODIFIED: table", + ] + ) + assert len(results) == 1 + + +class TestStateSelectionVarConfig(BaseModifiedState): + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_var_in_config_sql, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": { + "require_config_jinja_insensitivity_for_state_modified": True, + } + } + + def test_change_var(self, project): + # Generate ./state without changing variable value + run_dbt(["run", "--vars", "DBT_TEST_STATE_MODIFIED: view"]) + self.copy_state() + + # Assert no false positive + results = run_dbt( + [ + "list", + "-s", + "state:modified", + "--state", + "./state", + "--vars", + "DBT_TEST_STATE_MODIFIED: view", + ] + ) + assert len(results) == 0 + + # Change var and assert no sensitivity to var changes -- new behaviour until state:modified.vars included in state:modified by default + results = run_dbt( + [ + "list", + "-s", + "state:modified", + "--state", + "./state", + "--vars", + "DBT_TEST_STATE_MODIFIED: table", + ] + ) + assert len(results) == 0 From d625a230f9580dd2258b9ef8e157cad176261a5b Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 5 Sep 2024 20:22:26 -0400 Subject: [PATCH 13/23] cleanup comment --- core/dbt/parser/schemas.py | 1 - 1 file changed, 1 deletion(-) diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 73270ee7f57..30424e5e265 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -356,7 +356,6 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: # See the SchemaYamlRenderer entry = self.render_entry(entry) - # TODO: consider not storing if rendered config == unrendered_config schema_file = self.yaml.file assert isinstance(schema_file, SchemaSourceFile) From 4081627dc4a4fda9b76e562e43bd2f844da1b4e8 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Tue, 10 Sep 2024 14:13:59 -0400 Subject: [PATCH 14/23] rename behaviour flag --- core/dbt/context/context_config.py | 2 +- core/dbt/contracts/project.py | 4 +- core/dbt/parser/base.py | 2 +- index.html | 75 +++++++++++++++++++ .../configs/test_configs_in_schema_files.py | 10 +-- .../test_modified_state_environment_vars.py | 4 +- .../defer_state/test_modified_state_vars.py | 4 +- tests/unit/parser/test_parser.py | 6 +- 8 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 index.html diff --git a/core/dbt/context/context_config.py b/core/dbt/context/context_config.py index b23efd37cdd..13fa4cd1e0c 100644 --- a/core/dbt/context/context_config.py +++ b/core/dbt/context/context_config.py @@ -365,7 +365,7 @@ def build_config_dict( src = UnrenderedConfigGenerator(self._active_project) # type: ignore[assignment] # preserve legacy behaviour - using unreliable (potentially rendered) _config_call_dict - if get_flags().require_config_jinja_insensitivity_for_state_modified is False: + if get_flags().state_modified_compare_more_unrendered_values is False: config_call_dict = self._config_call_dict else: # Prefer _config_call_dict if it is available and _unrendered_config_call_dict is not, diff --git a/core/dbt/contracts/project.py b/core/dbt/contracts/project.py index c5e5c4e4b38..e08131ecd8f 100644 --- a/core/dbt/contracts/project.py +++ b/core/dbt/contracts/project.py @@ -338,18 +338,18 @@ class ProjectFlags(ExtensibleDbtClassMixin): write_json: Optional[bool] = None # legacy behaviors - https://github.com/dbt-labs/dbt-core/blob/main/docs/guides/behavior-change-flags.md - require_config_jinja_insensitivity_for_state_modified: bool = False require_explicit_package_overrides_for_builtin_materializations: bool = True require_resource_names_without_spaces: bool = False source_freshness_run_project_hooks: bool = False + state_modified_compare_more_unrendered_values: bool = False @property def project_only_flags(self) -> Dict[str, Any]: return { - "require_config_jinja_insensitivity_for_state_modified": self.require_config_jinja_insensitivity_for_state_modified, "require_explicit_package_overrides_for_builtin_materializations": self.require_explicit_package_overrides_for_builtin_materializations, "require_resource_names_without_spaces": self.require_resource_names_without_spaces, "source_freshness_run_project_hooks": self.source_freshness_run_project_hooks, + "state_modified_compare_more_unrendered_values": self.state_modified_compare_more_unrendered_values, } diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 6d54da28c90..d6092d084a4 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -373,7 +373,7 @@ def update_parsed_node_config( if hasattr(parsed_node, "contract"): parsed_node.contract = Contract.from_dict(contract_dct) - if get_flags().require_config_jinja_insensitivity_for_state_modified: + if get_flags().state_modified_compare_more_unrendered_values: # Use the patch_file.unrendered_configs if available to update patch_dict_config, # as provided patch_config_dict may actuallly already be rendered and thus sensitive to jinja evaluations if patch_file_id: diff --git a/index.html b/index.html new file mode 100644 index 00000000000..c580ce917dd --- /dev/null +++ b/index.html @@ -0,0 +1,75 @@ +dbt Docs
icons
diff --git a/tests/functional/configs/test_configs_in_schema_files.py b/tests/functional/configs/test_configs_in_schema_files.py index d5e0e68ec51..72d41722a53 100644 --- a/tests/functional/configs/test_configs_in_schema_files.py +++ b/tests/functional/configs/test_configs_in_schema_files.py @@ -112,7 +112,7 @@ class TestSchemaFileConfigs: @pytest.fixture(scope="class") def expected_unrendered_config(self): - # my_attr is unrendered when require_config_jinja_insensitivity_for_state_modified: True + # my_attr is unrendered when state_modified_compare_more_unrendered_values: True return { "materialized": "view", "meta": {"my_attr": "{{ var('my_var') }}", "owner": "Julie Smith"}, @@ -135,7 +135,7 @@ def seeds(self): def project_config_update(self): return { "flags": { - "require_config_jinja_insensitivity_for_state_modified": True, + "state_modified_compare_more_unrendered_values": True, }, "models": { "+meta": { @@ -263,7 +263,7 @@ def test_config_layering( class TestLegacySchemaFileConfigs(TestSchemaFileConfigs): @pytest.fixture(scope="class") def expected_unrendered_config(self): - # my_attr is rendered ("TESTING") when require_config_jinja_insensitivity_for_state_modified: False + # my_attr is rendered ("TESTING") when state_modified_compare_more_unrendered_values: False return { "materialized": "view", "meta": {"my_attr": "TESTING", "owner": "Julie Smith"}, @@ -274,9 +274,9 @@ def expected_unrendered_config(self): def project_config_update(self): return { # The uncommented below lines can be removed once the default behaviour is flipped. - # require_config_jinja_insensitivity_for_state_modified defaults to false currently + # state_modified_compare_more_unrendered_values defaults to false currently # "flags": { - # "require_config_jinja_insensitivity_for_state_modified": False, + # "state_modified_compare_more_unrendered_values": False, # }, "models": { "+meta": { diff --git a/tests/functional/defer_state/test_modified_state_environment_vars.py b/tests/functional/defer_state/test_modified_state_environment_vars.py index 7a80a154741..ffbf4525ff9 100644 --- a/tests/functional/defer_state/test_modified_state_environment_vars.py +++ b/tests/functional/defer_state/test_modified_state_environment_vars.py @@ -22,7 +22,7 @@ def setup(self): def project_config_update(self): return { "flags": { - "require_config_jinja_insensitivity_for_state_modified": True, + "state_modified_compare_more_unrendered_values": True, } } @@ -89,7 +89,7 @@ def models(self): def project_config_update(self): return { "flags": { - "require_config_jinja_insensitivity_for_state_modified": True, + "state_modified_compare_more_unrendered_values": True, }, "models": { "test": { diff --git a/tests/functional/defer_state/test_modified_state_vars.py b/tests/functional/defer_state/test_modified_state_vars.py index 353e9cd92cd..01068f0130f 100644 --- a/tests/functional/defer_state/test_modified_state_vars.py +++ b/tests/functional/defer_state/test_modified_state_vars.py @@ -16,7 +16,7 @@ def models(self): def project_config_update(self): return { "flags": { - "require_config_jinja_insensitivity_for_state_modified": False, + "state_modified_compare_more_unrendered_values": False, } } @@ -65,7 +65,7 @@ def models(self): def project_config_update(self): return { "flags": { - "require_config_jinja_insensitivity_for_state_modified": True, + "state_modified_compare_more_unrendered_values": True, } } diff --git a/tests/unit/parser/test_parser.py b/tests/unit/parser/test_parser.py index 1b1cbe6bf2e..524a0cb9ec6 100644 --- a/tests/unit/parser/test_parser.py +++ b/tests/unit/parser/test_parser.py @@ -59,7 +59,7 @@ ) set_from_args( - Namespace(warn_error=False, require_config_jinja_insensitivity_for_state_modified=False), None + Namespace(warn_error=False, state_modified_compare_more_unrendered_values=False), None ) @@ -95,9 +95,7 @@ def _generate_macros(self): def setUp(self): set_from_args( - Namespace( - warn_error=True, require_config_jinja_insensitivity_for_state_modified=False - ), + Namespace(warn_error=True, state_modified_compare_more_unrendered_values=False), None, ) # HACK: this is needed since tracking events can From ac42ec4f28927837630e69eaf7ec5ed9d5de74df Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 25 Sep 2024 13:10:33 +0100 Subject: [PATCH 15/23] changelog entry --- .changes/unreleased/Fixes-20240925-131028.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changes/unreleased/Fixes-20240925-131028.yaml diff --git a/.changes/unreleased/Fixes-20240925-131028.yaml b/.changes/unreleased/Fixes-20240925-131028.yaml new file mode 100644 index 00000000000..2e36f340f1c --- /dev/null +++ b/.changes/unreleased/Fixes-20240925-131028.yaml @@ -0,0 +1,7 @@ +kind: Fixes +body: Ignore rendered jinja in configs for state:modified, behind state_modified_compare_more_unrendered_values + behaviour flag +time: 2024-09-25T13:10:28.490042+01:00 +custom: + Author: michelleark + Issue: "9564" From 71bda2a7874dc834ab3e4efba3bb73e64a9ee8ce Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 25 Sep 2024 15:13:49 +0100 Subject: [PATCH 16/23] fix state:modified for jinja if statements in configs --- core/dbt/clients/jinja_static.py | 6 + core/dbt/contracts/files.py | 8 + core/dbt/parser/base.py | 1 + core/dbt/parser/partial.py | 1 + core/dbt/parser/schemas.py | 1 + tests/functional/defer_state/fixtures.py | 30 ++++ .../test_modified_state_environment_vars.py | 11 +- .../defer_state/test_modified_state_jinja.py | 145 ++++++++++++++++++ 8 files changed, 193 insertions(+), 10 deletions(-) create mode 100644 tests/functional/defer_state/test_modified_state_jinja.py diff --git a/core/dbt/clients/jinja_static.py b/core/dbt/clients/jinja_static.py index d1738a7e26f..3cf277977ed 100644 --- a/core/dbt/clients/jinja_static.py +++ b/core/dbt/clients/jinja_static.py @@ -280,5 +280,11 @@ def _construct_static_kwarg_value(kwarg) -> str: return f"{kwarg.value.node.name}({formatted_all_args})" + elif kwarg_type == "CondExpr": + # Instead of trying to re-assemble complex kwarg value, simply stringify the value + # This is still useful to be able to detect changes in unrendered configs, even if it is + # not an exact representation of the user input. + return str(kwarg.value) + else: return "" diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index 38f858b58d6..3dcf87a7cbd 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -337,6 +337,14 @@ def get_unrendered_config(self, yaml_key, name, version=None) -> Optional[Dict[s return self.unrendered_configs[yaml_key][versioned_name] + def delete_from_unrendered_configs(self, yaml_key, name): + # We delete all unrendered_configs for this yaml_key/name because the + # entry has been scheduled for reparsing. + if self.get_unrendered_config(yaml_key, name): + del self.unrendered_configs[yaml_key][name] + if not self.unrendered_configs[yaml_key]: + del self.unrendered_configs[yaml_key] + def add_env_var(self, var, yaml_key, name): if yaml_key not in self.env_vars: self.env_vars[yaml_key] = {} diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index d6092d084a4..16fea05dd10 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -390,6 +390,7 @@ def update_parsed_node_config( parsed_node.unrendered_config = config.build_config_dict( rendered=False, patch_config_dict=patch_config_dict ) + print(f"final unrendered_config: {parsed_node.unrendered_config}") parsed_node.config_call_dict = config._config_call_dict parsed_node.unrendered_config_call_dict = config._unrendered_config_call_dict diff --git a/core/dbt/parser/partial.py b/core/dbt/parser/partial.py index db3400dbc58..774edf8ce6d 100644 --- a/core/dbt/parser/partial.py +++ b/core/dbt/parser/partial.py @@ -808,6 +808,7 @@ def merge_patch(self, schema_file, key, patch, new_patch=False): pp_dict[key].append(patch) schema_file.delete_from_env_vars(key, patch["name"]) + schema_file.delete_from_unrendered_configs(key, patch["name"]) self.add_to_pp_files(schema_file) # For model, seed, snapshot, analysis schema dictionary keys, diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index c2f8cbe1442..338f79d4a84 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -403,6 +403,7 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: unrendered_config = {} if "config" in entry: unrendered_config = entry["config"] + print(f"unrendered_config: {unrendered_config}") unrendered_version_configs = {} if "versions" in entry: diff --git a/tests/functional/defer_state/fixtures.py b/tests/functional/defer_state/fixtures.py index 4d830cca562..4b409ea4a78 100644 --- a/tests/functional/defer_state/fixtures.py +++ b/tests/functional/defer_state/fixtures.py @@ -573,3 +573,33 @@ {{ config(materialized=var('DBT_TEST_STATE_MODIFIED')) }} select 1 as id """ + +model_with_jinja_in_config_sql = """ +{{ config( + materialized = ('table' if execute else 'view') +) }} + +select 1 as id +""" + +model_with_updated_jinja_in_config_sql = """ +{{ config( + materialized = ('view' if execute else 'table') +) }} + +select 1 as id +""" + +schema_model_with_jinja_in_config_yml = """ +models: + - name: model + config: + materialized: "{{ ('table' if execute else 'view') }}" +""" + +schema_model_with_updated_jinja_in_config_yml = """ +models: + - name: model + config: + materialized: "{{ ('view' if execute else 'table') }}" +""" diff --git a/tests/functional/defer_state/test_modified_state_environment_vars.py b/tests/functional/defer_state/test_modified_state_environment_vars.py index ffbf4525ff9..569a00e5b10 100644 --- a/tests/functional/defer_state/test_modified_state_environment_vars.py +++ b/tests/functional/defer_state/test_modified_state_environment_vars.py @@ -3,7 +3,7 @@ import pytest from dbt.tests.util import run_dbt -from tests.functional.defer_state.fixtures import ( # schema_source_with_env_var_as_property_yml +from tests.functional.defer_state.fixtures import ( model_with_env_var_in_config_sql, model_with_no_in_config_sql, schema_model_with_env_var_in_config_yml, @@ -106,12 +106,3 @@ def models(self): "model.sql": model_with_env_var_in_config_sql, "schema.yml": schema_model_with_env_var_in_config_yml, } - - -# class TestSourceNodeWithEnvVarConfigInSchema(BaseTestStateSelectionEnvVarConfig): -# @pytest.fixture(scope="class") -# def models(self): -# return { -# "schema.yml": schema_source_with_env_var_as_property_yml, -# # "model.sql": "select * from {{ source('jaffle_shop', 'customers') }}" -# } diff --git a/tests/functional/defer_state/test_modified_state_jinja.py b/tests/functional/defer_state/test_modified_state_jinja.py new file mode 100644 index 00000000000..b0c96c42ee1 --- /dev/null +++ b/tests/functional/defer_state/test_modified_state_jinja.py @@ -0,0 +1,145 @@ +import pytest + +from dbt.tests.util import ( + get_artifact, + get_project_config, + run_dbt, + update_config_file, + write_file, +) +from tests.functional.defer_state.fixtures import ( + model_with_jinja_in_config_sql, + model_with_no_in_config_sql, + model_with_updated_jinja_in_config_sql, + schema_model_with_jinja_in_config_yml, + schema_model_with_updated_jinja_in_config_yml, +) +from tests.functional.defer_state.test_modified_state import BaseModifiedState + + +class BaseTestStateSelectionJinjaInConfig(BaseModifiedState): + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": { + "state_modified_compare_more_unrendered_values": True, + } + } + + def update_jinja_expression_in_config(self, project): + pass + + def test_change_target_jinja_if(self, project, dbt_profile_data, profiles_root): + # Generate ./state without changing target.name + run_dbt(["run"]) + self.copy_state() + # Model is table when execute = True + manifest_json = get_artifact(project.project_root, "target", "manifest.json") + assert manifest_json["nodes"]["model.test.model"]["config"]["materialized"] == "view" + + # Assert no false positive (execute = False) + results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert len(results) == 0 + + # Update unrendered config (change jinja expression) + self.update_jinja_expression_in_config(project) + # Assert no false negatives (jinja expression has changed) + results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert len(results) == 1 + + +class TestModelNodeWithJinjaConfigInSqlFile(BaseTestStateSelectionJinjaInConfig): + def update_jinja_expression_in_config(self, project): + write_file( + model_with_updated_jinja_in_config_sql, project.project_root, "models", "model.sql" + ) + + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_jinja_in_config_sql, + } + + +class TestModelNodeWithEnvVarConfigInSchemaYml(BaseTestStateSelectionJinjaInConfig): + def update_jinja_expression_in_config(self, project): + write_file( + schema_model_with_updated_jinja_in_config_yml, + project.project_root, + "models", + "schema.yml", + ) + + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_no_in_config_sql, + "schema.yml": schema_model_with_jinja_in_config_yml, + } + + +class TestModelNodeWithJinjaConfigInProjectYml(BaseTestStateSelectionJinjaInConfig): + def update_jinja_expression_in_config(self, project): + config = get_project_config(project) + config["models"]["test"]["+materialized"] = "{{ ('view' if execute else 'table') }}" + update_config_file(config, "dbt_project.yml") + + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_no_in_config_sql, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "models": { + "test": { + "+materialized": "{{ ('table' if execute else 'view') }}", + } + } + } + + +class TestModelNodeWithJinjaConfigInProjectYmlAndSchemaYml(BaseTestStateSelectionJinjaInConfig): + def update_jinja_expression_in_config(self, project): + write_file( + schema_model_with_updated_jinja_in_config_yml, + project.project_root, + "models", + "schema.yml", + ) + + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_no_in_config_sql, + "schema.yml": schema_model_with_jinja_in_config_yml, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": { + "state_modified_compare_more_unrendered_values": True, + }, + "models": { + "test": { + "+materialized": "{{ ('view' if execute else 'table') }}", + } + }, + } + + +class TestModelNodeWithJinjaConfigInSqlAndSchemaYml(BaseTestStateSelectionJinjaInConfig): + def update_jinja_expression_in_config(self, project): + write_file( + model_with_updated_jinja_in_config_sql, project.project_root, "models", "model.sql" + ) + + @pytest.fixture(scope="class") + def models(self): + return { + "model.sql": model_with_jinja_in_config_sql, + "schema.yml": schema_model_with_jinja_in_config_yml, + } From 8677a9ae10f80d06923d3d7283a2011bda2ab1f1 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 25 Sep 2024 17:35:09 +0100 Subject: [PATCH 17/23] simplify constructing unrendered_config from jinja --- core/dbt/clients/jinja_static.py | 53 +++----------------------------- 1 file changed, 5 insertions(+), 48 deletions(-) diff --git a/core/dbt/clients/jinja_static.py b/core/dbt/clients/jinja_static.py index 3cf277977ed..3d32b3603c9 100644 --- a/core/dbt/clients/jinja_static.py +++ b/core/dbt/clients/jinja_static.py @@ -200,7 +200,7 @@ def statically_parse_unrendered_config(string: str) -> Optional[Dict[str, Any]]: For example, given: "{{ config(materialized=env_var('DBT_TEST_STATE_MODIFIED')) }}\nselect 1 as id" - returns: {"materialized" : "env_var('DBT_TEST_STATE_MODIFIED')"} + returns: {'materialized': "Keyword(key='materialized', value=Call(node=Name(name='env_var', ctx='load'), args=[Const(value='DBT_TEST_STATE_MODIFIED')], kwargs=[], dyn_args=None, dyn_kwargs=None))"} No config call: "select 1 as id" @@ -241,50 +241,7 @@ def statically_parse_unrendered_config(string: str) -> Optional[Dict[str, Any]]: def construct_static_kwarg_value(kwarg): - static_kwarg_value = _construct_static_kwarg_value(kwarg) - if isinstance(static_kwarg_value, str): - return static_kwarg_value.strip("'") - else: - return static_kwarg_value - - -def _construct_static_kwarg_value(kwarg) -> str: - kwarg_type = type(kwarg.value).__name__ - if kwarg_type == "Const": - return ( - f"'{kwarg.value.value}'" if isinstance(kwarg.value.value, str) else kwarg.value.value - ) - - elif kwarg_type == "Call": - unrendered_args = [] - for arg in kwarg.value.args: - arg_type = type(arg).__name__ - if arg_type == "Const": - unrendered_args.append( - f"'{arg.value}'" if isinstance(arg.value, str) else arg.value - ) - - unrendered_kwargs = {} - for call_kwarg in kwarg.value.kwargs: - kwarg_value = _construct_static_kwarg_value(call_kwarg) - - unrendered_kwargs[call_kwarg.key] = kwarg_value - - formatted_unrendered_kwargs = [ - f"{key}={value}" for key, value in unrendered_kwargs.items() - ] - - formatted_all_args = ", ".join( - str(x) for x in (unrendered_args + formatted_unrendered_kwargs) - ) - - return f"{kwarg.value.node.name}({formatted_all_args})" - - elif kwarg_type == "CondExpr": - # Instead of trying to re-assemble complex kwarg value, simply stringify the value - # This is still useful to be able to detect changes in unrendered configs, even if it is - # not an exact representation of the user input. - return str(kwarg.value) - - else: - return "" + # Instead of trying to re-assemble complex kwarg value, simply stringify the value + # This is still useful to be able to detect changes in unrendered configs, even if it is + # not an exact representation of the user input. + return str(kwarg) From 9adb4b8fd856b346f9960e4295de2fb008ed9dea Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 25 Sep 2024 18:45:19 +0100 Subject: [PATCH 18/23] update unit tests for unrendered_config_call_dict values --- tests/unit/clients/test_jinja_static.py | 25 ++++++++++++++++----- tests/unit/parser/test_parser.py | 30 ++++++++++++------------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/tests/unit/clients/test_jinja_static.py b/tests/unit/clients/test_jinja_static.py index cb6888a20f6..42264f24331 100644 --- a/tests/unit/clients/test_jinja_static.py +++ b/tests/unit/clients/test_jinja_static.py @@ -84,19 +84,34 @@ class TestStaticallyParseUnrenderedConfig: @pytest.mark.parametrize( "expression,expected_unrendered_config", [ - ("{{ config(materialized='view') }}", {"materialized": "view"}), + ( + "{{ config(materialized='view') }}", + {"materialized": "Keyword(key='materialized', value=Const(value='view'))"}, + ), ( "{{ config(materialized='view', enabled=True) }}", - {"materialized": "view", "enabled": True}, + { + "materialized": "Keyword(key='materialized', value=Const(value='view'))", + "enabled": "Keyword(key='enabled', value=Const(value=True))", + }, + ), + ( + "{{ config(materialized=env_var('test')) }}", + { + "materialized": "Keyword(key='materialized', value=Call(node=Name(name='env_var', ctx='load'), args=[Const(value='test')], kwargs=[], dyn_args=None, dyn_kwargs=None))" + }, ), - ("{{ config(materialized=env_var('test')) }}", {"materialized": "env_var('test')"}), ( "{{ config(materialized=env_var('test', default='default')) }}", - {"materialized": "env_var('test', default='default')"}, + { + "materialized": "Keyword(key='materialized', value=Call(node=Name(name='env_var', ctx='load'), args=[Const(value='test')], kwargs=[Keyword(key='default', value=Const(value='default'))], dyn_args=None, dyn_kwargs=None))" + }, ), ( "{{ config(materialized=env_var('test', default=env_var('default'))) }}", - {"materialized": "env_var('test', default=env_var('default'))"}, + { + "materialized": "Keyword(key='materialized', value=Call(node=Name(name='env_var', ctx='load'), args=[Const(value='test')], kwargs=[Keyword(key='default', value=Call(node=Name(name='env_var', ctx='load'), args=[Const(value='default')], kwargs=[], dyn_args=None, dyn_kwargs=None))], dyn_args=None, dyn_kwargs=None))" + }, ), ], ) diff --git a/tests/unit/parser/test_parser.py b/tests/unit/parser/test_parser.py index 524a0cb9ec6..9430aeddfaf 100644 --- a/tests/unit/parser/test_parser.py +++ b/tests/unit/parser/test_parser.py @@ -1475,11 +1475,11 @@ def test_single_block(self): "updated_at": "last_update", }, unrendered_config_call_dict={ - "strategy": "timestamp", - "target_database": "dbt", - "target_schema": "analytics", - "unique_key": "id", - "updated_at": "last_update", + "unique_key": "Keyword(key='unique_key', value=Const(value='id'))", + "target_schema": "Keyword(key='target_schema', value=Const(value='analytics'))", + "target_database": "Keyword(key='target_database', value=Const(value='dbt'))", + "strategy": "Keyword(key='strategy', value=Const(value='timestamp'))", + "updated_at": "Keyword(key='updated_at', value=Const(value='last_update'))", }, ) assertEqualNodes(expected, node) @@ -1552,11 +1552,11 @@ def test_multi_block(self): "updated_at": "last_update", }, unrendered_config_call_dict={ - "strategy": "timestamp", - "target_database": "dbt", - "target_schema": "analytics", - "unique_key": "id", - "updated_at": "last_update", + "unique_key": "Keyword(key='unique_key', value=Const(value='id'))", + "target_schema": "Keyword(key='target_schema', value=Const(value='analytics'))", + "target_database": "Keyword(key='target_database', value=Const(value='dbt'))", + "strategy": "Keyword(key='strategy', value=Const(value='timestamp'))", + "updated_at": "Keyword(key='updated_at', value=Const(value='last_update'))", }, ) expect_bar = SnapshotNode( @@ -1596,11 +1596,11 @@ def test_multi_block(self): "updated_at": "last_update", }, unrendered_config_call_dict={ - "strategy": "timestamp", - "target_database": "dbt", - "target_schema": "analytics", - "unique_key": "id", - "updated_at": "last_update", + "unique_key": "Keyword(key='unique_key', value=Const(value='id'))", + "target_schema": "Keyword(key='target_schema', value=Const(value='analytics'))", + "target_database": "Keyword(key='target_database', value=Const(value='dbt'))", + "strategy": "Keyword(key='strategy', value=Const(value='timestamp'))", + "updated_at": "Keyword(key='updated_at', value=Const(value='last_update'))", }, ) assertEqualNodes(nodes[0], expect_bar) From 428df36212e26729a8a70657d910dc27feb66a2f Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 25 Sep 2024 20:41:58 +0100 Subject: [PATCH 19/23] clean up debugging print --- core/dbt/parser/base.py | 1 - core/dbt/parser/models.py | 1 - core/dbt/parser/schemas.py | 3 --- 3 files changed, 5 deletions(-) diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 16fea05dd10..d6092d084a4 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -390,7 +390,6 @@ def update_parsed_node_config( parsed_node.unrendered_config = config.build_config_dict( rendered=False, patch_config_dict=patch_config_dict ) - print(f"final unrendered_config: {parsed_node.unrendered_config}") parsed_node.config_call_dict = config._config_call_dict parsed_node.unrendered_config_call_dict = config._unrendered_config_call_dict diff --git a/core/dbt/parser/models.py b/core/dbt/parser/models.py index 0b0473c7c9b..06e11a89649 100644 --- a/core/dbt/parser/models.py +++ b/core/dbt/parser/models.py @@ -302,7 +302,6 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None: statically_parsed = self.run_experimental_parser(node) # run the stable static parser unless it is explicitly turned off else: - # TODO: consider running new parser here? statically_parsed = self.run_static_parser(node) # if the static parser succeeded, extract some data in easy-to-compare formats diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 338f79d4a84..83f52f36974 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -403,7 +403,6 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: unrendered_config = {} if "config" in entry: unrendered_config = entry["config"] - print(f"unrendered_config: {unrendered_config}") unrendered_version_configs = {} if "versions" in entry: @@ -485,8 +484,6 @@ def parse(self) -> ParseResult: self.manifest.source_patches[key] = patch source_file.source_patches.append(key) else: - # TODO: add unrendered_database from manifest.unrendered_source_patch - # self.yaml.path.original_file_path source = self._target_from_dict(UnparsedSourceDefinition, data) self.add_source_definitions(source) return ParseResult() From 40f1a6725aadcc3566caf6769b089c13bfaddfec Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Wed, 25 Sep 2024 21:56:58 +0100 Subject: [PATCH 20/23] put static parsing behind behaviour change flag --- core/dbt/context/providers.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index dcac46cb396..dfc8c9bb40b 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -79,6 +79,7 @@ SecretEnvVarLocationError, TargetNotFoundError, ) +from dbt.flags import get_flags from dbt.materializations.incremental.microbatch import MicrobatchBuilder from dbt.node_types import ModelLanguage, NodeType from dbt.utils import MultiDict, args_to_dict @@ -398,9 +399,10 @@ def __call__(self, *args, **kwargs): raise DbtRuntimeError("At parse time, did not receive a context config") # Track unrendered opts to build parsed node unrendered_config later on - unrendered_config = statically_parse_unrendered_config(self.model.raw_code) - if unrendered_config: - self.context_config.add_unrendered_config_call(unrendered_config) + if get_flags().state_modified_compare_more_unrendered_values: + unrendered_config = statically_parse_unrendered_config(self.model.raw_code) + if unrendered_config: + self.context_config.add_unrendered_config_call(unrendered_config) # Use rendered opts to populate context_config self.context_config.add_config_call(opts) From 7986726b092963ea98cafc3fd05b70f33629412b Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 26 Sep 2024 01:24:56 +0100 Subject: [PATCH 21/23] tests reflect behaviour flag setting --- tests/unit/parser/test_parser.py | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/tests/unit/parser/test_parser.py b/tests/unit/parser/test_parser.py index 9430aeddfaf..b9b414ebac2 100644 --- a/tests/unit/parser/test_parser.py +++ b/tests/unit/parser/test_parser.py @@ -1474,13 +1474,7 @@ def test_single_block(self): "unique_key": "id", "updated_at": "last_update", }, - unrendered_config_call_dict={ - "unique_key": "Keyword(key='unique_key', value=Const(value='id'))", - "target_schema": "Keyword(key='target_schema', value=Const(value='analytics'))", - "target_database": "Keyword(key='target_database', value=Const(value='dbt'))", - "strategy": "Keyword(key='strategy', value=Const(value='timestamp'))", - "updated_at": "Keyword(key='updated_at', value=Const(value='last_update'))", - }, + unrendered_config_call_dict={}, ) assertEqualNodes(expected, node) file_id = "snowplow://" + normalize("snapshots/nested/snap_1.sql") @@ -1551,13 +1545,8 @@ def test_multi_block(self): "unique_key": "id", "updated_at": "last_update", }, - unrendered_config_call_dict={ - "unique_key": "Keyword(key='unique_key', value=Const(value='id'))", - "target_schema": "Keyword(key='target_schema', value=Const(value='analytics'))", - "target_database": "Keyword(key='target_database', value=Const(value='dbt'))", - "strategy": "Keyword(key='strategy', value=Const(value='timestamp'))", - "updated_at": "Keyword(key='updated_at', value=Const(value='last_update'))", - }, + # Empty until state_modified_compare_more_unrendered_values=True + unrendered_config_call_dict={}, ) expect_bar = SnapshotNode( alias="bar", @@ -1595,13 +1584,8 @@ def test_multi_block(self): "unique_key": "id", "updated_at": "last_update", }, - unrendered_config_call_dict={ - "unique_key": "Keyword(key='unique_key', value=Const(value='id'))", - "target_schema": "Keyword(key='target_schema', value=Const(value='analytics'))", - "target_database": "Keyword(key='target_database', value=Const(value='dbt'))", - "strategy": "Keyword(key='strategy', value=Const(value='timestamp'))", - "updated_at": "Keyword(key='updated_at', value=Const(value='last_update'))", - }, + # Empty until state_modified_compare_more_unrendered_values=True + unrendered_config_call_dict={}, ) assertEqualNodes(nodes[0], expect_bar) assertEqualNodes(nodes[1], expect_foo) From f4a5c0b3383df352ff508c63a312cbeecf82cc8b Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 26 Sep 2024 01:26:20 +0100 Subject: [PATCH 22/23] use resource_types_to_schema_file_keys to get patch unrendered configs --- core/dbt/parser/base.py | 5 +++-- core/dbt/parser/common.py | 20 ++++++++++++++++++++ core/dbt/parser/read_files.py | 3 ++- core/dbt/parser/schemas.py | 17 +---------------- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index d6092d084a4..32dbe6dedab 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -25,6 +25,7 @@ ) from dbt.flags import get_flags from dbt.node_types import AccessType, ModelLanguage, NodeType +from dbt.parser.common import resource_types_to_schema_file_keys from dbt.parser.search import FileBlock from dbt_common.dataclass_schema import ValidationError from dbt_common.utils import deep_merge @@ -379,9 +380,9 @@ def update_parsed_node_config( if patch_file_id: patch_file = self.manifest.files.get(patch_file_id, None) if patch_file and isinstance(patch_file, SchemaSourceFile): - # TODO: do not hardcode "models" + schema_key = resource_types_to_schema_file_keys[parsed_node.resource_type] if unrendered_patch_config := patch_file.get_unrendered_config( - "models", parsed_node.name, getattr(parsed_node, "version", None) + schema_key, parsed_node.name, getattr(parsed_node, "version", None) ): patch_config_dict = deep_merge(patch_config_dict, unrendered_patch_config) diff --git a/core/dbt/parser/common.py b/core/dbt/parser/common.py index d3bb640a78f..5cc4385ea1c 100644 --- a/core/dbt/parser/common.py +++ b/core/dbt/parser/common.py @@ -16,11 +16,31 @@ UnparsedSingularTestUpdate, ) from dbt.exceptions import ParsingError +from dbt.node_types import NodeType from dbt.parser.search import FileBlock from dbt_common.contracts.constraints import ColumnLevelConstraint, ConstraintType from dbt_common.exceptions import DbtInternalError from dbt_semantic_interfaces.type_enums import TimeGranularity +schema_file_keys_to_resource_types = { + "models": NodeType.Model, + "seeds": NodeType.Seed, + "snapshots": NodeType.Snapshot, + "sources": NodeType.Source, + "macros": NodeType.Macro, + "analyses": NodeType.Analysis, + "exposures": NodeType.Exposure, + "metrics": NodeType.Metric, + "semantic_models": NodeType.SemanticModel, + "saved_queries": NodeType.SavedQuery, +} + +resource_types_to_schema_file_keys = { + v: k for (k, v) in schema_file_keys_to_resource_types.items() +} + +schema_file_keys = list(schema_file_keys_to_resource_types.keys()) + def trimmed(inp: str) -> str: if len(inp) < 50: diff --git a/core/dbt/parser/read_files.py b/core/dbt/parser/read_files.py index e5e25841f06..d0ce1a551e3 100644 --- a/core/dbt/parser/read_files.py +++ b/core/dbt/parser/read_files.py @@ -17,7 +17,8 @@ ) from dbt.events.types import InputFileDiffError from dbt.exceptions import ParsingError -from dbt.parser.schemas import schema_file_keys, yaml_from_file +from dbt.parser.common import schema_file_keys +from dbt.parser.schemas import yaml_from_file from dbt.parser.search import filesystem_search from dbt_common.clients.system import load_file_contents from dbt_common.dataclass_schema import dbtClassMixin diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 83f52f36974..6fa73cc3ee9 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -58,6 +58,7 @@ TestBlock, VersionedTestBlock, YamlBlock, + schema_file_keys_to_resource_types, trimmed, ) from dbt.parser.schema_generic_tests import SchemaGenericTestParser @@ -70,22 +71,6 @@ from dbt_common.exceptions import DbtValidationError from dbt_common.utils import deep_merge -schema_file_keys_to_resource_types = { - "models": NodeType.Model, - "seeds": NodeType.Seed, - "snapshots": NodeType.Snapshot, - "sources": NodeType.Source, - "macros": NodeType.Macro, - "analyses": NodeType.Analysis, - "exposures": NodeType.Exposure, - "metrics": NodeType.Metric, - "semantic_models": NodeType.SemanticModel, - "saved_queries": NodeType.SavedQuery, -} - -schema_file_keys = list(schema_file_keys_to_resource_types.keys()) - - # =============================================================================== # Schema Parser classes # From 259d6f83fe04691b7cac430440cbd9a317cb4627 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 26 Sep 2024 01:39:56 +0100 Subject: [PATCH 23/23] handle versioned unrendered_configs during partial parsing --- core/dbt/contracts/files.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index 3dcf87a7cbd..04d7909133a 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -342,6 +342,14 @@ def delete_from_unrendered_configs(self, yaml_key, name): # entry has been scheduled for reparsing. if self.get_unrendered_config(yaml_key, name): del self.unrendered_configs[yaml_key][name] + # Delete all versioned keys associated with name + version_names_to_delete = [] + for potential_version_name in self.unrendered_configs[yaml_key]: + if potential_version_name.startswith(f"{name}_v"): + version_names_to_delete.append(potential_version_name) + for version_name in version_names_to_delete: + del self.unrendered_configs[yaml_key][version_name] + if not self.unrendered_configs[yaml_key]: del self.unrendered_configs[yaml_key]