From c734aaa8a9e6f5bf2f38b0ca4837b1475e7b5cdb Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Tue, 10 Nov 2020 18:07:00 -0800 Subject: [PATCH 01/34] integrate dbt-adapter-tests --- dbt/adapters/sqlserver/connections.py | 4 +- dbt/adapters/sqlserver/impl.py | 92 +++++++++++++++++++++++ dbt/include/sqlserver/macros/adapters.sql | 22 +++++- test/integration/sqlserver.dbtspec | 23 ++++++ 4 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 test/integration/sqlserver.dbtspec diff --git a/dbt/adapters/sqlserver/connections.py b/dbt/adapters/sqlserver/connections.py index 4e0b6041..b21b3796 100644 --- a/dbt/adapters/sqlserver/connections.py +++ b/dbt/adapters/sqlserver/connections.py @@ -49,7 +49,8 @@ class SQLServerCredentials(Credentials): # "sql", "ActiveDirectoryPassword" or "ActiveDirectoryInteractive", or # "ServicePrincipal" authentication: Optional[str] = "sql" - encrypt: Optional[str] = "yes" + encrypt: Optional[str] = 'false' + trust_cert: Optional[str] = 'false' _ALIASES = { "user": "UID", @@ -61,6 +62,7 @@ class SQLServerCredentials(Credentials): "auth": "authentication", "app_id": "client_id", "app_secret": "client_secret", + "TrustServerCertificate": "trust_cert", } @property diff --git a/dbt/adapters/sqlserver/impl.py b/dbt/adapters/sqlserver/impl.py index ddd0cbae..8cab4554 100644 --- a/dbt/adapters/sqlserver/impl.py +++ b/dbt/adapters/sqlserver/impl.py @@ -1,6 +1,11 @@ from dbt.adapters.sql import SQLAdapter from dbt.adapters.sqlserver import SQLServerConnectionManager +from dbt.adapters.base.relation import BaseRelation import agate +from typing import ( + Optional, Tuple, Callable, Iterable, Type, Dict, Any, List, Mapping, + Iterator, Union, Set +) class SQLServerAdapter(SQLAdapter): @@ -34,3 +39,90 @@ def convert_number_type(cls, agate_table, col_idx): @classmethod def convert_time_type(cls, agate_table, col_idx): return "datetime" + + # Methods used in adapter tests + def timestamp_add_sql( + self, add_to: str, number: int = 1, interval: str = "hour" + ) -> str: + # note: 'interval' is not supported for T-SQL + # for backwards compatibility, we're compelled to set some sort of + # default. A lot of searching has lead me to believe that the + # '+ interval' syntax used in postgres/redshift is relatively common + # and might even be the SQL standard's intention. + return f"DATEADD({interval},{number},{add_to})" + + def string_add_sql( + self, add_to: str, value: str, location='append', + ) -> str: + """ + `+` is T-SQL's string concatenation operator + """ + if location == 'append': + return f"{add_to} + '{value}'" + elif location == 'prepend': + return f"'{value}' + {add_to}" + else: + raise RuntimeException( + f'Got an unexpected location value of "{location}"' + ) + + def get_rows_different_sql( + self, + relation_a: BaseRelation, + relation_b: BaseRelation, + column_names: Optional[List[str]] = None, + except_operator: str = "EXCEPT", + ) -> str: + + """ + note: using is not supported on Synapse so COLUMNS_EQUAL_SQL is adjsuted + Generate SQL for a query that returns a single row with a two + columns: the number of rows that are different between the two + relations and the number of mismatched rows. + """ + # This method only really exists for test reasons. + names: List[str] + if column_names is None: + columns = self.get_columns_in_relation(relation_a) + names = sorted((self.quote(c.name) for c in columns)) + else: + names = sorted((self.quote(n) for n in column_names)) + columns_csv = ", ".join(names) + + sql = COLUMNS_EQUAL_SQL.format( + columns=columns_csv, + relation_a=str(relation_a), + relation_b=str(relation_b), + except_op=except_operator, + ) + + return sql + + +COLUMNS_EQUAL_SQL = """ +with diff_count as ( + SELECT + 1 as id, + COUNT(*) as num_missing FROM ( + (SELECT {columns} FROM {relation_a} {except_op} + SELECT {columns} FROM {relation_b}) + UNION ALL + (SELECT {columns} FROM {relation_b} {except_op} + SELECT {columns} FROM {relation_a}) + ) as a +), table_a as ( + SELECT COUNT(*) as num_rows FROM {relation_a} +), table_b as ( + SELECT COUNT(*) as num_rows FROM {relation_b} +), row_count_diff as ( + select + 1 as id, + table_a.num_rows - table_b.num_rows as difference + from table_a, table_b +) +select + row_count_diff.difference as row_count_difference, + diff_count.num_missing as num_mismatched +from row_count_diff +join diff_count on row_count_diff.id = diff_count.id +""".strip() diff --git a/dbt/include/sqlserver/macros/adapters.sql b/dbt/include/sqlserver/macros/adapters.sql index 9a974ec1..41382809 100644 --- a/dbt/include/sqlserver/macros/adapters.sql +++ b/dbt/include/sqlserver/macros/adapters.sql @@ -51,10 +51,24 @@ {% endcall %} {% endmacro %} -{% macro sqlserver__drop_schema(database_name, schema_name) -%} +{% macro sqlserver__drop_schema(relation) -%} + {%- set tables_in_schema_query %} + SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES + WHERE TABLE_SCHEMA = '{{ relation.schema }}' + {% endset %} + {% set tables_to_drop = run_query(tables_in_schema_query).columns[0].values() %} + {% for table in tables_to_drop %} + {%- set schema_relation = adapter.get_relation(database=relation.database, + schema=relation.schema, + identifier=table) -%} + {% do drop_relation(schema_relation) %} + {%- endfor %} + {% call statement('drop_schema') -%} - drop schema if exists {{ relation.without_identifier().schema }} - {% endcall %} + IF EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ relation.schema }}') + BEGIN + EXEC('DROP SCHEMA {{ relation.schema }}') + END {% endcall %} {% endmacro %} {% macro sqlserver__drop_relation(relation) -%} @@ -85,7 +99,7 @@ end {% endmacro %} -{% macro sqlserver__check_schema_exists(database, schema) -%} +{% macro sqlserver__check_schema_exists(information_schema, schema) -%} {% call statement('check_schema_exists', fetch_result=True, auto_begin=False) -%} --USE {{ database_name }} SELECT count(*) as schema_exist FROM sys.schemas WHERE name = '{{ schema }}' diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec new file mode 100644 index 00000000..4956bb9f --- /dev/null +++ b/test/integration/sqlserver.dbtspec @@ -0,0 +1,23 @@ + +target: + type: sqlserver + driver: "ODBC Driver 17 for SQL Server" + schema: "dbt_test_{{ var('_dbt_random_suffix') }}" + host: 0.0.0.0 + database: msdb + username: SA + password: 5atyaNadella + encrypt: no +# trust_cert: yes + port: 1433 + threads: 8 +sequences: + test_dbt_empty: empty +# test_dbt_base: base +# test_dbt_ephemeral: ephemeral +# test_dbt_incremental: incremental +# test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp +# test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols +# test_dbt_data_test: data_test +# test_dbt_schema_test: schema_test + # test_dbt_ephemeral_data_tests: data_test_ephemeral_models \ No newline at end of file From fb8c9c07d0608f86705aae83d2a30b17bcec4ab4 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 10:30:34 -0800 Subject: [PATCH 02/34] still missing docker commands --- .circleci/config.yml | 25 +++++++++++++++++++++++++ requirements.txt | 8 ++++++++ tox.ini | 11 +++++++++++ 3 files changed, 44 insertions(+) create mode 100644 .circleci/config.yml create mode 100644 requirements.txt create mode 100644 tox.ini diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 00000000..c3b0fbf6 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,25 @@ +version: 2.1 + +orbs: + python: circleci/python@1.1.0 + +jobs: + build-and-test: + docker: + - image: dataders/pyodbc:1.2 + - image: mcr.microsoft.com/mssql/server:2019-latest + executor: python/default + steps: + - checkout + - python/install-packages: + pkg-manager: pip + - run: + name: Test + command: tox -e integration-synapse + +workflows: + main: + jobs: + - build-and-test: + context: + - DBT_SYNAPSE_PROFILE diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 00000000..ef9ec051 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,8 @@ +dbt-core~=0.18.0 +pyodbc>=4.0.27 +azure-identity>=1.4.0 +black~=20.8b1 +pytest-dbt-adapter~=0.2.0 +tox==3.2.0 +flake8>=3.5.0 +certifi==2020.6.20 \ No newline at end of file diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..3cd05a59 --- /dev/null +++ b/tox.ini @@ -0,0 +1,11 @@ +[tox] +skipsdist = True +envlist = unit, flake8, integration-synapse + +[testenv:integration-synapse] +basepython = python3 +commands = /bin/bash -c '{envpython} -m pytest -v test/integration/sqlserver.dbtspec' +passenv = DBT_SYNAPSE_DB DBT_SYNAPSE_PORT DBT_SYNAPSE_PWD DBT_SYNAPSE_SERVER DBT_SYNAPSE_UID +deps = + -r{toxinidir}/requirements.txt + -e. \ No newline at end of file From e383a8d64181383e7e9619b7fa1eb662ce6df4e9 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 10:35:23 -0800 Subject: [PATCH 03/34] start up vars --- .circleci/config.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index c3b0fbf6..3c19c3d6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -8,6 +8,10 @@ jobs: docker: - image: dataders/pyodbc:1.2 - image: mcr.microsoft.com/mssql/server:2019-latest + environment: + ACCEPT_EULA: 'yes' + MSSQL_SA_PASSWORD: 5atyaNadella + MSSQL_IP_ADDRESS: 0.0.0.0 executor: python/default steps: - checkout From a92047cc28ba066c79b921b5ee18346deb8df2a4 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 10:38:28 -0800 Subject: [PATCH 04/34] has to be string for SQL Server --- test/integration/sqlserver.dbtspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 4956bb9f..9075185f 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -7,7 +7,7 @@ target: database: msdb username: SA password: 5atyaNadella - encrypt: no + encrypt: 'no' # trust_cert: yes port: 1433 threads: 8 From 7bbba905cb1b150720f6cb547d4fd3b39a5d01f0 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 11:14:02 -0800 Subject: [PATCH 05/34] add dockerfile --- pyodbc.Dockerfile | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 pyodbc.Dockerfile diff --git a/pyodbc.Dockerfile b/pyodbc.Dockerfile new file mode 100644 index 00000000..d17845c7 --- /dev/null +++ b/pyodbc.Dockerfile @@ -0,0 +1,41 @@ + +FROM python:3.7-slim AS base + +ADD requirements.txt ./ + +# Setup dependencies for pyodbc +RUN \ + apt-get update && \ + apt-get install -y curl build-essential unixodbc-dev g++ apt-transport-https && \ + gpg --keyserver hkp://keys.gnupg.net --recv-keys 5072E1F5 + +RUN \ + export ACCEPT_EULA='Y' && \ + # Install pyodbc db drivers for MSSQL + curl https://packages.microsoft.com/keys/microsoft.asc | apt-key add - && \ + curl https://packages.microsoft.com/config/debian/9/prod.list > /etc/apt/sources.list.d/mssql-release.list && \ + apt-get update && \ + apt-get install -y msodbcsql17 odbc-postgresql + +# Update odbcinst.ini to make sure full path to driver is listed +RUN \ + sed 's/Driver=psql/Driver=\/usr\/lib\/x86_64-linux-gnu\/odbc\/psql/' /etc/odbcinst.ini > /tmp/temp.ini && \ + mv -f /tmp/temp.ini /etc/odbcinst.ini +# Install pip +RUN \ + pip install --upgrade pip && \ + pip install -r requirements.txt && \ + rm requirements.txt +# permission management +RUN \ + chmod +rwx /etc/ssl/openssl.cnf && \ + # change TLS back to version 1 + sed -i 's/TLSv1.2/TLSv1/g' /etc/ssl/openssl.cnf && \ + # allow weak certificates (certificate signed with SHA1) + # by downgrading OpenSSL security level from 2 to 1 + sed -i 's/SECLEVEL=2/SECLEVEL=1/g' /etc/ssl/openssl.cnf + +RUN \ + # Cleanup build dependencies + apt-get remove -y curl apt-transport-https debconf-utils g++ gcc rsync build-essential gnupg2 && \ + apt-get autoremove -y && apt-get autoclean -y \ No newline at end of file From a0c27dd3e12276d43bee7271990a949a2f08359b Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 11:15:14 -0800 Subject: [PATCH 06/34] added for debugging purposes --- pyodbc.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyodbc.Dockerfile b/pyodbc.Dockerfile index d17845c7..34576cd5 100644 --- a/pyodbc.Dockerfile +++ b/pyodbc.Dockerfile @@ -6,7 +6,7 @@ ADD requirements.txt ./ # Setup dependencies for pyodbc RUN \ apt-get update && \ - apt-get install -y curl build-essential unixodbc-dev g++ apt-transport-https && \ + apt-get install -y curl build-essential mssql-tools unixodbc-dev g++ apt-transport-https && \ gpg --keyserver hkp://keys.gnupg.net --recv-keys 5072E1F5 RUN \ From 6119801170585de91dfc1352e0f26b43d302117f Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 11:35:23 -0800 Subject: [PATCH 07/34] mssql-tools only on packages.microsoft.com --- pyodbc.Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyodbc.Dockerfile b/pyodbc.Dockerfile index 34576cd5..1a8bfea1 100644 --- a/pyodbc.Dockerfile +++ b/pyodbc.Dockerfile @@ -6,7 +6,7 @@ ADD requirements.txt ./ # Setup dependencies for pyodbc RUN \ apt-get update && \ - apt-get install -y curl build-essential mssql-tools unixodbc-dev g++ apt-transport-https && \ + apt-get install -y curl build-essential unixodbc-dev g++ apt-transport-https && \ gpg --keyserver hkp://keys.gnupg.net --recv-keys 5072E1F5 RUN \ @@ -15,7 +15,7 @@ RUN \ curl https://packages.microsoft.com/keys/microsoft.asc | apt-key add - && \ curl https://packages.microsoft.com/config/debian/9/prod.list > /etc/apt/sources.list.d/mssql-release.list && \ apt-get update && \ - apt-get install -y msodbcsql17 odbc-postgresql + apt-get install -y msodbcsql17 odbc-postgresql mssql-tools # Update odbcinst.ini to make sure full path to driver is listed RUN \ From e9190ae1ae8dd64728491355d92454a75374264e Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 11:43:19 -0800 Subject: [PATCH 08/34] add to path --- pyodbc.Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyodbc.Dockerfile b/pyodbc.Dockerfile index 1a8bfea1..8bd020db 100644 --- a/pyodbc.Dockerfile +++ b/pyodbc.Dockerfile @@ -17,6 +17,9 @@ RUN \ apt-get update && \ apt-get install -y msodbcsql17 odbc-postgresql mssql-tools +# add sqlcmd to the path +ENV PATH="$PATH:/opt/mssql-tools/bin" + # Update odbcinst.ini to make sure full path to driver is listed RUN \ sed 's/Driver=psql/Driver=\/usr\/lib\/x86_64-linux-gnu\/odbc\/psql/' /etc/odbcinst.ini > /tmp/temp.ini && \ From 893ed2ff581bec514a647962b86fef255aea55e7 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 11:55:11 -0800 Subject: [PATCH 09/34] connect this way? --- test/integration/sqlserver.dbtspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 9075185f..d0f0184a 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -3,7 +3,7 @@ target: type: sqlserver driver: "ODBC Driver 17 for SQL Server" schema: "dbt_test_{{ var('_dbt_random_suffix') }}" - host: 0.0.0.0 + host: localhost database: msdb username: SA password: 5atyaNadella From 1cb1bb732d2c553a4be063447a959fa09a183269 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 13:19:45 -0800 Subject: [PATCH 10/34] add wait command --- .circleci/config.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3c19c3d6..5e7ef2d8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,6 +15,18 @@ jobs: executor: python/default steps: - checkout + - run: + name: wait for SQLServer container to finish + command: | + let rc=1 + while [[ $rc -eq 1 ]]; do + nc -z 0.0.0.0 1433 + let rc=$? + done + if [[ $rc -ne 0 ]]; then + echo "Fatal: Could not connect to $PGHOST" + exit 1 + fi - python/install-packages: pkg-manager: pip - run: From 0127c9c7f8f8adedc1da1bfad75523fbec39e3d1 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 13:32:15 -0800 Subject: [PATCH 11/34] needed for testing --- pyodbc.Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyodbc.Dockerfile b/pyodbc.Dockerfile index 8bd020db..81144b02 100644 --- a/pyodbc.Dockerfile +++ b/pyodbc.Dockerfile @@ -9,6 +9,9 @@ RUN \ apt-get install -y curl build-essential unixodbc-dev g++ apt-transport-https && \ gpg --keyserver hkp://keys.gnupg.net --recv-keys 5072E1F5 +# install netcat (i.e. `nc` command) +RUN apt install -y netcat + RUN \ export ACCEPT_EULA='Y' && \ # Install pyodbc db drivers for MSSQL From 3bfbef284c210c31a1536748c20ae0b5ef1f1b75 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 13:44:03 -0800 Subject: [PATCH 12/34] needed for wait time? --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5e7ef2d8..8b88bea9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -6,7 +6,7 @@ orbs: jobs: build-and-test: docker: - - image: dataders/pyodbc:1.2 + - image: dataders/pyodbc:1.4 - image: mcr.microsoft.com/mssql/server:2019-latest environment: ACCEPT_EULA: 'yes' From 150b87038839b22881f97c18cd4957c3b178937d Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 13:46:46 -0800 Subject: [PATCH 13/34] alternative --- .circleci/config.yml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8b88bea9..0796a4b9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,15 +18,16 @@ jobs: - run: name: wait for SQLServer container to finish command: | - let rc=1 - while [[ $rc -eq 1 ]]; do - nc -z 0.0.0.0 1433 - let rc=$? - done - if [[ $rc -ne 0 ]]; then - echo "Fatal: Could not connect to $PGHOST" - exit 1 - fi + sleep 30 + # let rc=1 + # while [[ $rc -eq 1 ]]; do + # nc -z 0.0.0.0 1433 + # let rc=$? + # done + # if [[ $rc -ne 0 ]]; then + # echo "Fatal: Could not connect to $PGHOST" + # exit 1 + # fi - python/install-packages: pkg-manager: pip - run: From acb4935a827df88382b9791f7ebd3bcff3482b62 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 13:53:46 -0800 Subject: [PATCH 14/34] explicitly encrypt and trust --- dbt/adapters/sqlserver/connections.py | 2 ++ test/integration/sqlserver.dbtspec | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dbt/adapters/sqlserver/connections.py b/dbt/adapters/sqlserver/connections.py index b21b3796..48d97639 100644 --- a/dbt/adapters/sqlserver/connections.py +++ b/dbt/adapters/sqlserver/connections.py @@ -171,6 +171,8 @@ def open(cls, connection): if not getattr(credentials, "encrypt", False): con_str.append(f"Encrypt={credentials.encrypt}") + if not getattr(credentials, "trust_cert", False): + con_str.append(f"TrustServerCertificate={credentials.trust_cert}") con_str_concat = ';'.join(con_str) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index d0f0184a..3f949eec 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -7,8 +7,8 @@ target: database: msdb username: SA password: 5atyaNadella - encrypt: 'no' -# trust_cert: yes + encrypt: 'true' + trust_cert: 'false' port: 1433 threads: 8 sequences: From cc3a80bb5f397c3ab8ee01f4ddd5826e5bc5d765 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 13:59:28 -0800 Subject: [PATCH 15/34] reprex simpler without dbt --- .circleci/config.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 0796a4b9..a00f5231 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,7 +18,7 @@ jobs: - run: name: wait for SQLServer container to finish command: | - sleep 30 + sleep 60 # let rc=1 # while [[ $rc -eq 1 ]]; do # nc -z 0.0.0.0 1433 @@ -28,6 +28,9 @@ jobs: # echo "Fatal: Could not connect to $PGHOST" # exit 1 # fi + run: + name: test connection via SQL CMD + command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' - python/install-packages: pkg-manager: pip - run: From 063ec4a4bb0d8271e73baed2a8b134214c0a239e Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:00:23 -0800 Subject: [PATCH 16/34] typo --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a00f5231..3ef3f048 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -28,7 +28,7 @@ jobs: # echo "Fatal: Could not connect to $PGHOST" # exit 1 # fi - run: + - run: name: test connection via SQL CMD command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' - python/install-packages: From e464bcfcf8d9367a6400ea8e68dc73562aaa7c3a Mon Sep 17 00:00:00 2001 From: Anders Date: Thu, 19 Nov 2020 14:01:52 -0800 Subject: [PATCH 17/34] indentation error --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3ef3f048..1059375b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,8 +29,8 @@ jobs: # exit 1 # fi - run: - name: test connection via SQL CMD - command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' + name: test connection via SQL CMD + command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' - python/install-packages: pkg-manager: pip - run: From 82a13d0a25b1d93c38fb65c8dad0675f7ccc067b Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:21:17 -0800 Subject: [PATCH 18/34] bool inputs outputs 'Yes' when True --- dbt/adapters/sqlserver/connections.py | 14 ++++++++------ test/integration/sqlserver.dbtspec | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/dbt/adapters/sqlserver/connections.py b/dbt/adapters/sqlserver/connections.py index 48d97639..e128afb9 100644 --- a/dbt/adapters/sqlserver/connections.py +++ b/dbt/adapters/sqlserver/connections.py @@ -49,8 +49,8 @@ class SQLServerCredentials(Credentials): # "sql", "ActiveDirectoryPassword" or "ActiveDirectoryInteractive", or # "ServicePrincipal" authentication: Optional[str] = "sql" - encrypt: Optional[str] = 'false' - trust_cert: Optional[str] = 'false' + encrypt: Optional[bool] = False + trust_cert: Optional[bool] = False _ALIASES = { "user": "UID", @@ -84,6 +84,7 @@ def _connection_keys(self): "client_id", "authentication", "encrypt", + "trust_cert" ) @@ -169,10 +170,11 @@ def open(cls, connection): con_str.append(f"UID={{{credentials.UID}}}") con_str.append(f"PWD={{{credentials.PWD}}}") - if not getattr(credentials, "encrypt", False): - con_str.append(f"Encrypt={credentials.encrypt}") - if not getattr(credentials, "trust_cert", False): - con_str.append(f"TrustServerCertificate={credentials.trust_cert}") + # still confused whether to use "Yes", "yes", "True", or "true" + if getattr(credentials, "encrypt", False): + con_str.append(f"Encrypt=Yes") + if getattr(credentials, "trust_cert", False): + con_str.append(f"TrustServerCertificate=Yes") con_str_concat = ';'.join(con_str) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 3f949eec..99821886 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -7,8 +7,8 @@ target: database: msdb username: SA password: 5atyaNadella - encrypt: 'true' - trust_cert: 'false' + encrypt: true + trust_cert: true port: 1433 threads: 8 sequences: From a79824d26001a308132961fc8c64383327e392b3 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:24:55 -0800 Subject: [PATCH 19/34] helping out future us --- dbt/adapters/sqlserver/connections.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dbt/adapters/sqlserver/connections.py b/dbt/adapters/sqlserver/connections.py index e128afb9..da942e0b 100644 --- a/dbt/adapters/sqlserver/connections.py +++ b/dbt/adapters/sqlserver/connections.py @@ -171,6 +171,8 @@ def open(cls, connection): con_str.append(f"PWD={{{credentials.PWD}}}") # still confused whether to use "Yes", "yes", "True", or "true" + # to learn more visit + # https://docs.microsoft.com/en-us/sql/relational-databases/native-client/features/using-encryption-without-validation?view=sql-server-ver15 if getattr(credentials, "encrypt", False): con_str.append(f"Encrypt=Yes") if getattr(credentials, "trust_cert", False): From 42b444fe6bb8799496f5ca7e457e580912d903c8 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:25:34 -0800 Subject: [PATCH 20/34] enable all tests I know should work --- test/integration/sqlserver.dbtspec | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 99821886..9751bd6a 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -13,11 +13,11 @@ target: threads: 8 sequences: test_dbt_empty: empty -# test_dbt_base: base -# test_dbt_ephemeral: ephemeral -# test_dbt_incremental: incremental -# test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp -# test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols -# test_dbt_data_test: data_test -# test_dbt_schema_test: schema_test + test_dbt_base: base + test_dbt_ephemeral: ephemeral + test_dbt_incremental: incremental + test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp + test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols + test_dbt_data_test: data_test + test_dbt_schema_test: schema_test # test_dbt_ephemeral_data_tests: data_test_ephemeral_models \ No newline at end of file From 18b6731acd74143e1be74b021b312afb46a48947 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:26:33 -0800 Subject: [PATCH 21/34] sleep is sufficient --- .circleci/config.yml | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1059375b..30786a76 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,17 +17,7 @@ jobs: - checkout - run: name: wait for SQLServer container to finish - command: | - sleep 60 - # let rc=1 - # while [[ $rc -eq 1 ]]; do - # nc -z 0.0.0.0 1433 - # let rc=$? - # done - # if [[ $rc -ne 0 ]]; then - # echo "Fatal: Could not connect to $PGHOST" - # exit 1 - # fi + command: sleep 60 - run: name: test connection via SQL CMD command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' From d6f5f95890eae2b66e13098c9902f4e357235a25 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:26:49 -0800 Subject: [PATCH 22/34] sleeping not needed --- .circleci/config.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 30786a76..5060df4e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,9 +15,6 @@ jobs: executor: python/default steps: - checkout - - run: - name: wait for SQLServer container to finish - command: sleep 60 - run: name: test connection via SQL CMD command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' From 6fc477940eb9ad75d7b804f93b141c235d799e97 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:28:35 -0800 Subject: [PATCH 23/34] update with upstream --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ef9ec051..80015832 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ dbt-core~=0.18.0 pyodbc>=4.0.27 azure-identity>=1.4.0 black~=20.8b1 -pytest-dbt-adapter~=0.2.0 +pytest-dbt-adapter~=0.3.0 tox==3.2.0 flake8>=3.5.0 certifi==2020.6.20 \ No newline at end of file From a45504d5c4e61c891209657e17ab6c5f29e590fc Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:29:36 -0800 Subject: [PATCH 24/34] clarity --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5060df4e..0df0f427 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,7 +21,7 @@ jobs: - python/install-packages: pkg-manager: pip - run: - name: Test + name: Test adapter against dbt-adapter-tests command: tox -e integration-synapse workflows: From 0ea46f275b98e6b88fd8086d5a786fa8a3fe2147 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 14:31:33 -0800 Subject: [PATCH 25/34] need sleep --- .circleci/config.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 0df0f427..32b2b41e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,6 +15,9 @@ jobs: executor: python/default steps: - checkout + - run: + name: wait for SQL Server container to set up + command: sleep 30 - run: name: test connection via SQL CMD command: sqlcmd -S 'localhost,1433' -U sa -P 5atyaNadella -Q 'create database blog' From b2b9c7597a2bc22789c3906a08e448144b7171a5 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 21:43:18 -0800 Subject: [PATCH 26/34] only failing test --- test/integration/sqlserver.dbtspec | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 9751bd6a..c581cf83 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -12,12 +12,12 @@ target: port: 1433 threads: 8 sequences: - test_dbt_empty: empty - test_dbt_base: base - test_dbt_ephemeral: ephemeral - test_dbt_incremental: incremental - test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp + # test_dbt_empty: empty + # test_dbt_base: base + # test_dbt_ephemeral: ephemeral + # test_dbt_incremental: incremental + # test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols - test_dbt_data_test: data_test - test_dbt_schema_test: schema_test + # test_dbt_data_test: data_test + # test_dbt_schema_test: schema_test # test_dbt_ephemeral_data_tests: data_test_ephemeral_models \ No newline at end of file From 0d4df19cfac4c3751c5c1c01dc79cb6e6ab94184 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Thu, 19 Nov 2020 23:01:51 -0800 Subject: [PATCH 27/34] run all tests that are currently passing --- test/integration/sqlserver.dbtspec | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index c581cf83..bf671fcf 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -12,12 +12,12 @@ target: port: 1433 threads: 8 sequences: - # test_dbt_empty: empty - # test_dbt_base: base - # test_dbt_ephemeral: ephemeral - # test_dbt_incremental: incremental - # test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp - test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols - # test_dbt_data_test: data_test - # test_dbt_schema_test: schema_test + test_dbt_empty: empty + test_dbt_base: base + test_dbt_ephemeral: ephemeral + test_dbt_incremental: incremental + test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp + # test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols + test_dbt_data_test: data_test + test_dbt_schema_test: schema_test # test_dbt_ephemeral_data_tests: data_test_ephemeral_models \ No newline at end of file From fdc6171bcf5a1dff38af324a25753f15dc5e4e3a Mon Sep 17 00:00:00 2001 From: mikaelene Date: Fri, 20 Nov 2020 09:00:48 +0100 Subject: [PATCH 28/34] New try at tests. Only changed user and pass. Need to replace with variables later... --- test/integration/sqlserver.dbtspec | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index c581cf83..2b37ac92 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -4,20 +4,22 @@ target: driver: "ODBC Driver 17 for SQL Server" schema: "dbt_test_{{ var('_dbt_random_suffix') }}" host: localhost - database: msdb + #database: msdb + database: dbt_test username: SA - password: 5atyaNadella + #password: 5atyaNadella + password: "TheStrongestPass!" encrypt: true trust_cert: true port: 1433 threads: 8 sequences: - # test_dbt_empty: empty - # test_dbt_base: base - # test_dbt_ephemeral: ephemeral - # test_dbt_incremental: incremental - # test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp + test_dbt_empty: empty + test_dbt_base: base + test_dbt_ephemeral: ephemeral + test_dbt_incremental: incremental + test_dbt_snapshot_strategy_timestamp: snapshot_strategy_timestamp test_dbt_snapshot_strategy_check_cols: snapshot_strategy_check_cols - # test_dbt_data_test: data_test - # test_dbt_schema_test: schema_test - # test_dbt_ephemeral_data_tests: data_test_ephemeral_models \ No newline at end of file + test_dbt_data_test: data_test + test_dbt_schema_test: schema_test + #test_dbt_ephemeral_data_tests: data_test_ephemeral_models \ No newline at end of file From 05dfb9e250708c44bd0d1e0c8f553cdafde426cb Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Mon, 23 Nov 2020 21:16:46 -0800 Subject: [PATCH 29/34] bugfix encrypt & trust_cert attr weren't respected --- dbt/adapters/sqlserver/connections.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbt/adapters/sqlserver/connections.py b/dbt/adapters/sqlserver/connections.py index da942e0b..747ca015 100644 --- a/dbt/adapters/sqlserver/connections.py +++ b/dbt/adapters/sqlserver/connections.py @@ -173,10 +173,10 @@ def open(cls, connection): # still confused whether to use "Yes", "yes", "True", or "true" # to learn more visit # https://docs.microsoft.com/en-us/sql/relational-databases/native-client/features/using-encryption-without-validation?view=sql-server-ver15 - if getattr(credentials, "encrypt", False): + if getattr(credentials, "encrypt", False) is True: con_str.append(f"Encrypt=Yes") - if getattr(credentials, "trust_cert", False): - con_str.append(f"TrustServerCertificate=Yes") + if getattr(credentials, "trust_cert", False) is True: + con_str.append(f"TrustServerCertificate=Yes") con_str_concat = ';'.join(con_str) From 5745b2f41d403435833a64693dd8119ba9d91180 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Mon, 23 Nov 2020 21:28:46 -0800 Subject: [PATCH 30/34] align with CircleCI config --- test/integration/sqlserver.dbtspec | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 46f1afc7..3c2384ab 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -4,11 +4,9 @@ target: driver: "ODBC Driver 17 for SQL Server" schema: "dbt_test_{{ var('_dbt_random_suffix') }}" host: localhost - #database: msdb - database: dbt_test + database: msdb username: SA - #password: 5atyaNadella - password: "TheStrongestPass!" + password: 5atyaNadella encrypt: true trust_cert: true port: 1433 From 94592a4746bb1986f43096683361c02e692f1686 Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Mon, 23 Nov 2020 23:07:44 -0800 Subject: [PATCH 31/34] document new params --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 5230f609..953b8147 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,16 @@ port: 1433 schema: schemaname ``` +### Security +If not specified, Encryption is not enabled by default. + +To enable encryption, add the following to your target definition. +```yaml +encrypt: true # adds "Encrypt=Yes" to connection string +trust_cert: false +``` +For a fully-secure, encrypted connection, you must enable `trust_cert: false` because "TrustServerCertificate=Yes" is default in order to not break already defined targets. For more information see [this docs page](https://docs.microsoft.com/en-us/dotnet/framework/data/adonet/connection-string-syntax#using-trustservercertificate?WT.mc_id=DP-MVP-5003930) + ### standard SQL Server authentication SQL Server credentials are supported for on-prem as well as cloud, and it is the default authentication method for `dbt-sqlsever` ``` From a820aaf4aad50f4cc06f8282f81a83c345d9e46c Mon Sep 17 00:00:00 2001 From: Anders Swanson Date: Mon, 23 Nov 2020 23:11:28 -0800 Subject: [PATCH 32/34] cleaner explanation --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 953b8147..293eea87 100644 --- a/README.md +++ b/README.md @@ -33,14 +33,14 @@ schema: schemaname ``` ### Security -If not specified, Encryption is not enabled by default. +Encryption is not enabled by default, unless you specify it. -To enable encryption, add the following to your target definition. +To enable encryption, add the following to your target definition. This is the default encryption strategy recommended by MSFT. For more information see [this docs page](https://docs.microsoft.com/en-us/dotnet/framework/data/adonet/connection-string-syntax#using-trustservercertificate?WT.mc_id=DP-MVP-5003930) ```yaml encrypt: true # adds "Encrypt=Yes" to connection string trust_cert: false ``` -For a fully-secure, encrypted connection, you must enable `trust_cert: false` because "TrustServerCertificate=Yes" is default in order to not break already defined targets. For more information see [this docs page](https://docs.microsoft.com/en-us/dotnet/framework/data/adonet/connection-string-syntax#using-trustservercertificate?WT.mc_id=DP-MVP-5003930) +For a fully-secure, encrypted connection, you must enable `trust_cert: false` because `"TrustServerCertificate=Yes"` is default for `dbt-sqlserver` in order to not break already defined targets. ### standard SQL Server authentication SQL Server credentials are supported for on-prem as well as cloud, and it is the default authentication method for `dbt-sqlsever` From f666ed55d8ee44d3dd4dece93af8ceee5c2c02e5 Mon Sep 17 00:00:00 2001 From: mikaelene Date: Mon, 30 Nov 2020 09:32:04 +0100 Subject: [PATCH 33/34] Removed Authentication=SqlPassword for standard sql user pass auth --- dbt/adapters/sqlserver/connections.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/adapters/sqlserver/connections.py b/dbt/adapters/sqlserver/connections.py index 747ca015..5b12a58a 100644 --- a/dbt/adapters/sqlserver/connections.py +++ b/dbt/adapters/sqlserver/connections.py @@ -166,7 +166,7 @@ def open(cls, connection): elif getattr(credentials, "windows_login", False): con_str.append(f"trusted_connection=yes") elif type_auth == "sql": - con_str.append("Authentication=SqlPassword") + #con_str.append("Authentication=SqlPassword") con_str.append(f"UID={{{credentials.UID}}}") con_str.append(f"PWD={{{credentials.PWD}}}") From 6743a1bad3bb17318c57d064cf036332668eec3a Mon Sep 17 00:00:00 2001 From: mikaelene Date: Tue, 15 Dec 2020 11:09:48 +0100 Subject: [PATCH 34/34] Updated the sqlserver.dbtspec to reflect how I imagine most users connect. --- test/integration/sqlserver.dbtspec | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/integration/sqlserver.dbtspec b/test/integration/sqlserver.dbtspec index 3c2384ab..f4df03e4 100644 --- a/test/integration/sqlserver.dbtspec +++ b/test/integration/sqlserver.dbtspec @@ -7,8 +7,6 @@ target: database: msdb username: SA password: 5atyaNadella - encrypt: true - trust_cert: true port: 1433 threads: 8 sequences: