From 34ded165352e110fca22c76923eb92da11819851 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 16:58:46 -0400 Subject: [PATCH 01/14] add state:modified.vars and depends_on.vars --- core/dbt/artifacts/resources/v1/components.py | 4 + core/dbt/context/providers.py | 12 ++ core/dbt/contracts/graph/nodes.py | 10 ++ core/dbt/graph/selector_methods.py | 1 + schemas/dbt/manifest/v12.json | 144 ++++++++++++++++++ .../functional/artifacts/expected_manifest.py | 29 +++- tests/unit/contracts/graph/test_nodes.py | 4 +- .../unit/contracts/graph/test_nodes_parsed.py | 19 ++- 8 files changed, 208 insertions(+), 15 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/components.py b/core/dbt/artifacts/resources/v1/components.py index 8eb43f35d8e..49aca760670 100644 --- a/core/dbt/artifacts/resources/v1/components.py +++ b/core/dbt/artifacts/resources/v1/components.py @@ -28,11 +28,15 @@ def add_macro(self, value: str): @dataclass class DependsOn(MacroDependsOn): nodes: List[str] = field(default_factory=list) + vars: Dict[str, Any] = field(default_factory=dict) def add_node(self, value: str): if value not in self.nodes: self.nodes.append(value) + def add_var(self, var_name: str, var_value: Any) -> None: + self.vars[var_name] = var_value + @dataclass class RefArgs(dbtClassMixin): diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index dfc8c9bb40b..cce8ecdc8c0 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -790,6 +790,18 @@ def get_missing_var(self, var_name): # in the parser, just always return None. return None + def __call__(self, var_name: str, default: Any = ModelConfiguredVar._VAR_NOTSET) -> Any: + var_value = super().__call__(var_name, default) + + if ( + self._node + and hasattr(self._node, "depends_on") + and hasattr(self._node.depends_on, "add_var") + ): + self._node.depends_on.add_var(var_name, var_value) + + return var_value + class RuntimeVar(ModelConfiguredVar): pass diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index f4b907a9a3c..f0628771527 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -421,6 +421,16 @@ def depends_on_nodes(self): def depends_on_macros(self): return self.depends_on.macros + @property + def depends_on_vars(self): + return self.depends_on.vars + + def same_depends_on_vars(self, old): + return self.depends_on_vars == old.depends_on_vars + + def same_contents(self, old, adapter_type) -> bool: + return super().same_contents(old, adapter_type) and self.same_depends_on_vars(old) + # ==================================== # CompiledNode subclasses diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index dbeaf7ed4c3..fcc5f9c02e6 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -752,6 +752,7 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu "modified.relation": self.check_modified_factory("same_database_representation"), "modified.macros": self.check_modified_macros, "modified.contract": self.check_modified_contract("same_contract", adapter_type), + "modified.vars": self.check_modified_factory("same_depends_on_vars"), } if selector in state_checks: checker = state_checks[selector] diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index d1246d526ce..06d1fe2dde5 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -1832,6 +1832,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -2480,6 +2486,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -3265,6 +3277,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -4069,6 +4087,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -5424,6 +5448,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -6072,6 +6102,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -7024,6 +7060,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -8480,6 +8522,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -9730,6 +9778,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -11636,6 +11690,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -12284,6 +12344,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -13069,6 +13135,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -13873,6 +13945,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -15228,6 +15306,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -15876,6 +15960,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -16828,6 +16918,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -18073,6 +18169,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -19316,6 +19418,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -19731,6 +19839,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -20385,6 +20499,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -20712,6 +20832,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -21265,6 +21391,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -21926,6 +22058,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false @@ -22260,6 +22398,12 @@ "items": { "type": "string" } + }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 606a3e8f5d7..a84501b9062 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -287,7 +287,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "language": "sql", "refs": [{"name": "seed", "package": None, "version": None}], "sources": [], - "depends_on": {"nodes": ["seed.test.seed"], "macros": []}, + "depends_on": {"nodes": ["seed.test.seed"], "macros": [], "vars": {}}, "deprecation_date": None, "unique_id": "model.test.model", "fqn": ["test", "model"], @@ -386,7 +386,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "language": "sql", "refs": [{"name": "seed", "package": None, "version": None}], "sources": [], - "depends_on": {"nodes": ["seed.test.seed"], "macros": []}, + "depends_on": {"nodes": ["seed.test.seed"], "macros": [], "vars": {}}, "deprecation_date": None, "unique_id": "model.test.second_model", "fqn": ["test", "second_model"], @@ -564,6 +564,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": ["macro.dbt.test_not_null", "macro.dbt.get_where_subquery"], "nodes": ["model.test.model"], + "vars": {}, }, "description": "", "file_key_name": "models.model", @@ -617,6 +618,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": [], "nodes": ["seed.test.seed"], + "vars": {"alternate_schema": alternate_schema}, }, "description": "", "docs": {"node_color": None, "show": True}, @@ -664,6 +666,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": ["macro.test.test_nothing", "macro.dbt.get_where_subquery"], "nodes": ["model.test.model"], + "vars": {}, }, "description": "", "file_key_name": "models.model", @@ -716,6 +719,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.model"], + "vars": {}, }, "description": "", "file_key_name": "models.model", @@ -815,6 +819,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": [], "nodes": ["model.test.model", "model.test.second_model"], + "vars": {}, }, "description": "A description of the complex exposure\n", "label": None, @@ -847,6 +852,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": [], "nodes": ["source.test.my_source.my_table", "model.test.model"], + "vars": {}, }, "description": "", "label": None, @@ -960,6 +966,7 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["source.test.my_source.my_table"], + "vars": {}, }, "deprecation_date": None, "description": "", @@ -1029,6 +1036,7 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.ephemeral_copy"], + "vars": {}, }, "deprecation_date": None, "description": "A summmary table of the ephemeral copy of the seed data", @@ -1101,6 +1109,7 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.ephemeral_summary"], + "vars": {}, }, "deprecation_date": None, "description": "A view of the summary of the ephemeral copy of the seed data", @@ -1227,7 +1236,11 @@ def expected_references_manifest(project): "config": get_rendered_snapshot_config(target_schema=alternate_schema), "contract": {"checksum": None, "enforced": False, "alias_types": True}, "database": model_database, - "depends_on": {"macros": [], "nodes": ["seed.test.seed"]}, + "depends_on": { + "macros": [], + "nodes": ["seed.test.seed"], + "vars": {"alternate_schema": alternate_schema}, + }, "description": "", "docs": {"node_color": None, "show": True}, "extra_ctes": [], @@ -1317,6 +1330,7 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.view_summary"], + "vars": {}, }, "description": "A description of the complex exposure", "label": None, @@ -1558,7 +1572,7 @@ def expected_versions_manifest(project): ), "constraints": [], "sources": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "description": "A versioned model", "primary_key": ["count", "first_name"], "deprecation_date": ANY, @@ -1632,7 +1646,7 @@ def expected_versions_manifest(project): "constraints": [], "contract": {"checksum": None, "enforced": False, "alias_types": True}, "sources": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "description": "A versioned model", "primary_key": ["first_name"], "deprecation_date": None, @@ -1686,6 +1700,7 @@ def expected_versions_manifest(project): "model.test.versioned_model.v2", "model.test.versioned_model.v1", ], + "vars": {}, }, "deprecation_date": None, "description": "", @@ -1745,6 +1760,7 @@ def expected_versions_manifest(project): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.versioned_model.v1"], + "vars": {}, }, "description": "", "file_key_name": "models.versioned_model", @@ -1798,6 +1814,7 @@ def expected_versions_manifest(project): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.versioned_model.v1"], + "vars": {}, }, "description": "", "file_key_name": "models.versioned_model", @@ -1851,6 +1868,7 @@ def expected_versions_manifest(project): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.versioned_model.v2"], + "vars": {}, }, "description": "", "file_key_name": "models.versioned_model", @@ -1894,6 +1912,7 @@ def expected_versions_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.versioned_model.v2"], + "vars": {}, }, "description": "notebook_info", "label": None, diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index 4e3a2353105..e27ad0b2410 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -160,7 +160,7 @@ def basic_compiled_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "primary_key": [], @@ -482,7 +482,7 @@ def basic_compiled_schema_test_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "schema": "dbt_test__audit", diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index eeec787dd54..f443d8a181d 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -164,7 +164,7 @@ def base_parsed_model_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "primary_key": [], @@ -275,7 +275,7 @@ def complex_parsed_model_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": ["model.test.bar"]}, + "depends_on": {"macros": [], "nodes": ["model.test.bar"], "vars": {}}, "database": "test_db", "description": "My parsed node", "primary_key": [], @@ -805,7 +805,7 @@ def base_parsed_hook_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "schema": "test_schema", @@ -887,7 +887,7 @@ def complex_parsed_hook_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": ["model.test.bar"]}, + "depends_on": {"macros": [], "nodes": ["model.test.bar"], "vars": {}}, "database": "test_db", "description": "My parsed node", "schema": "test_schema", @@ -1047,7 +1047,7 @@ def basic_parsed_schema_test_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "schema": "test_schema", @@ -1126,7 +1126,7 @@ def complex_parsed_schema_test_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": ["model.test.bar"]}, + "depends_on": {"macros": [], "nodes": ["model.test.bar"], "vars": {}}, "database": "test_db", "description": "My parsed node", "schema": "test_schema", @@ -1516,7 +1516,7 @@ def basic_timestamp_snapshot_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "schema": "test_schema", @@ -1621,7 +1621,7 @@ def basic_check_snapshot_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": []}, + "depends_on": {"macros": [], "nodes": [], "vars": {}}, "database": "test_db", "description": "", "schema": "test_schema", @@ -2063,6 +2063,7 @@ def basic_parsed_exposure_dict(): "depends_on": { "nodes": [], "macros": [], + "vars": {}, }, "refs": [], "sources": [], @@ -2122,6 +2123,7 @@ def complex_parsed_exposure_dict(): "depends_on": { "nodes": ["models.test.my_model"], "macros": [], + "vars": {}, }, "refs": [], "sources": [], @@ -2247,6 +2249,7 @@ def basic_parsed_metric_dict(): "depends_on": { "nodes": [], "macros": [], + "vars": {}, }, } From 87292ee7467af90ff13e103d9a541b0124c5e5cd Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 17:19:08 -0400 Subject: [PATCH 02/14] fix TestList, add TestModifiedVars --- .../defer_state/test_modified_state.py | 30 +++++++++++++++++++ tests/functional/list/test_list.py | 20 ++++++++++--- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 2ded38e742b..983cf0cb65c 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1144,3 +1144,33 @@ def test_changed_semantic_model_contents(self, project): write_file(modified_semantic_model_schema_yml, "models", "schema.yml") results = run_dbt(["list", "-s", "state:modified", "--state", "./state"]) assert len(results) == 1 + + +class TestModifiedVars(BaseModifiedState): + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "vars": {"my_var": 1}, + } + + @pytest.fixture(scope="class") + def models(self): + return { + "view_model.sql": "select {{ var('my_var') }} as id", + } + + def test_changed_vars(self, project): + self.run_and_save_state() + + # No var change + assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert not run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) + + # Modify var (my_var: 1 -> 2 + update_config_file({"vars": {"my_var": 2}}, "dbt_project.yml") + assert run_dbt(["list", "-s", "state:modified", "--state", "./state"]) == [ + "test.view_model" + ] + assert run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) == [ + "test.view_model" + ] diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index 6894ec96bb4..d1ace7d2069 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -49,7 +49,11 @@ def expect_snapshot_output(self, happy_path_project): # noqa: F811 "json": { "name": "my_snapshot", "package_name": "test", - "depends_on": {"nodes": [], "macros": []}, + "depends_on": { + "nodes": [], + "macros": [], + "vars": {"target_database": happy_path_project.database}, + }, "tags": [], "config": { "enabled": True, @@ -106,7 +110,7 @@ def expect_analyses_output(self): "json": { "name": "a", "package_name": "test", - "depends_on": {"nodes": [], "macros": []}, + "depends_on": {"nodes": [], "macros": [], "vars": {}}, "tags": [], "config": { "enabled": True, @@ -170,6 +174,7 @@ def expect_model_output(self): "depends_on": { "nodes": [], "macros": ["macro.dbt.current_timestamp", "macro.dbt.date_trunc"], + "vars": {}, }, "tags": [], "config": { @@ -212,6 +217,7 @@ def expect_model_output(self): "depends_on": { "nodes": ["seed.test.seed"], "macros": ["macro.dbt.is_incremental"], + "vars": {}, }, "tags": [], "config": { @@ -254,6 +260,7 @@ def expect_model_output(self): "depends_on": { "nodes": ["model.test.outer"], "macros": [], + "vars": {}, }, "tags": [], "config": { @@ -296,6 +303,7 @@ def expect_model_output(self): "depends_on": { "nodes": [], "macros": ["macro.dbt.current_timestamp", "macro.dbt.date_trunc"], + "vars": {}, }, "tags": [], "config": { @@ -338,6 +346,7 @@ def expect_model_output(self): "depends_on": { "nodes": [], "macros": ["macro.dbt.current_timestamp", "macro.dbt.date_trunc"], + "vars": {}, }, "tags": [], "config": { @@ -380,6 +389,7 @@ def expect_model_output(self): "depends_on": { "nodes": ["model.test.ephemeral"], "macros": [], + "vars": {}, }, "tags": [], "config": { @@ -437,7 +447,7 @@ def expect_model_ephemeral_output(self): { "name": "outer", "package_name": "test", - "depends_on": {"nodes": [], "macros": []}, + "depends_on": {"nodes": [], "macros": [], "vars": {}}, "tags": [], "config": { "enabled": True, @@ -555,6 +565,7 @@ def expect_test_output(self): "depends_on": { "nodes": ["model.test.outer"], "macros": ["macro.dbt.test_not_null"], + "vars": {}, }, "tags": [], "config": { @@ -583,7 +594,7 @@ def expect_test_output(self): { "name": "t", "package_name": "test", - "depends_on": {"nodes": [], "macros": []}, + "depends_on": {"nodes": [], "macros": [], "vars": {}}, "tags": [], "config": { "enabled": True, @@ -614,6 +625,7 @@ def expect_test_output(self): "depends_on": { "nodes": ["model.test.outer"], "macros": ["macro.dbt.test_unique"], + "vars": {}, }, "tags": [], "config": { From c435b6b04cac18e6f1dc645af5f6a79ee9f2d377 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 17:29:33 -0400 Subject: [PATCH 03/14] add --vars handling in TestModifiedVars --- tests/functional/defer_state/test_modified_state.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 983cf0cb65c..86d2e5af2d8 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1166,7 +1166,7 @@ def test_changed_vars(self, project): assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"]) assert not run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) - # Modify var (my_var: 1 -> 2 + # Modify var (my_var: 1 -> 2) update_config_file({"vars": {"my_var": 2}}, "dbt_project.yml") assert run_dbt(["list", "-s", "state:modified", "--state", "./state"]) == [ "test.view_model" @@ -1174,3 +1174,14 @@ def test_changed_vars(self, project): assert run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) == [ "test.view_model" ] + + # Reset dbt_project.yml + update_config_file({"vars": {"my_var": 1}}, "dbt_project.yml") + + # Modify var via --var CLI flag + assert not run_dbt( + ["list", "--vars", '{"my_var": 1}', "-s", "state:modified", "--state", "./state"] + ) + assert run_dbt( + ["list", "--vars", '{"my_var": 2}', "-s", "state:modified", "--state", "./state"] + ) == ["test.view_model"] From 3c11a53f4e2b4756792b68b629a32ac05b97d18e Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 17:32:13 -0400 Subject: [PATCH 04/14] changelog entry --- .changes/unreleased/Features-20240729-173203.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changes/unreleased/Features-20240729-173203.yaml diff --git a/.changes/unreleased/Features-20240729-173203.yaml b/.changes/unreleased/Features-20240729-173203.yaml new file mode 100644 index 00000000000..e788c8e9cc9 --- /dev/null +++ b/.changes/unreleased/Features-20240729-173203.yaml @@ -0,0 +1,7 @@ +kind: Features +body: Include models that depend on changed vars in state:modified, add state:modified.vars + selection method +time: 2024-07-29T17:32:03.368508-04:00 +custom: + Author: michelleark + Issue: "4304" From c32b542914860811c3ad930e2ee3ec95155f4302 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 18:08:39 -0400 Subject: [PATCH 05/14] fix TestUnitTestList --- tests/functional/unit_testing/test_ut_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/unit_testing/test_ut_list.py b/tests/functional/unit_testing/test_ut_list.py index 0b4f263909b..309f07743e0 100644 --- a/tests/functional/unit_testing/test_ut_list.py +++ b/tests/functional/unit_testing/test_ut_list.py @@ -58,7 +58,7 @@ def test_unit_test_list(self, project): "package_name": "test", "original_file_path": os.path.join("models", "test_my_model.yml"), "unique_id": "unit_test.test.my_model.test_my_model", - "depends_on": {"macros": [], "nodes": ["model.test.my_model"]}, + "depends_on": {"macros": [], "nodes": ["model.test.my_model"], "vars": {}}, "config": {"tags": [], "meta": {}}, } for result in results: From 0ee6ed9a98460c6303f18757f517a1c1df1ee263 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 21:20:18 -0400 Subject: [PATCH 06/14] vars at node-level --- core/dbt/artifacts/resources/v1/components.py | 5 +- core/dbt/artifacts/resources/v1/exposure.py | 1 + core/dbt/artifacts/resources/v1/macro.py | 1 + .../resources/v1/source_definition.py | 1 + core/dbt/context/providers.py | 8 +- core/dbt/contracts/graph/nodes.py | 13 +- core/dbt/graph/selector_methods.py | 4 +- core/dbt/parser/macros.py | 4 + schemas/dbt/manifest/v12.json | 240 +++++++----------- .../functional/artifacts/expected_manifest.py | 24 +- .../defer_state/test_modified_state.py | 51 +++- tests/functional/list/test_list.py | 15 +- tests/functional/unit_testing/test_ut_list.py | 2 +- tests/unit/contracts/graph/test_nodes.py | 4 +- .../unit/contracts/graph/test_nodes_parsed.py | 36 ++- 15 files changed, 195 insertions(+), 214 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/components.py b/core/dbt/artifacts/resources/v1/components.py index 49aca760670..02bfa5d875d 100644 --- a/core/dbt/artifacts/resources/v1/components.py +++ b/core/dbt/artifacts/resources/v1/components.py @@ -28,15 +28,11 @@ def add_macro(self, value: str): @dataclass class DependsOn(MacroDependsOn): nodes: List[str] = field(default_factory=list) - vars: Dict[str, Any] = field(default_factory=dict) def add_node(self, value: str): if value not in self.nodes: self.nodes.append(value) - def add_var(self, var_name: str, var_value: Any) -> None: - self.vars[var_name] = var_value - @dataclass class RefArgs(dbtClassMixin): @@ -201,6 +197,7 @@ class ParsedResource(ParsedResourceMandatory): unrendered_config_call_dict: Dict[str, Any] = field(default_factory=dict) relation_name: Optional[str] = None raw_code: str = "" + vars: Dict[str, Any] = field(default_factory=dict) def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None): dct = super().__post_serialize__(dct, context) diff --git a/core/dbt/artifacts/resources/v1/exposure.py b/core/dbt/artifacts/resources/v1/exposure.py index 00f3c8b89e1..7d8c291381b 100644 --- a/core/dbt/artifacts/resources/v1/exposure.py +++ b/core/dbt/artifacts/resources/v1/exposure.py @@ -41,6 +41,7 @@ class Exposure(GraphResource): tags: List[str] = field(default_factory=list) config: ExposureConfig = field(default_factory=ExposureConfig) unrendered_config: Dict[str, Any] = field(default_factory=dict) + vars: Dict[str, Any] = field(default_factory=dict) url: Optional[str] = None depends_on: DependsOn = field(default_factory=DependsOn) refs: List[RefArgs] = field(default_factory=list) diff --git a/core/dbt/artifacts/resources/v1/macro.py b/core/dbt/artifacts/resources/v1/macro.py index c5154a9a6d4..9220bcf5681 100644 --- a/core/dbt/artifacts/resources/v1/macro.py +++ b/core/dbt/artifacts/resources/v1/macro.py @@ -27,3 +27,4 @@ class Macro(BaseResource): arguments: List[MacroArgument] = field(default_factory=list) created_at: float = field(default_factory=lambda: time.time()) supported_languages: Optional[List[ModelLanguage]] = None + vars: Dict[str, Any] = field(default_factory=dict) diff --git a/core/dbt/artifacts/resources/v1/source_definition.py b/core/dbt/artifacts/resources/v1/source_definition.py index 6c1c3679a00..2650486baa0 100644 --- a/core/dbt/artifacts/resources/v1/source_definition.py +++ b/core/dbt/artifacts/resources/v1/source_definition.py @@ -69,5 +69,6 @@ class SourceDefinition(ParsedSourceMandatory): config: SourceConfig = field(default_factory=SourceConfig) patch_path: Optional[str] = None unrendered_config: Dict[str, Any] = field(default_factory=dict) + vars: Dict[str, Any] = field(default_factory=dict) relation_name: Optional[str] = None created_at: float = field(default_factory=lambda: time.time()) diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index cce8ecdc8c0..a0e3751587a 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -793,12 +793,8 @@ def get_missing_var(self, var_name): def __call__(self, var_name: str, default: Any = ModelConfiguredVar._VAR_NOTSET) -> Any: var_value = super().__call__(var_name, default) - if ( - self._node - and hasattr(self._node, "depends_on") - and hasattr(self._node.depends_on, "add_var") - ): - self._node.depends_on.add_var(var_name, var_value) + if self._node and hasattr(self._node, "vars"): + self._node.vars[var_name] = var_value return var_value diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index f0628771527..a06948ccb38 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -369,6 +369,9 @@ def same_contract(self, old, adapter_type=None) -> bool: # This would only apply to seeds return True + def same_vars(self, old) -> bool: + return self.vars == old.vars + def same_contents(self, old, adapter_type) -> bool: if old is None: return False @@ -382,6 +385,7 @@ def same_contents(self, old, adapter_type) -> bool: and self.same_persisted_description(old) and self.same_fqn(old) and self.same_database_representation(old) + and self.same_vars(old) and same_contract and True ) @@ -421,15 +425,8 @@ def depends_on_nodes(self): def depends_on_macros(self): return self.depends_on.macros - @property - def depends_on_vars(self): - return self.depends_on.vars - - def same_depends_on_vars(self, old): - return self.depends_on_vars == old.depends_on_vars - def same_contents(self, old, adapter_type) -> bool: - return super().same_contents(old, adapter_type) and self.same_depends_on_vars(old) + return super().same_contents(old, adapter_type) # ==================================== diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index fcc5f9c02e6..bf3fca19785 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -623,7 +623,7 @@ def _macros_modified(self) -> List[str]: for uid, macro in new_macros.items(): if uid in old_macros: old_macro = old_macros[uid] - if macro.macro_sql != old_macro.macro_sql: + if macro.macro_sql != old_macro.macro_sql or macro.vars != old_macro.vars: modified.append(uid) else: modified.append(uid) @@ -752,7 +752,7 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu "modified.relation": self.check_modified_factory("same_database_representation"), "modified.macros": self.check_modified_macros, "modified.contract": self.check_modified_contract("same_contract", adapter_type), - "modified.vars": self.check_modified_factory("same_depends_on_vars"), + "modified.vars": self.check_modified_factory("same_vars"), } if selector in state_checks: checker = state_checks[selector] diff --git a/core/dbt/parser/macros.py b/core/dbt/parser/macros.py index f7eaef62b69..6b597dc3dc8 100644 --- a/core/dbt/parser/macros.py +++ b/core/dbt/parser/macros.py @@ -34,6 +34,10 @@ def parse_macro(self, block: jinja.BlockTag, base_node: UnparsedMacro, name: str unique_id = self.generate_unique_id(name) macro_sql = block.full_block or "" + # TODO: + # statically extract potential "vars" calls + # If there are any, create a parse-time macro env + render macro sql to extract vars + return Macro( path=base_node.path, macro_sql=macro_sql, diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 06d1fe2dde5..3a35ce833e3 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -721,6 +721,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "root_path": { "anyOf": [ { @@ -1754,6 +1760,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -1832,12 +1844,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -2408,6 +2414,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -2486,12 +2498,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -3199,6 +3205,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -3277,12 +3289,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -4009,6 +4015,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -4087,12 +4099,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -5370,6 +5376,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -5448,12 +5460,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -6024,6 +6030,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -6102,12 +6114,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -6982,6 +6988,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -7060,12 +7072,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -8522,12 +8528,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -9778,12 +9778,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -10579,6 +10573,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "root_path": { "anyOf": [ { @@ -11612,6 +11612,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -11690,12 +11696,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -12266,6 +12266,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -12344,12 +12350,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -13057,6 +13057,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -13135,12 +13141,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -13867,6 +13867,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -13945,12 +13951,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -15228,6 +15228,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -15306,12 +15312,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -15882,6 +15882,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -15960,12 +15966,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -16840,6 +16840,12 @@ "type": "string", "default": "" }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "language": { "type": "string", "default": "sql" @@ -16918,12 +16924,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -18169,12 +18169,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -19418,12 +19412,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -19839,12 +19827,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -20499,12 +20481,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -20832,12 +20808,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -21391,12 +21361,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -22058,12 +22022,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false @@ -22398,12 +22356,6 @@ "items": { "type": "string" } - }, - "vars": { - "type": "object", - "propertyNames": { - "type": "string" - } } }, "additionalProperties": false diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index a84501b9062..1bc704ea4ba 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -287,7 +287,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "language": "sql", "refs": [{"name": "seed", "package": None, "version": None}], "sources": [], - "depends_on": {"nodes": ["seed.test.seed"], "macros": [], "vars": {}}, + "depends_on": {"nodes": ["seed.test.seed"], "macros": []}, "deprecation_date": None, "unique_id": "model.test.model", "fqn": ["test", "model"], @@ -386,7 +386,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "language": "sql", "refs": [{"name": "seed", "package": None, "version": None}], "sources": [], - "depends_on": {"nodes": ["seed.test.seed"], "macros": [], "vars": {}}, + "depends_on": {"nodes": ["seed.test.seed"], "macros": []}, "deprecation_date": None, "unique_id": "model.test.second_model", "fqn": ["test", "second_model"], @@ -564,7 +564,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": ["macro.dbt.test_not_null", "macro.dbt.get_where_subquery"], "nodes": ["model.test.model"], - "vars": {}, }, "description": "", "file_key_name": "models.model", @@ -618,7 +617,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": [], "nodes": ["seed.test.seed"], - "vars": {"alternate_schema": alternate_schema}, }, "description": "", "docs": {"node_color": None, "show": True}, @@ -666,7 +664,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": ["macro.test.test_nothing", "macro.dbt.get_where_subquery"], "nodes": ["model.test.model"], - "vars": {}, }, "description": "", "file_key_name": "models.model", @@ -719,7 +716,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.model"], - "vars": {}, }, "description": "", "file_key_name": "models.model", @@ -819,7 +815,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": [], "nodes": ["model.test.model", "model.test.second_model"], - "vars": {}, }, "description": "A description of the complex exposure\n", "label": None, @@ -852,7 +847,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "depends_on": { "macros": [], "nodes": ["source.test.my_source.my_table", "model.test.model"], - "vars": {}, }, "description": "", "label": None, @@ -966,7 +960,6 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["source.test.my_source.my_table"], - "vars": {}, }, "deprecation_date": None, "description": "", @@ -1036,7 +1029,6 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.ephemeral_copy"], - "vars": {}, }, "deprecation_date": None, "description": "A summmary table of the ephemeral copy of the seed data", @@ -1109,7 +1101,6 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.ephemeral_summary"], - "vars": {}, }, "deprecation_date": None, "description": "A view of the summary of the ephemeral copy of the seed data", @@ -1239,7 +1230,6 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["seed.test.seed"], - "vars": {"alternate_schema": alternate_schema}, }, "description": "", "docs": {"node_color": None, "show": True}, @@ -1330,7 +1320,6 @@ def expected_references_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.view_summary"], - "vars": {}, }, "description": "A description of the complex exposure", "label": None, @@ -1572,7 +1561,7 @@ def expected_versions_manifest(project): ), "constraints": [], "sources": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "description": "A versioned model", "primary_key": ["count", "first_name"], "deprecation_date": ANY, @@ -1646,7 +1635,7 @@ def expected_versions_manifest(project): "constraints": [], "contract": {"checksum": None, "enforced": False, "alias_types": True}, "sources": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "description": "A versioned model", "primary_key": ["first_name"], "deprecation_date": None, @@ -1700,7 +1689,6 @@ def expected_versions_manifest(project): "model.test.versioned_model.v2", "model.test.versioned_model.v1", ], - "vars": {}, }, "deprecation_date": None, "description": "", @@ -1760,7 +1748,6 @@ def expected_versions_manifest(project): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.versioned_model.v1"], - "vars": {}, }, "description": "", "file_key_name": "models.versioned_model", @@ -1814,7 +1801,6 @@ def expected_versions_manifest(project): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.versioned_model.v1"], - "vars": {}, }, "description": "", "file_key_name": "models.versioned_model", @@ -1868,7 +1854,6 @@ def expected_versions_manifest(project): "depends_on": { "macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"], "nodes": ["model.test.versioned_model.v2"], - "vars": {}, }, "description": "", "file_key_name": "models.versioned_model", @@ -1912,7 +1897,6 @@ def expected_versions_manifest(project): "depends_on": { "macros": [], "nodes": ["model.test.versioned_model.v2"], - "vars": {}, }, "description": "notebook_info", "label": None, diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 86d2e5af2d8..5f34280c64f 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1156,7 +1156,7 @@ def project_config_update(self): @pytest.fixture(scope="class") def models(self): return { - "view_model.sql": "select {{ var('my_var') }} as id", + "model_with_var.sql": "select {{ var('my_var') }} as id", } def test_changed_vars(self, project): @@ -1169,10 +1169,10 @@ def test_changed_vars(self, project): # Modify var (my_var: 1 -> 2) update_config_file({"vars": {"my_var": 2}}, "dbt_project.yml") assert run_dbt(["list", "-s", "state:modified", "--state", "./state"]) == [ - "test.view_model" + "test.model_with_var" ] assert run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) == [ - "test.view_model" + "test.model_with_var" ] # Reset dbt_project.yml @@ -1184,4 +1184,47 @@ def test_changed_vars(self, project): ) assert run_dbt( ["list", "--vars", '{"my_var": 2}', "-s", "state:modified", "--state", "./state"] - ) == ["test.view_model"] + ) == ["test.model_with_var"] + + +macro_with_var_sql = """ +{% macro macro_with_var() %} + {{ var('my_var') }} +{% endmacro %} +""" + + +class TestModifiedMacroVars(BaseModifiedState): + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "vars": {"my_var": 1}, + } + + @pytest.fixture(scope="class") + def models(self): + return { + "model_with_macro.sql": "select {{ macro_with_var() }} as id", + } + + @pytest.fixture(scope="class") + def macros(self): + return { + "macro_with_var.sql": macro_with_var_sql, + } + + def test_changed_vars(self, project): + self.run_and_save_state() + + # No var change + assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert not run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) + + # Modify var (my_var: 1 -> 2) + update_config_file({"vars": {"my_var": 2}}, "dbt_project.yml") + assert run_dbt(["list", "-s", "state:modified", "--state", "./state"]) == [ + "test.model_with_macro" + ] + assert run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) == [ + "test.model_with_macro" + ] diff --git a/tests/functional/list/test_list.py b/tests/functional/list/test_list.py index d1ace7d2069..7b98e5c8251 100644 --- a/tests/functional/list/test_list.py +++ b/tests/functional/list/test_list.py @@ -52,7 +52,6 @@ def expect_snapshot_output(self, happy_path_project): # noqa: F811 "depends_on": { "nodes": [], "macros": [], - "vars": {"target_database": happy_path_project.database}, }, "tags": [], "config": { @@ -110,7 +109,7 @@ def expect_analyses_output(self): "json": { "name": "a", "package_name": "test", - "depends_on": {"nodes": [], "macros": [], "vars": {}}, + "depends_on": {"nodes": [], "macros": []}, "tags": [], "config": { "enabled": True, @@ -174,7 +173,6 @@ def expect_model_output(self): "depends_on": { "nodes": [], "macros": ["macro.dbt.current_timestamp", "macro.dbt.date_trunc"], - "vars": {}, }, "tags": [], "config": { @@ -217,7 +215,6 @@ def expect_model_output(self): "depends_on": { "nodes": ["seed.test.seed"], "macros": ["macro.dbt.is_incremental"], - "vars": {}, }, "tags": [], "config": { @@ -260,7 +257,6 @@ def expect_model_output(self): "depends_on": { "nodes": ["model.test.outer"], "macros": [], - "vars": {}, }, "tags": [], "config": { @@ -303,7 +299,6 @@ def expect_model_output(self): "depends_on": { "nodes": [], "macros": ["macro.dbt.current_timestamp", "macro.dbt.date_trunc"], - "vars": {}, }, "tags": [], "config": { @@ -346,7 +341,6 @@ def expect_model_output(self): "depends_on": { "nodes": [], "macros": ["macro.dbt.current_timestamp", "macro.dbt.date_trunc"], - "vars": {}, }, "tags": [], "config": { @@ -389,7 +383,6 @@ def expect_model_output(self): "depends_on": { "nodes": ["model.test.ephemeral"], "macros": [], - "vars": {}, }, "tags": [], "config": { @@ -447,7 +440,7 @@ def expect_model_ephemeral_output(self): { "name": "outer", "package_name": "test", - "depends_on": {"nodes": [], "macros": [], "vars": {}}, + "depends_on": {"nodes": [], "macros": []}, "tags": [], "config": { "enabled": True, @@ -565,7 +558,6 @@ def expect_test_output(self): "depends_on": { "nodes": ["model.test.outer"], "macros": ["macro.dbt.test_not_null"], - "vars": {}, }, "tags": [], "config": { @@ -594,7 +586,7 @@ def expect_test_output(self): { "name": "t", "package_name": "test", - "depends_on": {"nodes": [], "macros": [], "vars": {}}, + "depends_on": {"nodes": [], "macros": []}, "tags": [], "config": { "enabled": True, @@ -625,7 +617,6 @@ def expect_test_output(self): "depends_on": { "nodes": ["model.test.outer"], "macros": ["macro.dbt.test_unique"], - "vars": {}, }, "tags": [], "config": { diff --git a/tests/functional/unit_testing/test_ut_list.py b/tests/functional/unit_testing/test_ut_list.py index 309f07743e0..0b4f263909b 100644 --- a/tests/functional/unit_testing/test_ut_list.py +++ b/tests/functional/unit_testing/test_ut_list.py @@ -58,7 +58,7 @@ def test_unit_test_list(self, project): "package_name": "test", "original_file_path": os.path.join("models", "test_my_model.yml"), "unique_id": "unit_test.test.my_model.test_my_model", - "depends_on": {"macros": [], "nodes": ["model.test.my_model"], "vars": {}}, + "depends_on": {"macros": [], "nodes": ["model.test.my_model"]}, "config": {"tags": [], "meta": {}}, } for result in results: diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index e27ad0b2410..4e3a2353105 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -160,7 +160,7 @@ def basic_compiled_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "primary_key": [], @@ -482,7 +482,7 @@ def basic_compiled_schema_test_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "schema": "dbt_test__audit", diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index f443d8a181d..1b323dd718b 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -164,7 +164,7 @@ def base_parsed_model_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "primary_key": [], @@ -200,6 +200,7 @@ def base_parsed_model_dict(): }, "unrendered_config": {}, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, "access": AccessType.Protected.value, "constraints": [], @@ -256,6 +257,7 @@ def minimal_parsed_model_dict(): "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", }, "unrendered_config": {}, + "vars": {}, } @@ -275,7 +277,7 @@ def complex_parsed_model_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": ["model.test.bar"], "vars": {}}, + "depends_on": {"macros": [], "nodes": ["model.test.bar"]}, "database": "test_db", "description": "My parsed node", "primary_key": [], @@ -323,6 +325,7 @@ def complex_parsed_model_dict(): "post_hook": ['insert into blah(a, b) select "1", 1'], }, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, "access": AccessType.Protected.value, "constraints": [], @@ -533,6 +536,7 @@ def basic_parsed_seed_dict(): "checksum": {"name": "path", "checksum": "seeds/seed.csv"}, "unrendered_config": {}, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -639,6 +643,7 @@ def complex_parsed_seed_dict(): "persist_docs": {"relation": True, "columns": True}, }, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -805,7 +810,7 @@ def base_parsed_hook_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "schema": "test_schema", @@ -839,6 +844,7 @@ def base_parsed_hook_dict(): }, "unrendered_config": {}, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -887,7 +893,7 @@ def complex_parsed_hook_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": ["model.test.bar"], "vars": {}}, + "depends_on": {"macros": [], "nodes": ["model.test.bar"]}, "database": "test_db", "description": "My parsed node", "schema": "test_schema", @@ -933,6 +939,7 @@ def complex_parsed_hook_dict(): "materialized": "table", }, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -1047,7 +1054,7 @@ def basic_parsed_schema_test_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "schema": "test_schema", @@ -1078,6 +1085,7 @@ def basic_parsed_schema_test_dict(): }, "unrendered_config": {}, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -1126,7 +1134,7 @@ def complex_parsed_schema_test_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": ["model.test.bar"], "vars": {}}, + "depends_on": {"macros": [], "nodes": ["model.test.bar"]}, "database": "test_db", "description": "My parsed node", "schema": "test_schema", @@ -1167,6 +1175,7 @@ def complex_parsed_schema_test_dict(): }, "unrendered_config": {"materialized": "table", "severity": "WARN"}, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -1516,7 +1525,7 @@ def basic_timestamp_snapshot_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "schema": "test_schema", @@ -1562,6 +1571,7 @@ def basic_timestamp_snapshot_dict(): "target_schema": "some_snapshot_schema", }, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } @@ -1621,7 +1631,7 @@ def basic_check_snapshot_dict(): "refs": [], "sources": [], "metrics": [], - "depends_on": {"macros": [], "nodes": [], "vars": {}}, + "depends_on": {"macros": [], "nodes": []}, "database": "test_db", "description": "", "schema": "test_schema", @@ -1668,6 +1678,7 @@ def basic_check_snapshot_dict(): }, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "vars": {}, } @@ -1760,6 +1771,7 @@ def _ok_dict(self): "description": "my macro description", "docs": {"show": True}, "arguments": [], + "vars": {}, } def test_ok(self): @@ -1776,6 +1788,7 @@ def test_ok(self): meta={}, description="my macro description", arguments=[], + vars={}, ) assert_symmetric(macro, macro_dict) pickle.loads(pickle.dumps(macro)) @@ -1877,6 +1890,7 @@ def basic_parsed_source_definition_dict(): "enabled": True, }, "unrendered_config": {}, + "vars": {}, } @@ -1909,6 +1923,7 @@ def complex_parsed_source_definition_dict(): "freshness": {"warn_after": {"period": "hour", "count": 1}, "error_after": {}}, "loaded_at_field": "loaded_at", "unrendered_config": {}, + "vars": {}, } @@ -2063,7 +2078,6 @@ def basic_parsed_exposure_dict(): "depends_on": { "nodes": [], "macros": [], - "vars": {}, }, "refs": [], "sources": [], @@ -2081,6 +2095,7 @@ def basic_parsed_exposure_dict(): "enabled": True, }, "unrendered_config": {}, + "vars": {}, } @@ -2123,7 +2138,6 @@ def complex_parsed_exposure_dict(): "depends_on": { "nodes": ["models.test.my_model"], "macros": [], - "vars": {}, }, "refs": [], "sources": [], @@ -2137,6 +2151,7 @@ def complex_parsed_exposure_dict(): "enabled": True, }, "unrendered_config": {}, + "vars": {}, } @@ -2249,7 +2264,6 @@ def basic_parsed_metric_dict(): "depends_on": { "nodes": [], "macros": [], - "vars": {}, }, } From cbe26886497a6eec961e4fc2d240ff6cebd8f804 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 29 Jul 2024 23:37:50 -0400 Subject: [PATCH 07/14] remove vars from Macro resource, fix unit + integration tests --- core/dbt/artifacts/resources/v1/macro.py | 1 - core/dbt/graph/selector_methods.py | 2 +- core/dbt/parser/macros.py | 4 ---- schemas/dbt/manifest/v12.json | 24 +++++++++++++++++++ .../functional/artifacts/expected_manifest.py | 24 +++++++++++++++++++ .../defer_state/test_modified_state.py | 3 +++ tests/unit/contracts/graph/test_manifest.py | 1 + tests/unit/contracts/graph/test_nodes.py | 2 ++ .../unit/contracts/graph/test_nodes_parsed.py | 2 -- 9 files changed, 55 insertions(+), 8 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/macro.py b/core/dbt/artifacts/resources/v1/macro.py index 9220bcf5681..c5154a9a6d4 100644 --- a/core/dbt/artifacts/resources/v1/macro.py +++ b/core/dbt/artifacts/resources/v1/macro.py @@ -27,4 +27,3 @@ class Macro(BaseResource): arguments: List[MacroArgument] = field(default_factory=list) created_at: float = field(default_factory=lambda: time.time()) supported_languages: Optional[List[ModelLanguage]] = None - vars: Dict[str, Any] = field(default_factory=dict) diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index bf3fca19785..c7a2490760b 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -623,7 +623,7 @@ def _macros_modified(self) -> List[str]: for uid, macro in new_macros.items(): if uid in old_macros: old_macro = old_macros[uid] - if macro.macro_sql != old_macro.macro_sql or macro.vars != old_macro.vars: + if macro.macro_sql != old_macro.macro_sql: modified.append(uid) else: modified.append(uid) diff --git a/core/dbt/parser/macros.py b/core/dbt/parser/macros.py index 6b597dc3dc8..f7eaef62b69 100644 --- a/core/dbt/parser/macros.py +++ b/core/dbt/parser/macros.py @@ -34,10 +34,6 @@ def parse_macro(self, block: jinja.BlockTag, base_node: UnparsedMacro, name: str unique_id = self.generate_unique_id(name) macro_sql = block.full_block or "" - # TODO: - # statically extract potential "vars" calls - # If there are any, create a parse-time macro env + render macro sql to extract vars - return Macro( path=base_node.path, macro_sql=macro_sql, diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 3a35ce833e3..ea21214b4fc 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -8131,6 +8131,12 @@ "type": "string" } }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -8502,6 +8508,12 @@ "type": "string" } }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "url": { "anyOf": [ { @@ -17974,6 +17986,12 @@ "type": "string" } }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -18143,6 +18161,12 @@ "type": "string" } }, + "vars": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "url": { "anyOf": [ { diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 1bc704ea4ba..368d6fec87d 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -363,6 +363,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "extra_ctes": [], "checksum": checksum_file(model_sql_path), "unrendered_config": unrendered_model_config, + "vars": {}, "access": "protected", "version": None, "latest_version": None, @@ -462,6 +463,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "extra_ctes": [], "checksum": checksum_file(second_model_sql_path), "unrendered_config": unrendered_second_config, + "vars": {}, "access": "protected", "version": None, "latest_version": None, @@ -544,6 +546,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "docs": {"node_color": None, "show": True}, "checksum": checksum_file(seed_path), "unrendered_config": unrendered_seed_config, + "vars": {}, "relation_name": relation_name_node_format.format( project.database, my_schema_name, "seed" ), @@ -599,6 +602,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "vars": {}, "contract": {"checksum": None, "enforced": False, "alias_types": True}, }, "snapshot.test.snapshot_seed": { @@ -646,6 +650,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "unique_id": "snapshot.test.snapshot_seed", "unrendered_config": unrendered_snapshot_config, + "vars": {"alternate_schema": alternate_schema}, }, "test.test.test_nothing_model_.5d38568946": { "alias": "test_nothing_model_", @@ -698,6 +703,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "vars": {}, }, "test.test.unique_model_id.67b76558ff": { "alias": "unique_model_id", @@ -751,6 +757,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "vars": {}, }, }, "sources": { @@ -807,6 +814,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "unique_id": "source.test.my_source.my_table", "fqn": ["test", "my_source", "my_table"], "unrendered_config": {}, + "vars": {}, }, }, "exposures": { @@ -841,6 +849,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "unique_id": "exposure.test.notebook_exposure", "url": "http://example.com/notebook/1", "unrendered_config": {}, + "vars": {}, }, "exposure.test.simple_exposure": { "created_at": ANY, @@ -873,6 +882,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "meta": {}, "tags": [], "unrendered_config": {}, + "vars": {}, }, }, "metrics": {}, @@ -990,6 +1000,7 @@ def expected_references_manifest(project): "extra_ctes": [], "checksum": checksum_file(ephemeral_copy_path), "unrendered_config": get_unrendered_model_config(materialized="ephemeral"), + "vars": {}, "access": "protected", "version": None, "latest_version": None, @@ -1062,6 +1073,7 @@ def expected_references_manifest(project): "unrendered_config": get_unrendered_model_config( materialized="table", group="test_group" ), + "vars": {}, "access": "protected", "version": None, "latest_version": None, @@ -1130,6 +1142,7 @@ def expected_references_manifest(project): "extra_ctes": [], "checksum": checksum_file(view_summary_path), "unrendered_config": get_unrendered_model_config(materialized="view"), + "vars": {}, "access": "protected", "version": None, "latest_version": None, @@ -1213,6 +1226,7 @@ def expected_references_manifest(project): "unique_id": "seed.test.seed", "checksum": checksum_file(seed_path), "unrendered_config": get_unrendered_seed_config(), + "vars": {}, "relation_name": '"{0}"."{1}".seed'.format(project.database, my_schema_name), }, "snapshot.test.snapshot_seed": { @@ -1258,6 +1272,7 @@ def expected_references_manifest(project): "unrendered_config": get_unrendered_snapshot_config( target_schema=alternate_schema ), + "vars": {"alternate_schema": alternate_schema}, }, }, "sources": { @@ -1312,6 +1327,7 @@ def expected_references_manifest(project): "unique_id": "source.test.my_source.my_table", "fqn": ["test", "my_source", "my_table"], "unrendered_config": {}, + "vars": {}, }, }, "exposures": { @@ -1337,6 +1353,7 @@ def expected_references_manifest(project): "package_name": "test", "path": "schema.yml", "refs": [{"name": "view_summary", "package": None, "version": None}], + "vars": {}, "resource_type": "exposure", "sources": [], "type": "notebook", @@ -1597,6 +1614,7 @@ def expected_versions_manifest(project): group="test_group", meta={"size": "large", "color": "blue"}, ), + "vars": {}, "access": "protected", "version": 1, "latest_version": 2, @@ -1668,6 +1686,7 @@ def expected_versions_manifest(project): "unrendered_config": get_unrendered_model_config( materialized="view", group="test_group", meta={"size": "large", "color": "red"} ), + "vars": {}, "access": "protected", "version": 2, "latest_version": 2, @@ -1726,6 +1745,7 @@ def expected_versions_manifest(project): "extra_ctes": [], "checksum": checksum_file(ref_versioned_model_path), "unrendered_config": get_unrendered_model_config(), + "vars": {}, "access": "protected", "version": None, "latest_version": None, @@ -1783,6 +1803,7 @@ def expected_versions_manifest(project): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "vars": {}, }, "test.test.unique_versioned_model_v1_count.0b4c0b688a": { "alias": "unique_versioned_model_v1_count", @@ -1836,6 +1857,7 @@ def expected_versions_manifest(project): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "vars": {}, }, "test.test.unique_versioned_model_v2_first_name.998430d28e": { "alias": "unique_versioned_model_v2_first_name", @@ -1889,6 +1911,7 @@ def expected_versions_manifest(project): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "vars": {}, }, }, "exposures": { @@ -1920,6 +1943,7 @@ def expected_versions_manifest(project): "unique_id": "exposure.test.notebook_exposure", "url": None, "unrendered_config": {}, + "vars": {}, }, }, "metrics": {}, diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 5f34280c64f..841263504e4 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1228,3 +1228,6 @@ def test_changed_vars(self, project): assert run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) == [ "test.model_with_macro" ] + + # Macros themselves not captured as modified because the var value depends on a node's context + assert not run_dbt(["list", "-s", "state:modified.macros", "--state", "./state"]) diff --git a/tests/unit/contracts/graph/test_manifest.py b/tests/unit/contracts/graph/test_manifest.py index 36f14537b09..eaff7f6c57e 100644 --- a/tests/unit/contracts/graph/test_manifest.py +++ b/tests/unit/contracts/graph/test_manifest.py @@ -97,6 +97,7 @@ "defer_relation", "time_spine", "batches", + "vars", } ) diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index 4e3a2353105..c22a22176af 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -200,6 +200,7 @@ def basic_compiled_dict(): }, "unrendered_config": {}, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, "access": "protected", "constraints": [], @@ -520,6 +521,7 @@ def basic_compiled_schema_test_dict(): "severity": "warn", }, "unrendered_config_call_dict": {}, + "vars": {}, "config_call_dict": {}, } diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index 1b323dd718b..27042777525 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -1771,7 +1771,6 @@ def _ok_dict(self): "description": "my macro description", "docs": {"show": True}, "arguments": [], - "vars": {}, } def test_ok(self): @@ -1788,7 +1787,6 @@ def test_ok(self): meta={}, description="my macro description", arguments=[], - vars={}, ) assert_symmetric(macro, macro_dict) pickle.loads(pickle.dumps(macro)) From 91fbe7f1285b69a0eeaf53f04753db001ef07b75 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Tue, 30 Jul 2024 20:41:30 -0400 Subject: [PATCH 08/14] TestModifiedVarsSchemaYml + support for modified.vars in schema.yml files --- core/dbt/context/configured.py | 38 +++++++---- core/dbt/contracts/files.py | 16 +++++ core/dbt/contracts/graph/nodes.py | 12 +++- core/dbt/parser/schema_yaml_readers.py | 5 +- core/dbt/parser/schemas.py | 18 ++++-- core/dbt/parser/sources.py | 6 ++ .../defer_state/test_modified_state.py | 64 +++++++++++++++++++ 7 files changed, 139 insertions(+), 20 deletions(-) diff --git a/core/dbt/context/configured.py b/core/dbt/context/configured.py index 240d9afb843..9ee7ebc79c3 100644 --- a/core/dbt/context/configured.py +++ b/core/dbt/context/configured.py @@ -31,23 +31,35 @@ def __init__(self, package_name: str): self.resource_type = NodeType.Model +class SchemaYamlVars: + def __init__(self): + self.env_vars = {} + self.vars = {} + + class ConfiguredVar(Var): def __init__( self, context: Dict[str, Any], config: AdapterRequiredConfig, project_name: str, + schema_yaml_vars: Optional[SchemaYamlVars] = None, ): super().__init__(context, config.cli_vars) self._config = config self._project_name = project_name + self.schema_yaml_vars = schema_yaml_vars def __call__(self, var_name, default=Var._VAR_NOTSET): my_config = self._config.load_dependencies()[self._project_name] + var_found = False + var_value = None + # cli vars > active project > local project if var_name in self._config.cli_vars: - return self._config.cli_vars[var_name] + var_found = True + var_value = self._config.cli_vars[var_name] adapter_type = self._config.credentials.type lookup = FQNLookup(self._project_name) @@ -58,19 +70,21 @@ def __call__(self, var_name, default=Var._VAR_NOTSET): all_vars.add(my_config.vars.vars_for(lookup, adapter_type)) all_vars.add(active_vars) - if var_name in all_vars: - return all_vars[var_name] + if not var_found and var_name in all_vars: + var_found = True + var_value = all_vars[var_name] - if default is not Var._VAR_NOTSET: - return default - - return self.get_missing_var(var_name) + if not var_found and default is not Var._VAR_NOTSET: + var_found = True + var_value = default + if not var_found: + return self.get_missing_var(var_name) + else: + if self.schema_yaml_vars: + self.schema_yaml_vars.vars[var_name] = var_value -class SchemaYamlVars: - def __init__(self): - self.env_vars = {} - self.vars = {} + return var_value class SchemaYamlContext(ConfiguredContext): @@ -82,7 +96,7 @@ def __init__(self, config, project_name: str, schema_yaml_vars: Optional[SchemaY @contextproperty() def var(self) -> ConfiguredVar: - return ConfiguredVar(self._ctx, self.config, self._project_name) + return ConfiguredVar(self._ctx, self.config, self._project_name, self.schema_yaml_vars) @contextmember() def env_var(self, var: str, default: Optional[str] = None) -> str: diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index 04d7909133a..2e8dd73f801 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -213,6 +213,7 @@ class SchemaSourceFile(BaseSourceFile): 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) + vars: Dict[str, Any] = field(default_factory=dict) pp_dict: Optional[Dict[str, Any]] = None pp_test_index: Optional[Dict[str, Any]] = None @@ -352,6 +353,21 @@ def delete_from_unrendered_configs(self, yaml_key, name): if not self.unrendered_configs[yaml_key]: del self.unrendered_configs[yaml_key] + def add_vars(self, vars: Dict[str, Any], yaml_key: str, name: str) -> None: + if yaml_key not in self.vars: + self.vars[yaml_key] = {} + + if name not in self.vars[yaml_key]: + self.vars[yaml_key][name] = vars + + def get_vars(self, yaml_key: str, name: str) -> Dict[str, Any]: + if yaml_key not in self.vars: + return {} + + if name not in self.vars[yaml_key]: + return {} + + return self.vars[yaml_key][name] def add_env_var(self, var, yaml_key, name): if yaml_key not in self.env_vars: diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index a06948ccb38..731a5c53b48 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -425,9 +425,6 @@ def depends_on_nodes(self): def depends_on_macros(self): return self.depends_on.macros - def same_contents(self, old, adapter_type) -> bool: - return super().same_contents(old, adapter_type) - # ==================================== # CompiledNode subclasses @@ -1258,6 +1255,9 @@ def same_config(self, old: "SourceDefinition") -> bool: old.unrendered_config, ) + def same_vars(self, other: "SourceDefinition") -> bool: + return self.vars == other.vars + def same_contents(self, old: Optional["SourceDefinition"]) -> bool: # existing when it didn't before is a change! if old is None: @@ -1278,6 +1278,7 @@ def same_contents(self, old: Optional["SourceDefinition"]) -> bool: and self.same_quoting(old) and self.same_freshness(old) and self.same_external(old) + and self.same_vars(old) and True ) @@ -1374,6 +1375,9 @@ def same_config(self, old: "Exposure") -> bool: old.unrendered_config, ) + def same_vars(self, old: "Exposure") -> bool: + return self.vars == old.vars + def same_contents(self, old: Optional["Exposure"]) -> bool: # existing when it didn't before is a change! # metadata/tags changes are not "changes" @@ -1390,6 +1394,7 @@ def same_contents(self, old: Optional["Exposure"]) -> bool: and self.same_label(old) and self.same_depends_on(old) and self.same_config(old) + and self.same_vars(old) and True ) @@ -1641,6 +1646,7 @@ class ParsedNodePatch(ParsedPatch): latest_version: Optional[NodeVersion] constraints: List[Dict[str, Any]] deprecation_date: Optional[datetime] + vars: Dict[str, Any] time_spine: Optional[TimeSpine] = None diff --git a/core/dbt/parser/schema_yaml_readers.py b/core/dbt/parser/schema_yaml_readers.py index dc99e87a218..4fe28a74866 100644 --- a/core/dbt/parser/schema_yaml_readers.py +++ b/core/dbt/parser/schema_yaml_readers.py @@ -91,6 +91,9 @@ def parse_exposure(self, unparsed: UnparsedExposure) -> None: unique_id = f"{NodeType.Exposure}.{package_name}.{unparsed.name}" path = self.yaml.path.relative_path + assert isinstance(self.yaml.file, SchemaSourceFile) + exposure_vars = self.yaml.file.get_vars(self.key, unparsed.name) + fqn = self.schema_parser.get_fqn_prefix(path) fqn.append(unparsed.name) @@ -133,6 +136,7 @@ def parse_exposure(self, unparsed: UnparsedExposure) -> None: maturity=unparsed.maturity, config=config, unrendered_config=unrendered_config, + vars=exposure_vars, ) ctx = generate_parse_exposure( parsed, @@ -144,7 +148,6 @@ def parse_exposure(self, unparsed: UnparsedExposure) -> None: get_rendered(depends_on_jinja, ctx, parsed, capture_macros=True) # parsed now has a populated refs/sources/metrics - assert isinstance(self.yaml.file, SchemaSourceFile) if parsed.config.enabled: self.manifest.add_exposure(self.yaml.file, parsed) else: diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 29f36f96b34..3bde9a2440a 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -403,10 +403,14 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]: if self.schema_yaml_vars.env_vars: self.schema_parser.manifest.env_vars.update(self.schema_yaml_vars.env_vars) - for var in self.schema_yaml_vars.env_vars.keys(): - schema_file.add_env_var(var, self.key, entry["name"]) + for env_var in self.schema_yaml_vars.env_vars.keys(): + schema_file.add_env_var(env_var, self.key, entry["name"]) self.schema_yaml_vars.env_vars = {} + if self.schema_yaml_vars.vars: + schema_file.add_vars(self.schema_yaml_vars.vars, self.key, entry["name"]) + self.schema_yaml_vars.vars = {} + yield entry def render_entry(self, dct): @@ -677,6 +681,9 @@ def parse_patch(self, block: TargetBlock[NodeTarget], refs: ParserRef) -> None: # code consistency. deprecation_date: Optional[datetime.datetime] = None time_spine: Optional[TimeSpine] = None + assert isinstance(self.yaml.file, SchemaSourceFile) + source_file: SchemaSourceFile = self.yaml.file + if isinstance(block.target, UnparsedModelUpdate): deprecation_date = block.target.deprecation_date time_spine = ( @@ -709,9 +716,9 @@ def parse_patch(self, block: TargetBlock[NodeTarget], refs: ParserRef) -> None: constraints=block.target.constraints, deprecation_date=deprecation_date, time_spine=time_spine, + vars=source_file.get_vars(block.target.yaml_key, block.target.name), ) - assert isinstance(self.yaml.file, SchemaSourceFile) - source_file: SchemaSourceFile = self.yaml.file + if patch.yaml_key in ["models", "seeds", "snapshots"]: unique_id = self.manifest.ref_lookup.get_unique_id( patch.name, self.project.project_name, None @@ -805,6 +812,8 @@ def patch_node_properties(self, node, patch: "ParsedNodePatch") -> None: node.description = patch.description node.columns = patch.columns node.name = patch.name + # Prefer node-level vars to vars from patch + node.vars = {**patch.vars, **node.vars} if not isinstance(node, ModelNode): for attr in ["latest_version", "access", "version", "constraints"]: @@ -954,6 +963,7 @@ def parse_patch(self, block: TargetBlock[UnparsedModelUpdate], refs: ParserRef) latest_version=latest_version, constraints=unparsed_version.constraints or target.constraints, deprecation_date=unparsed_version.deprecation_date, + vars=source_file.get_vars(block.target.yaml_key, block.target.name), ) # Node patched before config because config patching depends on model name, # which may have been updated in the version patch diff --git a/core/dbt/parser/sources.py b/core/dbt/parser/sources.py index 68dbed94ce5..4aa08787b37 100644 --- a/core/dbt/parser/sources.py +++ b/core/dbt/parser/sources.py @@ -12,6 +12,7 @@ ContextConfigGenerator, UnrenderedConfigGenerator, ) +from dbt.contracts.files import SchemaSourceFile from dbt.contracts.graph.manifest import Manifest, SourceKey from dbt.contracts.graph.nodes import ( GenericTestNode, @@ -158,6 +159,10 @@ def parse_source(self, target: UnpatchedSourceDefinition) -> SourceDefinition: rendered=False, ) + schema_file = self.manifest.files[target.file_id] + assert isinstance(schema_file, SchemaSourceFile) + source_vars = schema_file.get_vars("sources", source.name) + if not isinstance(config, SourceConfig): raise DbtInternalError( f"Calculated a {type(config)} for a source, but expected a SourceConfig" @@ -190,6 +195,7 @@ def parse_source(self, target: UnpatchedSourceDefinition) -> SourceDefinition: tags=tags, config=config, unrendered_config=unrendered_config, + vars=source_vars, ) if ( diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 841263504e4..13af62c8e82 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1231,3 +1231,67 @@ def test_changed_vars(self, project): # Macros themselves not captured as modified because the var value depends on a node's context assert not run_dbt(["list", "-s", "state:modified.macros", "--state", "./state"]) + + +# TODO: test versioned models, tests +model_with_var_schema_yml = """ +version: 2 +models: + - name: model_with_var + config: + materialized: "{{ var('my_var') }}" + +exposures: + - name: exposure_name + type: dashboard + owner: + name: "{{ var('my_var') }}" + +sources: + - name: jaffle_shop + database: "{{ var('my_var') }}" + schema: jaffle_shop + tables: + - name: orders + - name: customers +""" + + +class TestModifiedVarsSchemaYml(BaseModifiedState): + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "vars": {"my_var": "table"}, + } + + @pytest.fixture(scope="class") + def models(self): + return {"model_with_var.sql": "select 1 as id", "schema.yml": model_with_var_schema_yml} + + def test_changed_vars(self, project): + self.run_and_save_state() + + # No var change + assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert not run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) + + # Modify var (my_var: table -> view) + update_config_file({"vars": {"my_var": "view"}}, "dbt_project.yml") + assert sorted(run_dbt(["list", "-s", "state:modified", "--state", "./state"])) == sorted( + [ + "test.model_with_var", + "exposure:test.exposure_name", + "source:test.jaffle_shop.customers", + "source:test.jaffle_shop.orders", + ] + ) + assert sorted( + run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) + ) == sorted( + [ + "test.model_with_var", + "exposure:test.exposure_name", + "source:test.jaffle_shop.customers", + "source:test.jaffle_shop.orders", + ] + ) From 142294439e8c9b38dac2dbc54aaf29c05375d9da Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 30 Sep 2024 12:03:18 +0200 Subject: [PATCH 09/14] put state:modified.vars behind behaviour flag --- core/dbt/contracts/graph/nodes.py | 15 ++++++++++++--- core/dbt/contracts/project.py | 2 ++ .../functional/defer_state/test_modified_state.py | 9 +++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 731a5c53b48..6c589f11cf5 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -370,7 +370,10 @@ def same_contract(self, old, adapter_type=None) -> bool: return True def same_vars(self, old) -> bool: - return self.vars == old.vars + if get_flags().state_modified_compare_vars: + return self.vars == old.vars + else: + return True def same_contents(self, old, adapter_type) -> bool: if old is None: @@ -1256,7 +1259,10 @@ def same_config(self, old: "SourceDefinition") -> bool: ) def same_vars(self, other: "SourceDefinition") -> bool: - return self.vars == other.vars + if get_flags().state_modified_compare_vars: + return self.vars == other.vars + else: + return True def same_contents(self, old: Optional["SourceDefinition"]) -> bool: # existing when it didn't before is a change! @@ -1376,7 +1382,10 @@ def same_config(self, old: "Exposure") -> bool: ) def same_vars(self, old: "Exposure") -> bool: - return self.vars == old.vars + if get_flags().state_modified_compare_vars: + return self.vars == old.vars + else: + return True def same_contents(self, old: Optional["Exposure"]) -> bool: # existing when it didn't before is a change! diff --git a/core/dbt/contracts/project.py b/core/dbt/contracts/project.py index e08131ecd8f..198b6c6325b 100644 --- a/core/dbt/contracts/project.py +++ b/core/dbt/contracts/project.py @@ -342,6 +342,7 @@ class ProjectFlags(ExtensibleDbtClassMixin): require_resource_names_without_spaces: bool = False source_freshness_run_project_hooks: bool = False state_modified_compare_more_unrendered_values: bool = False + state_modified_compare_vars: bool = False @property def project_only_flags(self) -> Dict[str, Any]: @@ -350,6 +351,7 @@ def project_only_flags(self) -> Dict[str, Any]: "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, + "state_modified_compare_vars": self.state_modified_compare_vars, } diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 13af62c8e82..2fe52584b51 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1150,6 +1150,9 @@ class TestModifiedVars(BaseModifiedState): @pytest.fixture(scope="class") def project_config_update(self): return { + "flags": { + "state_modified_compare_vars": True, + }, "vars": {"my_var": 1}, } @@ -1198,6 +1201,9 @@ class TestModifiedMacroVars(BaseModifiedState): @pytest.fixture(scope="class") def project_config_update(self): return { + "flags": { + "state_modified_compare_vars": True, + }, "vars": {"my_var": 1}, } @@ -1261,6 +1267,9 @@ class TestModifiedVarsSchemaYml(BaseModifiedState): @pytest.fixture(scope="class") def project_config_update(self): return { + "flags": { + "state_modified_compare_vars": True, + }, "vars": {"my_var": "table"}, } From 7e9ab1cb3a887db47c9eb24cd230ad817e9dbbd8 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 30 Sep 2024 13:22:32 +0200 Subject: [PATCH 10/14] fix unit tests --- tests/unit/contracts/graph/test_nodes.py | 8 ++++++++ tests/unit/contracts/graph/test_nodes_parsed.py | 6 ++++-- tests/unit/graph/test_selector_methods.py | 5 +++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index c22a22176af..efb56c06548 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -1,3 +1,4 @@ +from argparse import Namespace import pickle import re from dataclasses import replace @@ -24,6 +25,13 @@ ) +@pytest.fixture +def args_for_flags() -> Namespace: + return Namespace( + state_modified_compare_vars=False + ) + + def norm_whitespace(string): _RE_COMBINE_WHITESPACE = re.compile(r"\s+") string = _RE_COMBINE_WHITESPACE.sub(" ", string).strip() diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index 27042777525..dc4410da29d 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -65,8 +65,10 @@ @pytest.fixture -def flags_for_args() -> Namespace: - return Namespace(SEND_ANONYMOUS_USAGE_STATS=False) +def args_for_flags() -> Namespace: + return Namespace( + send_anonymous_usage_stats=False, state_modified_compare_vars=False + ) @pytest.fixture diff --git a/tests/unit/graph/test_selector_methods.py b/tests/unit/graph/test_selector_methods.py index 04aebe052d1..1190ae5129d 100644 --- a/tests/unit/graph/test_selector_methods.py +++ b/tests/unit/graph/test_selector_methods.py @@ -1,3 +1,4 @@ +from argparse import Namespace import copy from dataclasses import replace from pathlib import Path @@ -643,6 +644,10 @@ def previous_state(manifest): return create_previous_state(manifest) +@pytest.fixture +def args_for_flags(): + return Namespace(state_modified_compare_vars=False) + def add_node(manifest, node): manifest.nodes[node.unique_id] = node From a8414641b8448b1e24a111234e9a8f44afb0ec98 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 30 Sep 2024 13:27:05 +0200 Subject: [PATCH 11/14] linting --- core/dbt/contracts/files.py | 1 + tests/unit/contracts/graph/test_nodes.py | 6 ++---- tests/unit/contracts/graph/test_nodes_parsed.py | 4 +--- tests/unit/graph/test_selector_methods.py | 3 ++- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/core/dbt/contracts/files.py b/core/dbt/contracts/files.py index 2e8dd73f801..f62ab6ae9b8 100644 --- a/core/dbt/contracts/files.py +++ b/core/dbt/contracts/files.py @@ -353,6 +353,7 @@ def delete_from_unrendered_configs(self, yaml_key, name): if not self.unrendered_configs[yaml_key]: del self.unrendered_configs[yaml_key] + def add_vars(self, vars: Dict[str, Any], yaml_key: str, name: str) -> None: if yaml_key not in self.vars: self.vars[yaml_key] = {} diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index efb56c06548..1271cb7108f 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -1,6 +1,6 @@ -from argparse import Namespace import pickle import re +from argparse import Namespace from dataclasses import replace import pytest @@ -27,9 +27,7 @@ @pytest.fixture def args_for_flags() -> Namespace: - return Namespace( - state_modified_compare_vars=False - ) + return Namespace(state_modified_compare_vars=False) def norm_whitespace(string): diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index dc4410da29d..b89e3fac552 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -66,9 +66,7 @@ @pytest.fixture def args_for_flags() -> Namespace: - return Namespace( - send_anonymous_usage_stats=False, state_modified_compare_vars=False - ) + return Namespace(send_anonymous_usage_stats=False, state_modified_compare_vars=False) @pytest.fixture diff --git a/tests/unit/graph/test_selector_methods.py b/tests/unit/graph/test_selector_methods.py index 1190ae5129d..9447062a1ac 100644 --- a/tests/unit/graph/test_selector_methods.py +++ b/tests/unit/graph/test_selector_methods.py @@ -1,5 +1,5 @@ -from argparse import Namespace import copy +from argparse import Namespace from dataclasses import replace from pathlib import Path from unittest import mock @@ -648,6 +648,7 @@ def previous_state(manifest): def args_for_flags(): return Namespace(state_modified_compare_vars=False) + def add_node(manifest, node): manifest.nodes[node.unique_id] = node From 219118aa647c13e612659b4ce978961f52138872 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 30 Sep 2024 13:46:00 +0200 Subject: [PATCH 12/14] Fix unit test setup --- tests/unit/parser/test_parser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/parser/test_parser.py b/tests/unit/parser/test_parser.py index b9b414ebac2..0d95a495cdf 100644 --- a/tests/unit/parser/test_parser.py +++ b/tests/unit/parser/test_parser.py @@ -463,6 +463,7 @@ def test__parse_basic_source(self): @mock.patch("dbt.parser.sources.get_adapter") def test__parse_basic_source_meta(self, mock_get_adapter): block = self.file_block_for(MULTIPLE_TABLE_SOURCE_META, "test_one.yml") + self.parser.manifest.files[block.file.file_id] = block.file dct = yaml_from_file(block.file) self.parser.parse_file(block, dct) self.assert_has_manifest_lengths(self.parser.manifest, sources=2) From 6e133bd093e250983a707301018b5af8cd15bfac Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 30 Sep 2024 15:13:00 +0200 Subject: [PATCH 13/14] fix artifact test --- tests/functional/artifacts/expected_manifest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 368d6fec87d..8e40d6e35e4 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -814,7 +814,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "unique_id": "source.test.my_source.my_table", "fqn": ["test", "my_source", "my_table"], "unrendered_config": {}, - "vars": {}, + "vars": {"test_schema": ANY}, }, }, "exposures": { @@ -1327,7 +1327,7 @@ def expected_references_manifest(project): "unique_id": "source.test.my_source.my_table", "fqn": ["test", "my_source", "my_table"], "unrendered_config": {}, - "vars": {}, + "vars": {"test_schema": ANY}, }, }, "exposures": { From 093be30177faa30c747bb67801ebe721adc74075 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 30 Sep 2024 15:50:56 +0200 Subject: [PATCH 14/14] move behaviour flag + test legacy behaviour --- core/dbt/contracts/graph/nodes.py | 40 +++++--- schemas/dbt/manifest/v12.json | 96 +++++++++++++++++++ .../defer_state/test_modified_state.py | 53 ++++++++++ 3 files changed, 174 insertions(+), 15 deletions(-) diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 6c589f11cf5..dea8af5bf37 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -370,10 +370,7 @@ def same_contract(self, old, adapter_type=None) -> bool: return True def same_vars(self, old) -> bool: - if get_flags().state_modified_compare_vars: - return self.vars == old.vars - else: - return True + return self.vars == old.vars def same_contents(self, old, adapter_type) -> bool: if old is None: @@ -382,13 +379,20 @@ def same_contents(self, old, adapter_type) -> bool: # Need to ensure that same_contract is called because it # could throw an error same_contract = self.same_contract(old, adapter_type) + + # Legacy behaviour + if not get_flags().state_modified_compare_vars: + same_vars = True + else: + same_vars = self.same_vars(old) + return ( self.same_body(old) and self.same_config(old) and self.same_persisted_description(old) and self.same_fqn(old) and self.same_database_representation(old) - and self.same_vars(old) + and same_vars and same_contract and True ) @@ -1259,10 +1263,7 @@ def same_config(self, old: "SourceDefinition") -> bool: ) def same_vars(self, other: "SourceDefinition") -> bool: - if get_flags().state_modified_compare_vars: - return self.vars == other.vars - else: - return True + return self.vars == other.vars def same_contents(self, old: Optional["SourceDefinition"]) -> bool: # existing when it didn't before is a change! @@ -1277,6 +1278,12 @@ def same_contents(self, old: Optional["SourceDefinition"]) -> bool: # freshness changes are changes, I guess # metadata/tags changes are not "changes" # patching/description changes are not "changes" + # Legacy behaviour + if not get_flags().state_modified_compare_vars: + same_vars = True + else: + same_vars = self.same_vars(old) + return ( self.same_database_representation(old) and self.same_fqn(old) @@ -1284,7 +1291,7 @@ def same_contents(self, old: Optional["SourceDefinition"]) -> bool: and self.same_quoting(old) and self.same_freshness(old) and self.same_external(old) - and self.same_vars(old) + and same_vars and True ) @@ -1382,10 +1389,7 @@ def same_config(self, old: "Exposure") -> bool: ) def same_vars(self, old: "Exposure") -> bool: - if get_flags().state_modified_compare_vars: - return self.vars == old.vars - else: - return True + return self.vars == old.vars def same_contents(self, old: Optional["Exposure"]) -> bool: # existing when it didn't before is a change! @@ -1393,6 +1397,12 @@ def same_contents(self, old: Optional["Exposure"]) -> bool: if old is None: return True + # Legacy behaviour + if not get_flags().state_modified_compare_vars: + same_vars = True + else: + same_vars = self.same_vars(old) + return ( self.same_fqn(old) and self.same_exposure_type(old) @@ -1403,7 +1413,7 @@ def same_contents(self, old: Optional["Exposure"]) -> bool: and self.same_label(old) and self.same_depends_on(old) and self.same_config(old) - and self.same_vars(old) + and same_vars and True ) diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index ea21214b4fc..3527e4d8136 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -706,6 +706,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -1745,6 +1751,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -2399,6 +2411,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -3190,6 +3208,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -4000,6 +4024,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -5361,6 +5391,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -6015,6 +6051,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -6973,6 +7015,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -10570,6 +10618,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -11609,6 +11663,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -12263,6 +12323,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -13054,6 +13120,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -13864,6 +13936,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -15225,6 +15303,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -15879,6 +15963,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { @@ -16837,6 +16927,12 @@ "type": "string" } }, + "unrendered_config_call_dict": { + "type": "object", + "propertyNames": { + "type": "string" + } + }, "relation_name": { "anyOf": [ { diff --git a/tests/functional/defer_state/test_modified_state.py b/tests/functional/defer_state/test_modified_state.py index 2fe52584b51..3d0f0955c93 100644 --- a/tests/functional/defer_state/test_modified_state.py +++ b/tests/functional/defer_state/test_modified_state.py @@ -1146,6 +1146,59 @@ def test_changed_semantic_model_contents(self, project): assert len(results) == 1 +class TestModifiedVarsLegacy(BaseModifiedState): + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": { + "state_modified_compare_vars": False, + }, + "vars": {"my_var": 1}, + } + + @pytest.fixture(scope="class") + def models(self): + return { + "model_with_var.sql": "select {{ var('my_var') }} as id", + } + + def test_changed_vars(self, project): + self.run_and_save_state() + + # No var change + assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"]) + assert not run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) + + # Modify var (my_var: 1 -> 2) + update_config_file({"vars": {"my_var": 2}}, "dbt_project.yml") + + # By default, do not detect vars in state:modified to preserve legacy behaviour + assert run_dbt(["list", "-s", "state:modified", "--state", "./state"]) == [] + + # state:modified.vars is a new selector, opt-in method -> returns results + assert run_dbt(["list", "-s", "state:modified.vars", "--state", "./state"]) == [ + "test.model_with_var" + ] + + # Reset dbt_project.yml + update_config_file({"vars": {"my_var": 1}}, "dbt_project.yml") + + # Modify var via --var CLI flag + assert not run_dbt( + ["list", "--vars", '{"my_var": 1}', "-s", "state:modified", "--state", "./state"] + ) + assert ( + run_dbt( + ["list", "--vars", '{"my_var": 2}', "-s", "state:modified", "--state", "./state"] + ) + == [] + ) + # state:modified.vars is a new selector, opt-in method -> returns results + assert run_dbt( + ["list", "--vars", '{"my_var": 2}', "-s", "state:modified.vars", "--state", "./state"] + ) == ["test.model_with_var"] + + class TestModifiedVars(BaseModifiedState): @pytest.fixture(scope="class") def project_config_update(self):