From dbd206f74841b7df4013845e2ea031cb137e9c42 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Mon, 17 Jul 2023 14:23:14 -0400 Subject: [PATCH 1/6] Update database/schema node config update code to handle target_schema/target_database --- core/dbt/parser/base.py | 20 +++++++++---- tests/functional/simple_snapshot/fixtures.py | 20 +++++++++++++ .../simple_snapshot/test_basic_snapshot.py | 29 +++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 2e8a4663a3e..0624d519f10 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -102,8 +102,7 @@ def __init__(self, config: RuntimeConfig, manifest: Manifest, component: str) -> self.package_updaters = package_updaters self.component = component - def __call__(self, parsed_node: Any, config_dict: Dict[str, Any]) -> None: - override = config_dict.get(self.component) + def __call__(self, parsed_node: Any, override: Optional[str]) -> None: if parsed_node.package_name in self.package_updaters: new_value = self.package_updaters[parsed_node.package_name](override, parsed_node) else: @@ -280,9 +279,18 @@ def update_parsed_node_config_dict( def update_parsed_node_relation_names( self, parsed_node: IntermediateNode, config_dict: Dict[str, Any] ) -> None: - self._update_node_database(parsed_node, config_dict) - self._update_node_schema(parsed_node, config_dict) - self._update_node_alias(parsed_node, config_dict) + + if parsed_node.resource_type == "snapshot": + if "target_database" in config_dict and config_dict["target_database"]: + parsed_node.database = config_dict["target_database"] + if "target_schema" in config_dict and config_dict["target_schema"]: + parsed_node.schema = config_dict["target_schema"] + else: + # These call the RelationUpdate callable to go through generate_name macros + self._update_node_database(parsed_node, config_dict.get("database")) + self._update_node_schema(parsed_node, config_dict.get("schema")) + self._update_node_alias(parsed_node, config_dict.get("alias")) + self._update_node_relation_name(parsed_node) def update_parsed_node_config( @@ -349,7 +357,7 @@ def update_parsed_node_config( # do this once before we parse the node database/schema/alias, so # parsed_node.config is what it would be if they did nothing self.update_parsed_node_config_dict(parsed_node, config_dict) - # This updates the node database/schema/alias + # This updates the node database/schema/alias/relation_name self.update_parsed_node_relation_names(parsed_node, config_dict) # tests don't have hooks diff --git a/tests/functional/simple_snapshot/fixtures.py b/tests/functional/simple_snapshot/fixtures.py index 633a6be4efa..5a0a545f43d 100644 --- a/tests/functional/simple_snapshot/fixtures.py +++ b/tests/functional/simple_snapshot/fixtures.py @@ -281,6 +281,26 @@ {% endsnapshot %} """ +snapshots_pg__snapshot_no_target_schema_sql = """ +{% snapshot snapshot_actual %} + + {{ + config( + target_database=var('target_database', database), + unique_key='id || ' ~ "'-'" ~ ' || first_name', + strategy='timestamp', + updated_at='updated_at', + ) + }} + + {% if var('invalidate_hard_deletes', 'false') | as_bool %} + {{ config(invalidate_hard_deletes=True) }} + {% endif %} + + select * from {{target.database}}.{{target.schema}}.seed + +{% endsnapshot %} +""" models_slow__gen_sql = """ diff --git a/tests/functional/simple_snapshot/test_basic_snapshot.py b/tests/functional/simple_snapshot/test_basic_snapshot.py index d6aec68ab6c..8d627624415 100644 --- a/tests/functional/simple_snapshot/test_basic_snapshot.py +++ b/tests/functional/simple_snapshot/test_basic_snapshot.py @@ -9,6 +9,7 @@ seeds__seed_newcol_csv, seeds__seed_csv, snapshots_pg__snapshot_sql, + snapshots_pg__snapshot_no_target_schema_sql, macros__test_no_overlaps_sql, macros_custom_snapshot__custom_sql, snapshots_pg_custom_namespaced__snapshot_sql, @@ -123,6 +124,34 @@ def test_basic_ref(self, project): ref_setup(project, num_snapshot_models=1) +class TestBasicTargetSchemaConfig(Basic): + @pytest.fixture(scope="class") + def snapshots(self): + return {"snapshot.sql": snapshots_pg__snapshot_no_target_schema_sql} + + @pytest.fixture(scope="class") + def project_config_update(self, unique_schema): + return { + "snapshots": { + "test": { + "target_schema": unique_schema + "_alt", + } + } + } + + def test_target_schema(self, project): + manifest = run_dbt(["parse"]) + print(f"\n\n--- node keys: {manifest.nodes.keys()}") + assert len(manifest.nodes) == 5 + snapshot_id = "snapshot.test.snapshot_actual" + snapshot_node = manifest.nodes[snapshot_id] + assert snapshot_node.schema == f"{project.test_schema}_alt" + assert ( + snapshot_node.relation_name + == f'"{project.database}"."{project.test_schema}_alt"."snapshot_actual"' + ) + + class CustomNamespace: @pytest.fixture(scope="class") def snapshots(self): From 3fd6d5fcc13955649b37940c5b8a0adfb320eab6 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Mon, 17 Jul 2023 16:07:03 -0400 Subject: [PATCH 2/6] Changie --- .changes/unreleased/Fixes-20230717-160652.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20230717-160652.yaml diff --git a/.changes/unreleased/Fixes-20230717-160652.yaml b/.changes/unreleased/Fixes-20230717-160652.yaml new file mode 100644 index 00000000000..8c63a584db9 --- /dev/null +++ b/.changes/unreleased/Fixes-20230717-160652.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Copy target_schema from config into snapshot node +time: 2023-07-17T16:06:52.957724-04:00 +custom: + Author: gshank + Issue: "6745" From 80d62673dfab48d8cdcee2b55b81a540d7c1a6f8 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Mon, 17 Jul 2023 16:44:02 -0400 Subject: [PATCH 3/6] Update test --- tests/functional/simple_snapshot/fixtures.py | 12 ++++++++++++ .../simple_snapshot/test_basic_snapshot.py | 12 ++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/functional/simple_snapshot/fixtures.py b/tests/functional/simple_snapshot/fixtures.py index 5a0a545f43d..6b3ecc2b101 100644 --- a/tests/functional/simple_snapshot/fixtures.py +++ b/tests/functional/simple_snapshot/fixtures.py @@ -96,6 +96,18 @@ owner: 'a_owner' """ +models__schema_with_target_schema_yml = """ +version: 2 +snapshots: + - name: snapshot_actual + tests: + - mutually_exclusive_ranges + config: + meta: + owner: 'a_owner' + target_schema: schema_from_schema_yml +""" + models__ref_snapshot_sql = """ select * from {{ ref('snapshot_actual') }} """ diff --git a/tests/functional/simple_snapshot/test_basic_snapshot.py b/tests/functional/simple_snapshot/test_basic_snapshot.py index 8d627624415..ff4799f10ab 100644 --- a/tests/functional/simple_snapshot/test_basic_snapshot.py +++ b/tests/functional/simple_snapshot/test_basic_snapshot.py @@ -2,9 +2,10 @@ from datetime import datetime import pytz import pytest -from dbt.tests.util import run_dbt, check_relations_equal, relation_from_name +from dbt.tests.util import run_dbt, check_relations_equal, relation_from_name, write_file from tests.functional.simple_snapshot.fixtures import ( models__schema_yml, + models__schema_with_target_schema_yml, models__ref_snapshot_sql, seeds__seed_newcol_csv, seeds__seed_csv, @@ -141,8 +142,8 @@ def project_config_update(self, unique_schema): def test_target_schema(self, project): manifest = run_dbt(["parse"]) - print(f"\n\n--- node keys: {manifest.nodes.keys()}") assert len(manifest.nodes) == 5 + # ensure that the schema in the snapshot node is the same as target_schema snapshot_id = "snapshot.test.snapshot_actual" snapshot_node = manifest.nodes[snapshot_id] assert snapshot_node.schema == f"{project.test_schema}_alt" @@ -150,6 +151,13 @@ def test_target_schema(self, project): snapshot_node.relation_name == f'"{project.database}"."{project.test_schema}_alt"."snapshot_actual"' ) + assert snapshot_node.meta == {"owner": "a_owner"} + + # write out schema.yml file and check again + write_file(models__schema_with_target_schema_yml, "models", "schema.yml") + manifest = run_dbt(["parse"]) + snapshot_node = manifest.nodes[snapshot_id] + assert snapshot_node.schema == "schema_from_schema_yml" class CustomNamespace: From 6ae9994df2aa0bc2b579a0c3fd6395f6b00337d0 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 18 Jul 2023 10:46:28 -0400 Subject: [PATCH 4/6] a couple of comments --- core/dbt/contracts/graph/model_config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index 2b9947d85d2..b69b2500d63 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -619,6 +619,8 @@ class SnapshotConfig(EmptySnapshotConfig): @classmethod def validate(cls, data): super().validate(data) + # Note: currently you can't just set these keys in schema.yml because this validation + # will fail when parsing the snapshot node. if not data.get("strategy") or not data.get("unique_key") or not data.get("target_schema"): raise ValidationError( "Snapshots must be configured with a 'strategy', 'unique_key', " @@ -649,6 +651,7 @@ def validate(cls, data): if data.get("materialized") and data.get("materialized") != "snapshot": raise ValidationError("A snapshot must have a materialized value of 'snapshot'") + # Called by "calculate_node_config_dict" in ContextConfigGenerator def finalize_and_validate(self): data = self.to_dict(omit_none=True) self.validate(data) From 64b86f8750bdc272d073a3a13d787f5884c57264 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Wed, 26 Jul 2023 17:30:44 -0400 Subject: [PATCH 5/6] Use NodeType.Snapshot --- core/dbt/parser/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 0624d519f10..9ea76375019 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -280,7 +280,7 @@ def update_parsed_node_relation_names( self, parsed_node: IntermediateNode, config_dict: Dict[str, Any] ) -> None: - if parsed_node.resource_type == "snapshot": + if parsed_node.resource_type == NodeType.Snapshot: if "target_database" in config_dict and config_dict["target_database"]: parsed_node.database = config_dict["target_database"] if "target_schema" in config_dict and config_dict["target_schema"]: From fa45019dbe84dfecfa0e14f9cc22a55f415a05c1 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 27 Jul 2023 13:10:30 -0400 Subject: [PATCH 6/6] Rearrange code in update_parsed_node_relation_names --- core/dbt/parser/base.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index 9ea76375019..b24cd4712d4 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -280,16 +280,17 @@ def update_parsed_node_relation_names( self, parsed_node: IntermediateNode, config_dict: Dict[str, Any] ) -> None: + # These call the RelationUpdate callable to go through generate_name macros + self._update_node_database(parsed_node, config_dict.get("database")) + self._update_node_schema(parsed_node, config_dict.get("schema")) + self._update_node_alias(parsed_node, config_dict.get("alias")) + + # Snapshot nodes use special "target_database" and "target_schema" fields for some reason if parsed_node.resource_type == NodeType.Snapshot: if "target_database" in config_dict and config_dict["target_database"]: parsed_node.database = config_dict["target_database"] if "target_schema" in config_dict and config_dict["target_schema"]: parsed_node.schema = config_dict["target_schema"] - else: - # These call the RelationUpdate callable to go through generate_name macros - self._update_node_database(parsed_node, config_dict.get("database")) - self._update_node_schema(parsed_node, config_dict.get("schema")) - self._update_node_alias(parsed_node, config_dict.get("alias")) self._update_node_relation_name(parsed_node)