search

dbt-msft / dbt-sqlserver

dbt adapter for SQL Server and Azure SQL
MIT License
205 stars 96 forks source link

Determine if Azure CLI tests are applicable to SQL Server adapter #463

Closed schlich closed 7 months ago

schlich commented 7 months ago

Issue split from conversation

_Originally posted by @schlich in https://github.com/dbt-msft/dbt-sqlserver/pull/461#discussion_r1445044674_

Login through the Azure CLI is not necessarily exclusive to Fabric, correct? Is it possible to log in to on-prem with the CLI?

prescode commented 7 months ago

Based on my research, it is possible to use Entra ID to log in to an on-prem instance of SQL Server starting with SQL Server 2022 (16.x) if server is registered with Azure Arc. Which would make it technically possible to use the Azure CLI credentials to login. I don't have an environment to try it out on my own. I'm unsure if that is the spirit of the question. Is the question should this adapter support using the Azure CLI authentication method? I would say yes, connecting to Azure SQL Server instances should still be supported. To my knowledge dbt-fabric connects to Synapse Data Warehouse and not Azure SQL Server instances.

schlich commented 7 months ago

That is definitely helpful info and fully within the spirit of the question! You are welcome to try and get an environment going -- happy to answer any questions or do what I can to make environment setup easier-- but this info is helpful on its own, at least from my perspective, not knowing what kind of research @cody-scott has done on this or how these restrictions might be checked against in the adapter/test. One outstanding question I have is if this would be considered a breaking change from previous functionality of the adapter -- if not, maybe we should merge the update PR before implementing this.

prescode commented 7 months ago

I'm still getting up-to-speed on the code and now I understand what @cody-scott was saying. We are importing the dbt-fabric adapter and delegating to it for connections. The dbt-fabric adapter already has these same tests, for us to run them is redundant. The functionality will not change, this adapter will support Azure CLI auth via the dbt-fabric adapter. So the real question is do we want to run these redundant tests? I think we can leave them out.

schlich commented 7 months ago

that makes sense to me. I'll close this out, I now our next steps is to just copy over the new implementations of the breaking tests from fabric as cody outlines here.

Should be an easy change if you wanna take that up as well, at this point its first come first serve between the 3 of us