dbt-sqlserver icon indicating copy to clipboard operation
dbt-sqlserver copied to clipboard

Port is not being applied properly when using Windows Login with non-default port

Open cody-scott opened this issue 1 year ago • 4 comments

When calling dbt debug from windows using the following profile, the windows login defaults to the Fabric adapter implementation, which omits the port from the connection string.

dbt_project:
  target: dev
  outputs:
    dev:
      type: sqlserver
      driver: 'ODBC Driver 18 for SQL Server'
      server: 0.0.0.0
      port: 1444
      database: data_base
      schema: dbt
      trust_cert: true
      encrypt: false
      windows_login: true

this resolves to

DRIVER={ODBC Driver 18 for SQL Server};SERVER=0.0.0.0;Database=data_base;trusted_connection=Yes;encrypt=No;TrustServerCertificate=Yes;APP=dbt-sqlserver/1.7.4;ConnectRetryCount=1

When using a the standard sql login flow, it adds the port as expected.

Solved by either adding the port to the SERVER line (0.0.0.0,1444) + communicating this to users or lifting the windows login piece out of fabric into this adapter.

dbt_project:
  target: dev
  outputs:
    dev:
      type: sqlserver
      driver: 'ODBC Driver 18 for SQL Server'
      server: 0.0.0.0,1444
      database: data_base
      schema: dbt
      trust_cert: true
      encrypt: false
      windows_login: true

cody-scott avatar Apr 17 '24 19:04 cody-scott

I'm seeing this as well. Adding port to server as suggest works.

rlshuhart avatar Sep 18 '24 14:09 rlshuhart

Just stumble across this problem again and found I've already been here. I'll add that if I use sql server authentication, the non-standard port is used, however, if I use windows authentication, the non-standard port is ignored and fails to connect. As before, in the case of window authentication, adding the port to the server line resolves the problem, but this seems to be a work-around.

rlshuhart avatar Dec 02 '24 16:12 rlshuhart

Digging more this appears to start from here:

https://github.com/dbt-msft/dbt-sqlserver/blob/909f61cda860ddb6ba5ec4ca88fb5cf16c613afd/dbt/adapters/sqlserver/sqlserver_connections.py#L81-L82

This ends up using open from fabric_connection_manager as @cody-scott referred to. Unfortunately, fabric does not consider the port attribute and excludes it from the con_str here:

https://github.com/microsoft/dbt-fabric/blob/525fe95c960beb3c8dc0b34130111d6284aa3eab/dbt/adapters/fabric/fabric_connection_manager.py#L347-L352

In summary, the issue is that sqlserver_connections.py does not handle 'Windows Login' and out sources it to dbt-fabric/../fabric_connection_manager.py, but that doesn't use the port attribute. I'll open an issue there and we'll see what happens.

rlshuhart avatar Dec 02 '24 17:12 rlshuhart

Per https://github.com/microsoft/dbt-fabric/issues/247 use of non-default ports is out of scope in dbt-fabric since, "customers have no control over the port in Fabric DW." Therefore, in my opinion, it is fitting for dbt-sqlserver to handle windows authentication with non-default ports. I can take a crack at a PR to make a change, borrowing mostly from FabricConnectionManager, unless someone beats me to it.

rlshuhart avatar Dec 04 '24 22:12 rlshuhart