diff --git a/.github/workflows/integration-tests-azure.yml b/.github/workflows/integration-tests-azure.yml index 0691777..bbeed0e 100644 --- a/.github/workflows/integration-tests-azure.yml +++ b/.github/workflows/integration-tests-azure.yml @@ -1,32 +1,56 @@ ---- -name: Integration tests on Azure -on: # yamllint disable-line rule:truthy +name: Integration tests on Fabric DW +on: # yamllint disable-line rule:truthy workflow_dispatch: pull_request: branches: - main jobs: - integration-tests-azure: + integration-tests-fabric-dw: name: Regular strategy: fail-fast: false max-parallel: 1 matrix: - profile: ["ci_azure_auto"] + profile: ["integration_tests"] python_version: ["3.11"] - msodbc_version: ["17", "18"] + msodbc_version: ["18"] runs-on: ubuntu-latest + permissions: + contents: read # Required to access repository files + packages: read # Grant explicit read access to packages + id-token: write # Needed if using OIDC authentication container: image: ghcr.io/${{ github.repository }}:CI-${{ matrix.python_version }}-msodbc${{ matrix.msodbc_version }} steps: - - name: AZ CLI login - run: az login --service-principal --username="${AZURE_CLIENT_ID}" --password="${AZURE_CLIENT_SECRET}" --tenant="${AZURE_TENANT_ID}" - env: - AZURE_CLIENT_ID: ${{ secrets.DBT_AZURE_SP_NAME }} - AZURE_CLIENT_SECRET: ${{ secrets.DBT_AZURE_SP_SECRET }} - AZURE_TENANT_ID: ${{ secrets.DBT_AZURE_TENANT }} + # Azure login using federated credentials + - name: Azure login with OIDC + uses: azure/login@v2 + with: + client-id: ${{ secrets.DBT_AZURE_SP_NAME }} + tenant-id: ${{ secrets.DBT_AZURE_TENANT }} + allow-no-subscriptions: true + federated-token: true + + - name: Test Connection To Fabric Data Warehouse + id: fetch_token + run: | + pip install azure-identity pyodbc azure-core + + python - < AccessToken: } -def get_pyodbc_attrs_before(credentials: FabricCredentials) -> Dict: +def get_pyodbc_attrs_before_credentials(credentials: FabricCredentials) -> Dict: """ Get the pyodbc attrs before. @@ -220,6 +220,36 @@ def get_pyodbc_attrs_before(credentials: FabricCredentials) -> Dict: return attrs_before +def get_pyodbc_attrs_before_accesstoken(accessToken: str) -> Dict: + """ + Get the pyodbc attrs before. + + Parameters + ---------- + credentials : Access Token for Integration Tests + Credentials. + + Returns + ------- + out : Dict + The pyodbc attrs before. + + Source + ------ + Authentication for SQL server with an access token: + https://docs.microsoft.com/en-us/sql/connect/odbc/using-azure-active-directory?view=sql-server-ver15#authenticating-with-an-access-token + """ + + access_token_utf16 = accessToken.encode("utf-16-le") + token_struct = struct.pack( + f" str: """ Convert a boolean to a connection string argument. @@ -323,7 +353,7 @@ def open(cls, connection: Connection) -> Connection: con_str.append(f"Database={credentials.database}") - #Enabling trace flag + # Enabling trace flag if credentials.trace_flag: con_str.append("SQL_ATTR_TRACE=SQL_OPT_TRACE_ON") else: @@ -331,7 +361,10 @@ def open(cls, connection: Connection) -> Connection: assert credentials.authentication is not None - if "ActiveDirectory" in credentials.authentication: + if ( + "ActiveDirectory" in credentials.authentication + and credentials.authentication != "ActiveDirectoryAccessToken" + ): con_str.append(f"Authentication={credentials.authentication}") if credentials.authentication == "ActiveDirectoryPassword": @@ -395,7 +428,11 @@ def open(cls, connection: Connection) -> Connection: def connect(): logger.debug(f"Using connection string: {con_str_display}") - attrs_before = get_pyodbc_attrs_before(credentials) + if credentials.authentication == "ActiveDirectoryAccessToken": + attrs_before = get_pyodbc_attrs_before_accesstoken(credentials.access_token) + else: + attrs_before = get_pyodbc_attrs_before_credentials(credentials) + handle = pyodbc.connect( con_str_concat, attrs_before=attrs_before, diff --git a/dbt/adapters/fabric/fabric_credentials.py b/dbt/adapters/fabric/fabric_credentials.py index a824fac..138e3bd 100644 --- a/dbt/adapters/fabric/fabric_credentials.py +++ b/dbt/adapters/fabric/fabric_credentials.py @@ -17,6 +17,7 @@ class FabricCredentials(Credentials): tenant_id: Optional[str] = None client_id: Optional[str] = None client_secret: Optional[str] = None + access_token: Optional[str] = None authentication: Optional[str] = "ActiveDirectoryServicePrincipal" encrypt: Optional[bool] = True # default value in MS ODBC Driver 18 as well trust_cert: Optional[bool] = False # default value in MS ODBC Driver 18 as well diff --git a/dbt/include/fabric/macros/adapters/catalog.sql b/dbt/include/fabric/macros/adapters/catalog.sql index 6bd1398..555b5a7 100644 --- a/dbt/include/fabric/macros/adapters/catalog.sql +++ b/dbt/include/fabric/macros/adapters/catalog.sql @@ -96,7 +96,7 @@ c.column_id as column_index, t.name as column_type from sys.columns as c {{ information_schema_hints() }} - left join sys.types as t on c.system_type_id = t.system_type_id {{ information_schema_hints() }} + left join sys.types as t on c.system_type_id = t.system_type_id ) select @@ -223,7 +223,7 @@ c.column_id as column_index, t.name as column_type from sys.columns as c {{ information_schema_hints() }} - left join sys.types as t on c.system_type_id = t.system_type_id {{ information_schema_hints() }} + left join sys.types as t on c.system_type_id = t.system_type_id ) select diff --git a/dbt/include/fabric/macros/adapters/show.sql b/dbt/include/fabric/macros/adapters/show.sql index 205fa1f..5b60e8b 100644 --- a/dbt/include/fabric/macros/adapters/show.sql +++ b/dbt/include/fabric/macros/adapters/show.sql @@ -1,24 +1,12 @@ {% macro fabric__get_limit_sql(sql, limit) %} - - {% if limit == -1 or limit is none %} - {% if sql.strip().lower().startswith('with') %} - {{ sql }} - {% else -%} - select * - from ( - {{ sql }} - ) as model_limit_subq - {%- endif -%} - {% else -%} - {% if sql.strip().lower().startswith('with') %} - {{ sql }} order by (select null) - offset 0 rows fetch first {{ limit }} rows only - {% else -%} - select * - from ( - {{ sql }} - ) as model_limit_subq order by (select null) - offset 0 rows fetch first {{ limit }} rows only - {%- endif -%} + {%- if limit == -1 or limit is none -%} + {{ sql }} + {#- Special processing if the last non-blank line starts with order by -#} + {%- elif sql.strip().splitlines()[-1].strip().lower().startswith('order by') -%} + {{ sql }} + offset 0 rows fetch first {{ limit }} rows only + {%- else -%} + {{ sql }} + order by (select null) offset 0 rows fetch first {{ limit }} rows only {%- endif -%} {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 4eb1f4a..6964013 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -35,20 +35,25 @@ {% if existing_relation is not none and existing_relation.is_table %} - -- making a backup relation, this will come in use when contract is enforced or not - {%- set backup_relation = make_backup_relation(existing_relation, 'table') -%} + -- making a backup relation, this will come in use when contract is enforced or not + {%- set set_backup_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} + {% if (set_backup_relation != none and set_backup_relation.type == "table") %} + {%- set backup_relation = make_backup_relation(target_relation, 'table') -%} + {% elif (set_backup_relation != none and set_backup_relation.type == "view") %} + {%- set backup_relation = make_backup_relation(target_relation, 'view') -%} + {% endif %} - -- Dropping a temp relation if it exists - {{ adapter.drop_relation(backup_relation) }} + -- Dropping a temp relation if it exists + {{ adapter.drop_relation(backup_relation) }} - -- Rename existing relation to back up relation - {{ adapter.rename_relation(existing_relation, backup_relation) }} + -- Rename existing relation to back up relation + {{ adapter.rename_relation(existing_relation, backup_relation) }} - -- Renaming temp relation as main relation - {{ adapter.rename_relation(temp_relation, target_relation) }} + -- Renaming temp relation as main relation + {{ adapter.rename_relation(temp_relation, target_relation) }} - -- Drop backup relation - {{ adapter.drop_relation(backup_relation) }} + -- Drop backup relation + {{ adapter.drop_relation(backup_relation) }} {%- else %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql index 29d4934..d5915e4 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql @@ -3,56 +3,12 @@ {% do drop_relation_if_exists(staging_relation) %} {% endmacro %} ---Due to Alter not being supported, have to rely on this for temporarily {% macro fabric__create_columns(relation, columns) %} - {# default__ macro uses "add column" - TSQL preferes just "add" - #} - - {% set columns %} - {% for column in columns %} - , CAST(NULL AS {{column.data_type}}) AS {{column.name}} - {% endfor %} - {% endset %} - - {% set tempTableName %} - [{{relation.database}}].[{{ relation.schema }}].[{{ relation.identifier }}_{{ range(1300, 19000) | random }}] - {% endset %} - {{ log("Creating new columns are not supported without dropping a table. Using random table as a temp table. - " ~ tempTableName) }} - - {% set tempTable %} - CREATE TABLE {{tempTableName}} - AS SELECT * {{columns}} FROM [{{relation.database}}].[{{ relation.schema }}].[{{ relation.identifier }}] {{ information_schema_hints() }} {{ apply_label() }} - {% endset %} - - {% call statement('create_temp_table') -%} - {{ tempTable }} - {%- endcall %} - - {% set dropTable %} - DROP TABLE [{{relation.database}}].[{{ relation.schema }}].[{{ relation.identifier }}] - {% endset %} - - {% call statement('drop_table') -%} - {{ dropTable }} - {%- endcall %} - - {% set createTable %} - CREATE TABLE {{ relation }} - AS SELECT * FROM {{tempTableName}} {{ information_schema_hints() }} {{ apply_label() }} - {% endset %} - - {% call statement('create_Table') -%} - {{ createTable }} - {%- endcall %} - - {% set dropTempTable %} - DROP TABLE {{tempTableName}} - {% endset %} - - {% call statement('drop_temp_table') -%} - {{ dropTempTable }} - {%- endcall %} + {% for column in columns %} + {% call statement() %} + alter table {{ relation.render() }} add "{{ column.name }}" {{ column.data_type }} NULL; + {% endcall %} + {% endfor %} {% endmacro %} {% macro fabric__get_true_sql() %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql index 07c6789..9ed2f91 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql @@ -31,10 +31,11 @@ -- Create a temporary view to manage if user SQl uses CTE {% set temp_snapshot_relation_sql = model['compiled_code'].replace("'", "''") %} - {% call statement('create temp_snapshot_relation') %} - EXEC('DROP VIEW IF EXISTS {{ temp_snapshot_relation.include(database=False) }};'); - EXEC('create view {{ temp_snapshot_relation.include(database=False) }} as {{ temp_snapshot_relation_sql }};'); - {% endcall %} + {{ adapter.drop_relation(temp_snapshot_relation) }} + + {% call statement('create temp_snapshot_relation') -%} + {{ get_create_view_as_sql(temp_snapshot_relation, temp_snapshot_relation_sql) }} + {%- endcall %} {% if not target_relation_exists %} diff --git a/dbt/include/fabric/macros/materializations/tests/helpers.sql b/dbt/include/fabric/macros/materializations/tests/helpers.sql index 4f04547..4fd157c 100644 --- a/dbt/include/fabric/macros/materializations/tests/helpers.sql +++ b/dbt/include/fabric/macros/materializations/tests/helpers.sql @@ -1,42 +1,12 @@ {% macro fabric__get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) -%} - -- Create target schema if it does not - USE [{{ target.database }}]; - IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ target.schema }}') - BEGIN - EXEC('CREATE SCHEMA [{{ target.schema }}]') - END + With dbt_internal_test AS ( + {{ main_sql }} + ) + select + COUNT(*) AS failures, + CASE WHEN COUNT(*) != 0 THEN 'true' ELSE 'false' END AS should_warn, + CASE WHEN COUNT(*) != 0 THEN 'true' ELSE 'false' END AS should_error + FROM dbt_internal_test - {% if main_sql.strip().lower().startswith('with') %} - {% set testview %} - [{{ target.schema }}.testview_{{ range(1300, 19000) | random }}] - {% endset %} - - {% set sql = main_sql.replace("'", "''")%} - EXEC('create view {{testview}} as {{ sql }};') - select - {{ "top (" ~ limit ~ ')' if limit != none }} - {{ fail_calc }} as failures, - case when {{ fail_calc }} {{ warn_if }} - then 'true' else 'false' end as should_warn, - case when {{ fail_calc }} {{ error_if }} - then 'true' else 'false' end as should_error - from ( - select * from {{testview}} - ) dbt_internal_test; - - EXEC('drop view {{testview}};') - - {% else -%} - select - {{ "top (" ~ limit ~ ')' if limit != none }} - {{ fail_calc }} as failures, - case when {{ fail_calc }} {{ warn_if }} - then 'true' else 'false' end as should_warn, - case when {{ fail_calc }} {{ error_if }} - then 'true' else 'false' end as should_error - from ( - {{ main_sql }} - ) dbt_internal_test - {%- endif -%} {%- endmacro %} diff --git a/dev_requirements.txt b/dev_requirements.txt index d3313a4..f28c23a 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,9 +1,8 @@ # install latest changes in dbt-core # TODO: how to automate switching from develop to version branches? -git+https://github.com/dbt-labs/dbt-core.git@v1.8.0#egg=dbt-core&subdirectory=core +git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core git+https://github.com/dbt-labs/dbt-adapters.git git+https://github.com/dbt-labs/dbt-adapters.git#subdirectory=dbt-tests-adapter -git+https://github.com/dbt-labs/dbt-common.git pytest==8.0.1 twine==5.1.1 diff --git a/tests/conftest.py b/tests/conftest.py index 3e60ce0..ecb2e67 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,8 @@ def dbt_profile_target(request: FixtureRequest, dbt_profile_target_update): target = _profile_ci_azure_environment() elif profile == "user_azure": target = _profile_user_azure() + elif profile == "integration_tests": + target = _profile_integration_tests() else: raise ValueError(f"Unknown profile: {profile}") @@ -55,7 +57,7 @@ def _profile_ci_azure_base(): "database": os.getenv("DBT_AZURESQL_DB"), "encrypt": True, "trust_cert": True, - "trace_flag":False, + "trace_flag": False, }, } @@ -92,13 +94,21 @@ def _profile_user_azure(): **_all_profiles_base(), **{ "host": os.getenv("FABRIC_TEST_HOST"), - "authentication": os.getenv("FABRIC_TEST_AUTH", "auto"), + "authentication": os.getenv("FABRIC_TEST_AUTH", "CLI"), "encrypt": True, "trust_cert": True, "database": os.getenv("FABRIC_TEST_DBNAME"), - "client_id": os.getenv("FABRIC_TEST_CLIENT_ID"), - "client_secret": os.getenv("FABRIC_TEST_CLIENT_SECRET"), - "tenant_id": os.getenv("FABRIC_TEST_TENANT_ID"), + }, + } + return profile + + +def _profile_integration_tests(): + profile = { + **_profile_ci_azure_base(), + **{ + "authentication": os.getenv("FABRIC_TEST_AUTH", "ActiveDirectoryAccessToken"), + "access_token": os.getenv("FABRIC_INTEGRATION_TESTS_TOKEN"), }, } return profile diff --git a/tests/unit/adapters/fabric/test_sql_server_connection_manager.py b/tests/unit/adapters/fabric/test_sql_server_connection_manager.py index 757806b..539281c 100644 --- a/tests/unit/adapters/fabric/test_sql_server_connection_manager.py +++ b/tests/unit/adapters/fabric/test_sql_server_connection_manager.py @@ -8,7 +8,7 @@ from dbt.adapters.fabric.fabric_connection_manager import ( bool_to_connection_string_arg, byte_array_to_datetime, - get_pyodbc_attrs_before, + get_pyodbc_attrs_before_credentials, ) from dbt.adapters.fabric.fabric_credentials import FabricCredentials @@ -20,7 +20,7 @@ @pytest.fixture def credentials() -> FabricCredentials: credentials = FabricCredentials( - driver="ODBC Driver 17 for SQL Server", + driver="ODBC Driver 18 for SQL Server", host="fake.sql.fabric.net", database="dbt", schema="fabric", @@ -52,7 +52,7 @@ def test_get_pyodbc_attrs_before_empty_dict_when_service_principal( """ When the authentication is set to sql we expect an empty attrs before. """ - attrs_before = get_pyodbc_attrs_before(credentials) + attrs_before = get_pyodbc_attrs_before_credentials(credentials) assert attrs_before == {} @@ -68,7 +68,7 @@ def test_get_pyodbc_attrs_before_contains_access_token_key_for_cli_authenticatio """ credentials.authentication = authentication with mock.patch(CHECK_OUTPUT, mock.Mock(return_value=mock_cli_access_token)): - attrs_before = get_pyodbc_attrs_before(credentials) + attrs_before = get_pyodbc_attrs_before_credentials(credentials) assert 1256 in attrs_before.keys()