From 3c558062033e8fde33cbf9f923886edc2842d346 Mon Sep 17 00:00:00 2001 From: aliceliu Date: Fri, 23 Aug 2024 12:22:38 -0700 Subject: [PATCH] Fix state:modified check for exports (#10565) --- .../unreleased/Fixes-20240813-154235.yaml | 6 ++ .../dbt/artifacts/resources/v1/saved_query.py | 1 + core/dbt/contracts/graph/nodes.py | 11 +-- core/dbt/parser/schema_yaml_readers.py | 4 +- schemas/dbt/manifest/v12.json | 18 ++++ .../saved_queries/test_saved_query_parsing.py | 97 +++++++++++++++++++ 6 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240813-154235.yaml diff --git a/.changes/unreleased/Fixes-20240813-154235.yaml b/.changes/unreleased/Fixes-20240813-154235.yaml new file mode 100644 index 00000000000..03c3a3c7cac --- /dev/null +++ b/.changes/unreleased/Fixes-20240813-154235.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix state:modified check for exports +time: 2024-08-13T15:42:35.471685-07:00 +custom: + Author: aliceliu + Issue: "10138" diff --git a/core/dbt/artifacts/resources/v1/saved_query.py b/core/dbt/artifacts/resources/v1/saved_query.py index 2f0e1257a93..1eea7990cc1 100644 --- a/core/dbt/artifacts/resources/v1/saved_query.py +++ b/core/dbt/artifacts/resources/v1/saved_query.py @@ -34,6 +34,7 @@ class Export(dbtClassMixin): name: str config: ExportConfig + unrendered_config: Dict[str, str] = field(default_factory=dict) @dataclass diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 164ce19acd4..4130a4483f7 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -1579,13 +1579,12 @@ def same_exports(self, old: "SavedQuery") -> bool: # exports should be in the same order, so we zip them for easy iteration for old_export, new_export in zip(old.exports, self.exports): - if not ( - old_export.name == new_export.name - and old_export.config.export_as == new_export.config.export_as - and old_export.config.schema_name == new_export.config.schema_name - and old_export.config.alias == new_export.config.alias - ): + if not (old_export.name == new_export.name): return False + keys = ["export_as", "schema", "alias"] + for key in keys: + if old_export.unrendered_config.get(key) != new_export.unrendered_config.get(key): + return False return True diff --git a/core/dbt/parser/schema_yaml_readers.py b/core/dbt/parser/schema_yaml_readers.py index e63d39953c8..dc99e87a218 100644 --- a/core/dbt/parser/schema_yaml_readers.py +++ b/core/dbt/parser/schema_yaml_readers.py @@ -778,7 +778,9 @@ def _get_export( self, unparsed: UnparsedExport, saved_query_config: SavedQueryConfig ) -> Export: return Export( - name=unparsed.name, config=self._get_export_config(unparsed.config, saved_query_config) + name=unparsed.name, + config=self._get_export_config(unparsed.config, saved_query_config), + unrendered_config=unparsed.config, ) def _get_query_params(self, unparsed: UnparsedQueryParams) -> QueryParams: diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 45679b9be96..dfa5744ce70 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -19164,6 +19164,15 @@ "required": [ "export_as" ] + }, + "unrendered_config": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false, @@ -20689,6 +20698,15 @@ "required": [ "export_as" ] + }, + "unrendered_config": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "propertyNames": { + "type": "string" + } } }, "additionalProperties": false, diff --git a/tests/functional/saved_queries/test_saved_query_parsing.py b/tests/functional/saved_queries/test_saved_query_parsing.py index b5333bb770b..be171718a7c 100644 --- a/tests/functional/saved_queries/test_saved_query_parsing.py +++ b/tests/functional/saved_queries/test_saved_query_parsing.py @@ -1,5 +1,6 @@ import os import shutil +from copy import deepcopy from typing import List import pytest @@ -36,6 +37,17 @@ def models(self): "docs.md": saved_query_description, } + @pytest.fixture(scope="class") + def other_schema(self, unique_schema): + return unique_schema + "_other" + + @pytest.fixture(scope="class") + def profiles_config_update(self, dbt_profile_target, unique_schema, other_schema): + outputs = {"default": dbt_profile_target, "prod": deepcopy(dbt_profile_target)} + outputs["default"]["schema"] = unique_schema + outputs["prod"]["schema"] = other_schema + return {"test": {"outputs": outputs, "target": "default"}} + def copy_state(self): if not os.path.exists("state"): os.makedirs("state") @@ -60,6 +72,11 @@ def test_semantic_model_parsing(self, project): assert saved_query.exports[0].config.alias == "my_export_alias" assert saved_query.exports[0].config.export_as == ExportDestinationType.TABLE assert saved_query.exports[0].config.schema_name == "my_export_schema_name" + assert saved_query.exports[0].unrendered_config == { + "alias": "my_export_alias", + "export_as": "table", + "schema": "my_export_schema_name", + } # Save state self.copy_state() @@ -86,6 +103,86 @@ def test_semantic_model_parsing(self, project): results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"]) assert len(results) == 1 + def test_semantic_model_parsing_change_export(self, project, other_schema): + runner = dbtTestRunner() + result = runner.invoke(["parse", "--no-partial-parse"]) + assert result.success + assert isinstance(result.result, Manifest) + manifest = result.result + assert len(manifest.saved_queries) == 1 + saved_query = manifest.saved_queries["saved_query.test.test_saved_query"] + assert saved_query.name == "test_saved_query" + assert saved_query.exports[0].name == "my_export" + + # Save state + self.copy_state() + # Nothing has changed, so no state:modified results + results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"]) + assert len(results) == 0 + + # Change export name + write_file( + saved_queries_yml.replace("name: my_export", "name: my_expor2"), + project.project_root, + "models", + "saved_queries.yml", + ) + # State modified finds changed saved_query + results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"]) + assert len(results) == 1 + + # Change export schema + write_file( + saved_queries_yml.replace( + "schema: my_export_schema_name", "schema: my_export_schema_name2" + ), + project.project_root, + "models", + "saved_queries.yml", + ) + # State modified finds changed saved_query + results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"]) + assert len(results) == 1 + + def test_semantic_model_parsing_with_default_schema(self, project, other_schema): + write_file( + saved_queries_with_defaults_yml, project.project_root, "models", "saved_queries.yml" + ) + runner = dbtTestRunner() + result = runner.invoke(["parse", "--no-partial-parse", "--target", "prod"]) + assert result.success + assert isinstance(result.result, Manifest) + manifest = result.result + assert len(manifest.saved_queries) == 1 + saved_query = manifest.saved_queries["saved_query.test.test_saved_query"] + assert saved_query.name == "test_saved_query" + assert len(saved_query.query_params.metrics) == 1 + assert len(saved_query.query_params.group_by) == 1 + assert len(saved_query.query_params.where.where_filters) == 3 + assert len(saved_query.depends_on.nodes) == 1 + assert saved_query.description == "My SavedQuery Description" + assert len(saved_query.exports) == 1 + assert saved_query.exports[0].name == "my_export" + assert saved_query.exports[0].config.alias == "my_export_alias" + assert saved_query.exports[0].config.export_as == ExportDestinationType.TABLE + assert saved_query.exports[0].config.schema_name == other_schema + assert saved_query.exports[0].unrendered_config == { + "alias": "my_export_alias", + "export_as": "table", + } + + # Save state + self.copy_state() + # Nothing has changed, so no state:modified results + results = run_dbt( + ["ls", "--select", "state:modified", "--state", "./state", "--target", "prod"] + ) + assert len(results) == 0 + + # There should also be no state:modified results when using the default schema + results = run_dbt(["ls", "--select", "state:modified", "--state", "./state"]) + assert len(results) == 0 + def test_saved_query_error(self, project): error_schema_yml = saved_queries_yml.replace("simple_metric", "metric_not_found") write_file(error_schema_yml, project.project_root, "models", "saved_queries.yml")