From cc1388885c596a251489e65d6a8f97a0985a000e Mon Sep 17 00:00:00 2001 From: Colin Rogers <111200756+colin-rogers-dbt@users.noreply.github.com> Date: Wed, 5 Jun 2024 19:48:17 -0700 Subject: [PATCH 1/2] replace underscores with hyphens in account ids (#1067) * replace underscores with hyphens in account ids * fix unit test fixtures * add changie --- .../unreleased/Features-20240604-154856.yaml | 6 ++++ dbt/adapters/snowflake/connections.py | 1 + tests/unit/test_connections.py | 13 +++++++ tests/unit/test_snowflake_adapter.py | 36 +++++++++---------- 4 files changed, 38 insertions(+), 18 deletions(-) create mode 100644 .changes/unreleased/Features-20240604-154856.yaml diff --git a/.changes/unreleased/Features-20240604-154856.yaml b/.changes/unreleased/Features-20240604-154856.yaml new file mode 100644 index 000000000..7d83b1da7 --- /dev/null +++ b/.changes/unreleased/Features-20240604-154856.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Replace underscores with hyphens in account IDs to prevent SSL issues +time: 2024-06-04T15:48:56.845374-07:00 +custom: + Author: colin-rogers-dbt + Issue: "1068" diff --git a/dbt/adapters/snowflake/connections.py b/dbt/adapters/snowflake/connections.py index 1d6e31c93..aca115b4b 100644 --- a/dbt/adapters/snowflake/connections.py +++ b/dbt/adapters/snowflake/connections.py @@ -105,6 +105,7 @@ def __post_init__(self): base_msg="Authenticator is not set to oauth, but an oauth-only parameter is set! Did you mean to set authenticator: oauth?" ) ) + self.account = self.account.replace("_", "-") @property def type(self): diff --git a/tests/unit/test_connections.py b/tests/unit/test_connections.py index 555091c57..dd452b3cb 100644 --- a/tests/unit/test_connections.py +++ b/tests/unit/test_connections.py @@ -23,3 +23,16 @@ def test_connections_does_not_set_logs_in_response_to_env_var(monkeypatch): assert log_mock.debug.call_count == 0 assert log_mock.set_adapter_dependency_log_level.call_count == 0 + + +def test_connnections_credentials_replaces_underscores_with_hyphens(): + credentials = { + "account": "account_id_with_underscores", + "user": "user", + "password": "password", + "database": "database", + "warehouse": "warehouse", + "schema": "schema", + } + creds = connections.SnowflakeCredentials(**credentials) + assert creds.account == "account-id-with-underscores" diff --git a/tests/unit/test_snowflake_adapter.py b/tests/unit/test_snowflake_adapter.py index 2666e7557..ff92b9b65 100644 --- a/tests/unit/test_snowflake_adapter.py +++ b/tests/unit/test_snowflake_adapter.py @@ -278,7 +278,7 @@ def test_client_session_keep_alive_false_by_default(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -305,7 +305,7 @@ def test_client_session_keep_alive_true(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=True, database="test_database", @@ -332,7 +332,7 @@ def test_client_has_query_tag(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -366,7 +366,7 @@ def test_user_pass_authentication(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -397,7 +397,7 @@ def test_authenticator_user_pass_authentication(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -428,7 +428,7 @@ def test_authenticator_externalbrowser_authentication(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -461,7 +461,7 @@ def test_authenticator_oauth_authentication(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -499,7 +499,7 @@ def test_authenticator_private_key_authentication(self, mock_get_private_key): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -533,7 +533,7 @@ def test_authenticator_private_key_authentication_no_passphrase(self, mock_get_p self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -562,7 +562,7 @@ def test_query_tag(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -592,7 +592,7 @@ def test_reuse_connections_with_keep_alive(self): self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=True, database="test_database", @@ -626,7 +626,7 @@ def test_authenticator_private_key_string_authentication(self, mock_get_private_ self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -662,7 +662,7 @@ def test_authenticator_private_key_string_authentication_no_passphrase( self.snowflake.assert_has_calls( [ mock.call( - account="test_account", + account="test-account", autocommit=True, client_session_keep_alive=False, database="test_database", @@ -907,7 +907,7 @@ class TestSnowflakeAdapterCredentials(unittest.TestCase): def test_private_key_string(self): creds = SnowflakeCredentials( - account="test_account", + account="test-account", user="test_user", database="test_database", schema="public", @@ -917,7 +917,7 @@ def test_private_key_string(self): def test_private_key_string_encrypted(self): creds = SnowflakeCredentials( - account="test_account", + account="test-account", user="test_user", database="test_database", schema="public", @@ -928,7 +928,7 @@ def test_private_key_string_encrypted(self): def test_malformed_private_key_string(self): creds = SnowflakeCredentials( - account="test_account", + account="test-account", user="test_user", database="test_database", schema="public", @@ -938,7 +938,7 @@ def test_malformed_private_key_string(self): def test_invalid_private_key_string(self): creds = SnowflakeCredentials( - account="test_account", + account="test-account", user="test_user", database="test_database", schema="public", @@ -948,7 +948,7 @@ def test_invalid_private_key_string(self): def test_invalid_private_key_path(self): creds = SnowflakeCredentials( - account="test_account", + account="test-account", user="test_user", database="test_database", schema="public", From 57285d57d910b965dc0c769398d1c83e0f537bcc Mon Sep 17 00:00:00 2001 From: Matthew McKnight <91097623+McKnight-42@users.noreply.github.com> Date: Fri, 7 Jun 2024 16:43:00 -0500 Subject: [PATCH 2/2] quoting config not working with 1.8 (#1075) * revert change for quoting policy issue * make revet in both places * update format to ignore identifier name * update test to use SnowflakeRelation and render out the schema_relation, update macros to have condtion to check based on type * remove unneeded comments from updating test * add test based on quoting schema names * update name to more generic my_model --- .../unreleased/Fixes-20240607-102708.yaml | 6 +++++ dbt/include/snowflake/macros/adapters.sql | 27 +++++++++++++------ .../list_relations_tests/test_pagination.py | 16 +++++------ .../test_special_characters.py | 25 +++++++++++++++++ 4 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240607-102708.yaml create mode 100644 tests/functional/adapter/list_relations_tests/test_special_characters.py diff --git a/.changes/unreleased/Fixes-20240607-102708.yaml b/.changes/unreleased/Fixes-20240607-102708.yaml new file mode 100644 index 000000000..58cd9bbee --- /dev/null +++ b/.changes/unreleased/Fixes-20240607-102708.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: return to previous naming convention to return to quoting policy +time: 2024-06-07T10:27:08.542159-05:00 +custom: + Author: McKnight-42 + Issue: "1074" diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index 0bf7b7d1b..6e7ea8f6c 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -72,9 +72,15 @@ {% for _ in range(0, max_iter) %} - {%- set paginated_sql -%} - show objects in {{ schema_relation.database }}.{{ schema_relation.schema }} limit {{ max_results_per_iter }} from '{{ watermark.table_name }}' - {%- endset -%} + {% if schema_relation is string %} + {%- set paginated_sql -%} + show objects in {{ schema_relation }} limit {{ max_results_per_iter }} from '{{ watermark.table_name }}' + {%- endset -%} + {% else %} + {%- set paginated_sql -%} + show objects in {{ schema_relation.include(identifier=False) }} limit {{ max_results_per_iter }} from '{{ watermark.table_name }}' + {%- endset -%} + {% endif -%} {%- set paginated_result = run_query(paginated_sql) %} {%- set paginated_n = (paginated_result | length) -%} @@ -96,7 +102,7 @@ {%- if loop.index == max_iter -%} {%- set msg -%} - dbt will list a maximum of {{ max_total_results }} objects in schema {{ schema_relation.database }}.{{ schema_relation.schema }}. + dbt will list a maximum of {{ max_total_results }} objects in schema {{ schema_relation }}. Your schema exceeds this limit. Please contact support@getdbt.com for troubleshooting tips, or review and reduce the number of objects contained. {%- endset -%} @@ -122,10 +128,15 @@ {% macro snowflake__list_relations_without_caching(schema_relation, max_iter=10, max_results_per_iter=10000) %} {%- set max_total_results = max_results_per_iter * max_iter -%} - - {%- set sql -%} - show objects in {{ schema_relation.database }}.{{ schema_relation.schema }} limit {{ max_results_per_iter }} - {%- endset -%} + {% if schema_relation is string %} + {%- set sql -%} + show objects in {{ schema_relation }} limit {{ max_results_per_iter }} + {%- endset -%} + {% else %} + {%- set sql -%} + show objects in {{ schema_relation.include(identifier=False) }} limit {{ max_results_per_iter }} + {%- endset -%} + {% endif -%} {%- set result = run_query(sql) -%} diff --git a/tests/functional/adapter/list_relations_tests/test_pagination.py b/tests/functional/adapter/list_relations_tests/test_pagination.py index 8f14a0012..407f9c501 100644 --- a/tests/functional/adapter/list_relations_tests/test_pagination.py +++ b/tests/functional/adapter/list_relations_tests/test_pagination.py @@ -1,9 +1,8 @@ import os - import pytest - import json from dbt.tests.util import run_dbt, run_dbt_and_capture +from dbt.adapters.snowflake import SnowflakeRelation # Ensure this is the correct import path # Testing rationale: # - snowflake SHOW TERSE OBJECTS command returns at max 10K objects in a single call @@ -122,8 +121,8 @@ def test__snowflake__list_relations_without_caching_termination(self, project): schemas = project.created_schemas for schema in schemas: - schema_relation = {"database": database, "schema": schema} - kwargs = {"schema_relation": schema_relation} + schema_relation = SnowflakeRelation.create(database=database, schema=schema) + kwargs = {"schema_relation": schema_relation.render()} _, log_output = run_dbt_and_capture( [ "--debug", @@ -137,7 +136,6 @@ def test__snowflake__list_relations_without_caching_termination(self, project): parsed_logs = parse_json_logs(log_output) n_relations = find_result_in_parsed_logs(parsed_logs, "n_relations") - assert n_relations == "n_relations: 1" @@ -171,8 +169,8 @@ def test__snowflake__list_relations_without_caching(self, project): schemas = project.created_schemas for schema in schemas: - schema_relation = {"database": database, "schema": schema} - kwargs = {"schema_relation": schema_relation} + schema_relation = SnowflakeRelation.create(database=database, schema=schema) + kwargs = {"schema_relation": schema_relation.render()} _, log_output = run_dbt_and_capture( [ "--debug", @@ -199,9 +197,9 @@ def test__snowflake__list_relations_without_caching_raise_error(self, project): schemas = project.created_schemas for schema in schemas: - schema_relation = {"database": database, "schema": schema} + schema_relation = SnowflakeRelation.create(database=database, schema=schema) - kwargs = {"schema_relation": schema_relation} + kwargs = {"schema_relation": schema_relation.render()} _, log_output = run_dbt_and_capture( [ "--debug", diff --git a/tests/functional/adapter/list_relations_tests/test_special_characters.py b/tests/functional/adapter/list_relations_tests/test_special_characters.py new file mode 100644 index 000000000..4dce56da9 --- /dev/null +++ b/tests/functional/adapter/list_relations_tests/test_special_characters.py @@ -0,0 +1,25 @@ +import pytest +from dbt.tests.util import run_dbt + + +TABLE_BASE_SQL = """ +-- models/my_model.sql +{{ config(schema = '1_contains_special*character$') }} +select 1 as id +""" + + +class TestSpecialCharactersInSchema: + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"quoting": {"schema": True}} + + @pytest.fixture(scope="class") + def models(self): + return { + "my_model.sql": TABLE_BASE_SQL, + } + + def test_schema_with_special_chars(self, project): + run_dbt(["run", "-s", "my_model"])