From 032b5b586ae5cd3e22b0559c546fb77bec7ac6a6 Mon Sep 17 00:00:00 2001 From: Matthew McKnight Date: Fri, 14 Apr 2023 15:33:14 -0500 Subject: [PATCH 1/6] update RELEASE_BRANCH env --- .github/workflows/nightly-release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/nightly-release.yml b/.github/workflows/nightly-release.yml index 54c5fdc69..4762d1218 100644 --- a/.github/workflows/nightly-release.yml +++ b/.github/workflows/nightly-release.yml @@ -26,7 +26,7 @@ defaults: shell: bash env: - RELEASE_BRANCH: "1.4.latest" + RELEASE_BRANCH: "1.5.latest" jobs: aggregate-release-data: From 18da2c6e39056a41406adb6b420fdccbf8e10bde Mon Sep 17 00:00:00 2001 From: Matthew McKnight Date: Wed, 9 Aug 2023 11:14:51 -0500 Subject: [PATCH 2/6] initial push for ADAP-631 to convert target_lag to a str type --- dbt/adapters/snowflake/relation.py | 6 +- .../snowflake/relation_configs/__init__.py | 5 - .../relation_configs/dynamic_table.py | 30 +---- .../snowflake/relation_configs/target_lag.py | 111 ------------------ 4 files changed, 6 insertions(+), 146 deletions(-) delete mode 100644 dbt/adapters/snowflake/relation_configs/target_lag.py diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 9d09e96b4..108786365 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -9,7 +9,6 @@ from dbt.adapters.snowflake.relation_configs import ( SnowflakeDynamicTableConfig, SnowflakeDynamicTableConfigChangeset, - SnowflakeDynamicTableTargetLagConfigChange, SnowflakeDynamicTableWarehouseConfigChange, SnowflakeQuotePolicy, SnowflakeRelationType, @@ -45,10 +44,7 @@ def dynamic_table_config_changeset( config_change_collection = SnowflakeDynamicTableConfigChangeset() if new_dynamic_table.target_lag != existing_dynamic_table.target_lag: - config_change_collection.target_lag = SnowflakeDynamicTableTargetLagConfigChange( - action=RelationConfigChangeAction.alter, - context=new_dynamic_table.target_lag, - ) + config_change_collection.target_lag = new_dynamic_table.target_lag if new_dynamic_table.warehouse != existing_dynamic_table.warehouse: config_change_collection.warehouse = SnowflakeDynamicTableWarehouseConfigChange( diff --git a/dbt/adapters/snowflake/relation_configs/__init__.py b/dbt/adapters/snowflake/relation_configs/__init__.py index e53604526..7718639d5 100644 --- a/dbt/adapters/snowflake/relation_configs/__init__.py +++ b/dbt/adapters/snowflake/relation_configs/__init__.py @@ -8,8 +8,3 @@ SnowflakeQuotePolicy, SnowflakeRelationType, ) -from dbt.adapters.snowflake.relation_configs.target_lag import ( - SnowflakeDynamicTableTargetLagConfig, - SnowflakeDynamicTableTargetLagConfigChange, - SnowflakeDynamicTableTargetLagPeriod, -) diff --git a/dbt/adapters/snowflake/relation_configs/dynamic_table.py b/dbt/adapters/snowflake/relation_configs/dynamic_table.py index 2b385155f..e6db73528 100644 --- a/dbt/adapters/snowflake/relation_configs/dynamic_table.py +++ b/dbt/adapters/snowflake/relation_configs/dynamic_table.py @@ -7,10 +7,6 @@ from dbt.contracts.relation import ComponentName from dbt.adapters.snowflake.relation_configs.base import SnowflakeRelationConfigBase -from dbt.adapters.snowflake.relation_configs.target_lag import ( - SnowflakeDynamicTableTargetLagConfig, - SnowflakeDynamicTableTargetLagConfigChange, -) @dataclass(frozen=True, eq=True, unsafe_hash=True) @@ -32,7 +28,7 @@ class SnowflakeDynamicTableConfig(SnowflakeRelationConfigBase): schema_name: str database_name: str query: str - target_lag: SnowflakeDynamicTableTargetLagConfig + target_lag: str warehouse: str @classmethod @@ -45,13 +41,9 @@ def from_dict(cls, config_dict) -> "SnowflakeDynamicTableConfig": ), "query": config_dict.get("query"), "warehouse": config_dict.get("warehouse"), + "target_lag": config_dict.get("target_lag"), } - if target_lag := config_dict.get("target_lag"): - kwargs_dict.update( - {"target_lag": SnowflakeDynamicTableTargetLagConfig.from_dict(target_lag)} - ) - dynamic_table: "SnowflakeDynamicTableConfig" = super().from_dict(kwargs_dict) # type: ignore return dynamic_table @@ -63,13 +55,9 @@ def parse_model_node(cls, model_node: ModelNode) -> dict: "database_name": model_node.database, "query": model_node.compiled_code, "warehouse": model_node.config.extra.get("snowflake_warehouse"), + "target_lag": model_node.config.extra.get("target_lag"), } - if model_node.config.extra.get("target_lag"): - config_dict.update( - {"target_lag": SnowflakeDynamicTableTargetLagConfig.parse_model_node(model_node)} - ) - return config_dict @classmethod @@ -82,17 +70,9 @@ def parse_relation_results(cls, relation_results: RelationResults) -> dict: "database_name": dynamic_table.get("database_name"), "query": dynamic_table.get("text"), "warehouse": dynamic_table.get("warehouse"), + "target_lag": dynamic_table.get("target_lag"), } - if dynamic_table.get("target_lag"): - config_dict.update( - { - "target_lag": SnowflakeDynamicTableTargetLagConfig.parse_relation_results( - dynamic_table - ) - } - ) - return config_dict @@ -107,7 +87,7 @@ def requires_full_refresh(self) -> bool: @dataclass class SnowflakeDynamicTableConfigChangeset: - target_lag: Optional[SnowflakeDynamicTableTargetLagConfigChange] = None + target_lag: Optional[SnowflakeDynamicTableWarehouseConfigChange] = None warehouse: Optional[SnowflakeDynamicTableWarehouseConfigChange] = None @property diff --git a/dbt/adapters/snowflake/relation_configs/target_lag.py b/dbt/adapters/snowflake/relation_configs/target_lag.py deleted file mode 100644 index 9744e3b00..000000000 --- a/dbt/adapters/snowflake/relation_configs/target_lag.py +++ /dev/null @@ -1,111 +0,0 @@ -from dataclasses import dataclass -from typing import Any, Dict, Optional, Union - -import agate -from dbt.adapters.relation_configs import RelationConfigChange -from dbt.contracts.graph.nodes import ModelNode -from dbt.dataclass_schema import StrEnum - -from dbt.adapters.snowflake.relation_configs.base import SnowflakeRelationConfigBase - - -class SnowflakeDynamicTableTargetLagPeriod(StrEnum): - seconds = "seconds" - minutes = "minutes" - hours = "hours" - days = "days" - second = "second" - minute = "minute" - hour = "hour" - day = "day" - - -@dataclass(frozen=True, eq=True, unsafe_hash=True) -class SnowflakeDynamicTableTargetLagConfig(SnowflakeRelationConfigBase): - """ - This config follow the specs found here: - https://docs.snowflake.com/en/sql-reference/sql/create-dynamic-table - - The following parameters are configurable by dbt: - - duration: the numeric part of the lag - - period: the scale part of the lag - - There are currently no non-configurable parameters. - """ - - duration: int - period: SnowflakeDynamicTableTargetLagPeriod - - def __str__(self) -> str: - return f"{self.duration} {self.period}" - - @classmethod - def from_dict(cls, config_dict) -> "SnowflakeDynamicTableTargetLagConfig": - kwargs_dict: Dict[str, Union[int, SnowflakeDynamicTableTargetLagPeriod]] = {} - - if duration := config_dict.get("duration"): - kwargs_dict.update({"duration": int(duration)}) - - if period := config_dict.get("period"): - kwargs_dict.update({"period": SnowflakeDynamicTableTargetLagPeriod(period)}) - - target_lag: "SnowflakeDynamicTableTargetLagConfig" = super().from_dict(kwargs_dict) # type: ignore - return target_lag - - @classmethod - def parse_model_node(cls, model_node: ModelNode) -> Dict[str, Any]: - """ - Translate ModelNode objects from the user-provided config into a standard dictionary. - - Args: - model_node: the description of the target lag from the user in this format: - - { - "target_lag": "int any("seconds", "minutes", "hours", "days")" - } - - Returns: a standard dictionary describing this `SnowflakeDynamicTableTargetLagConfig` instance - """ - target_lag: str = model_node.config.extra["target_lag"] - return cls._parse_target_lag_string(target_lag) - - @classmethod - def parse_relation_results(cls, relation_results_entry: agate.Row) -> Dict[str, Any]: - """ - Translate agate objects from the database into a standard dictionary. - - Args: - relation_results_entry: the description of the target lag from the database in this format: - - agate.Row({ - "target_lag": "int any("seconds", "minutes", "hours", "days")" - }) - - Returns: a standard dictionary describing this `SnowflakeDynamicTableTargetLagConfig` instance - """ - target_lag: str = relation_results_entry["target_lag"] - return cls._parse_target_lag_string(target_lag) - - @staticmethod - def _parse_target_lag_string(target_lag: str) -> Dict[str, Union[Optional[Union[int, str]]]]: - try: - # Snowflake supports strings like `1 \n minutes` despite the docs not suggesting that - duration_str, *_, period = target_lag.split(" ") - duration = int(duration_str) - except (AttributeError, IndexError): - duration, period = None, None - - config_dict = { - "duration": duration, - "period": period, - } - return config_dict - - -@dataclass(frozen=True, eq=True, unsafe_hash=True) -class SnowflakeDynamicTableTargetLagConfigChange(RelationConfigChange): - context: Optional[SnowflakeDynamicTableTargetLagConfig] = None - - @property - def requires_full_refresh(self) -> bool: - return False From d0307b5cd69be35c5c635e07ee11667b878f4a55 Mon Sep 17 00:00:00 2001 From: Matthew McKnight Date: Wed, 9 Aug 2023 13:56:16 -0500 Subject: [PATCH 3/6] readd SnowflakeDynamicTableTargetLagConfigChange class as part of dynamic_table.py --- dbt/adapters/snowflake/relation.py | 6 +++++- dbt/adapters/snowflake/relation_configs/__init__.py | 1 + .../snowflake/relation_configs/dynamic_table.py | 11 ++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 108786365..9d09e96b4 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -9,6 +9,7 @@ from dbt.adapters.snowflake.relation_configs import ( SnowflakeDynamicTableConfig, SnowflakeDynamicTableConfigChangeset, + SnowflakeDynamicTableTargetLagConfigChange, SnowflakeDynamicTableWarehouseConfigChange, SnowflakeQuotePolicy, SnowflakeRelationType, @@ -44,7 +45,10 @@ def dynamic_table_config_changeset( config_change_collection = SnowflakeDynamicTableConfigChangeset() if new_dynamic_table.target_lag != existing_dynamic_table.target_lag: - config_change_collection.target_lag = new_dynamic_table.target_lag + config_change_collection.target_lag = SnowflakeDynamicTableTargetLagConfigChange( + action=RelationConfigChangeAction.alter, + context=new_dynamic_table.target_lag, + ) if new_dynamic_table.warehouse != existing_dynamic_table.warehouse: config_change_collection.warehouse = SnowflakeDynamicTableWarehouseConfigChange( diff --git a/dbt/adapters/snowflake/relation_configs/__init__.py b/dbt/adapters/snowflake/relation_configs/__init__.py index 7718639d5..e5ceabe49 100644 --- a/dbt/adapters/snowflake/relation_configs/__init__.py +++ b/dbt/adapters/snowflake/relation_configs/__init__.py @@ -2,6 +2,7 @@ SnowflakeDynamicTableConfig, SnowflakeDynamicTableConfigChangeset, SnowflakeDynamicTableWarehouseConfigChange, + SnowflakeDynamicTableTargetLagConfigChange, ) from dbt.adapters.snowflake.relation_configs.policies import ( SnowflakeIncludePolicy, diff --git a/dbt/adapters/snowflake/relation_configs/dynamic_table.py b/dbt/adapters/snowflake/relation_configs/dynamic_table.py index e6db73528..053e236ba 100644 --- a/dbt/adapters/snowflake/relation_configs/dynamic_table.py +++ b/dbt/adapters/snowflake/relation_configs/dynamic_table.py @@ -76,6 +76,15 @@ def parse_relation_results(cls, relation_results: RelationResults) -> dict: return config_dict +@dataclass(frozen=True, eq=True, unsafe_hash=True) +class SnowflakeDynamicTableTargetLagConfigChange(RelationConfigChange): + context: Optional[str] = None + + @property + def requires_full_refresh(self) -> bool: + return False + + @dataclass(frozen=True, eq=True, unsafe_hash=True) class SnowflakeDynamicTableWarehouseConfigChange(RelationConfigChange): context: Optional[str] = None @@ -87,7 +96,7 @@ def requires_full_refresh(self) -> bool: @dataclass class SnowflakeDynamicTableConfigChangeset: - target_lag: Optional[SnowflakeDynamicTableWarehouseConfigChange] = None + target_lag: Optional[SnowflakeDynamicTableTargetLagConfigChange] = None warehouse: Optional[SnowflakeDynamicTableWarehouseConfigChange] = None @property From 207a36b40e440e6e2cd112f4ca78fdf7c19ce814 Mon Sep 17 00:00:00 2001 From: Matthew McKnight Date: Thu, 10 Aug 2023 15:46:27 -0500 Subject: [PATCH 4/6] pull in ADAP-774 and add changelog --- .changes/unreleased/Fixes-20230810-154613.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20230810-154613.yaml diff --git a/.changes/unreleased/Fixes-20230810-154613.yaml b/.changes/unreleased/Fixes-20230810-154613.yaml new file mode 100644 index 000000000..e1794e899 --- /dev/null +++ b/.changes/unreleased/Fixes-20230810-154613.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: change target_lag type to allow for downstream as a option +time: 2023-08-10T15:46:13.896057-05:00 +custom: + Author: McKnight-42 + Issue: "734" From 393a21f48b895b1eee244093745db1269264a415 Mon Sep 17 00:00:00 2001 From: Matthew McKnight Date: Thu, 10 Aug 2023 16:33:19 -0500 Subject: [PATCH 5/6] add missing changelog entry for pr 727 --- .changes/unreleased/Fixes-20230810-163232.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20230810-163232.yaml diff --git a/.changes/unreleased/Fixes-20230810-163232.yaml b/.changes/unreleased/Fixes-20230810-163232.yaml new file mode 100644 index 000000000..80643d93f --- /dev/null +++ b/.changes/unreleased/Fixes-20230810-163232.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: update snowflake_warehouse field for dynamic tables to be more accounted for +time: 2023-08-10T16:32:32.417917-05:00 +custom: + Author: McKnight-42 + Issue: "735" From 0f83b8a734e40376aba35bc1d355d8a1cb461a17 Mon Sep 17 00:00:00 2001 From: Matthew McKnight Date: Mon, 14 Aug 2023 11:41:06 -0500 Subject: [PATCH 6/6] update to main, and add basic test case for passing downstream via alter --- .../test_dynamic_tables_changes.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/functional/adapter/dynamic_table_tests/test_dynamic_tables_changes.py b/tests/functional/adapter/dynamic_table_tests/test_dynamic_tables_changes.py index 5d206dc9c..84fa22a0c 100644 --- a/tests/functional/adapter/dynamic_table_tests/test_dynamic_tables_changes.py +++ b/tests/functional/adapter/dynamic_table_tests/test_dynamic_tables_changes.py @@ -43,12 +43,26 @@ def change_config_via_alter(project, dynamic_table): ) set_model_file(project, dynamic_table, new_model) + @staticmethod + def change_config_via_alter_downstream(project, dynamic_table): + initial_model = get_model_file(project, dynamic_table) + new_model = initial_model.replace( + "target_lag='120 seconds'", "target_lag='downstream'" + ) + set_model_file(project, dynamic_table, new_model) + @staticmethod def check_state_alter_change_is_applied(adapter, dynamic_table): # see above assert query_target_lag(adapter, dynamic_table) == "5 minutes" assert query_warehouse(adapter, dynamic_table) == "DBT_TESTING" + @staticmethod + def check_state_alter_change_is_applied_downstream(adapter, dynamic_table): + # see above + assert query_target_lag(adapter, dynamic_table) == "downstream" + assert query_warehouse(adapter, dynamic_table) == "DBT_TESTING" + @staticmethod def change_config_via_replace(project, dynamic_table): # dbt-snowflake does not currently monitor any changes that trigger a full refresh @@ -127,6 +141,20 @@ def test_change_is_applied_via_alter(self, project, adapter, my_dynamic_table): assert_message_in_logs(f"Applying ALTER to: {my_dynamic_table}", logs) assert_message_in_logs(f"Applying REPLACE to: {my_dynamic_table}", logs, False) + def test_change_is_applied_via_alter_downstream(self, project, adapter, my_dynamic_table): + """ + See above about the two commented assertions. In the meantime, these have been validated manually. + """ + # self.check_start_state(adapter, my_dynamic_table) + + self.change_config_via_alter_downstream(project, my_dynamic_table) + _, logs = run_dbt_and_capture(["--debug", "run", "--models", my_dynamic_table.name]) + + # self.check_state_alter_change_is_applied_downstream(adapter, my_dynamic_table) + + assert_message_in_logs(f"Applying ALTER to: {my_dynamic_table}", logs) + assert_message_in_logs(f"Applying REPLACE to: {my_dynamic_table}", logs, False) + @pytest.mark.skip( "dbt-snowflake does not currently monitor any changes the trigger a full refresh" )