rr-wfm / MSBuild.Sdk.SqlProj

An MSBuild SDK that provides similar functionality to SQL Server Data Tools (.sqlproj) projects
MIT License
409 stars 45 forks source link

Pass SQLCMD variables at build time #39

Closed lewism36 closed 3 years ago

lewism36 commented 4 years ago

Is it possible to reference objects defined in a referenced package? In a view for example.

I can't see how to specify the DatabaseSqlCmdVariable or an equivalent, like when adding a project reference to a sqlproj, but I'm also not sure if the SqlCmd variable is being expanded with the default value anyway.

Without this the build fails with validation errors e.g.

1>Target CoreCompile: 1> Using package name Database4 and version 1.0.0 1> Using SQL Server version Sql150 1> Adding reference to {}\.nuget\packages\database3\1.0.0\tools\Database3.dacpac 1> Adding SqlCmd variable Database3 1> Adding {}\Database4\View2.sql to the model 1> EXEC : error SQL71561: Error validating element [dbo].[View2]: SqlView: [dbo].[View2] has an unresolved reference to object [$(Database3)].[dbo].[Table2].[Id]. 1> EXEC : error SQL71561: Error validating element [dbo].[View2]: SqlView: [dbo].[View2] has an unresolved reference to object [$(Database3)].[dbo].[Table2]. 1> EXEC : error SQL71561: Error validating element [dbo].[View2].[Id]: SqlComputedColumn: [dbo].[View2].[Id] has an unresolved reference to object [$(Database3)].[dbo].[Table2].[Id]. 1> Found 3 error(s), skip building package

jmezach commented 4 years ago

@lewism36 Thanks for reporting this. I think we're currently not passing values for SQLCMD variables in the build phase. Are you willing to share your .csproj and the relevant sections from your .sqlproj so that we can try to figure out how to fix this?

lewism36 commented 4 years ago

@jmezach Thanks for looking into this. I've shared an example in here.

jmezach commented 4 years ago

Looking at the example I think there's actually two things that are relevant here. The first is that this requires the value of SQLCMD variables to be passed at build time. For example, given the following excerpt from the project file:

<ItemGroup>
  <SqlCmdVariable Include="Database1">
    <DefaultValue>Database1</DefaultValue>
    <Value>$(SqlCmdVar__1)</Value>
  </SqlCmdVariable>
</ItemGroup>

We would need to pass a SQLCMD variable to the DacpacTool from Sdk.targets for the CoreCompile target and supply either the value of the MSBuild SqlCmdVar__1 property or the default value of Database1. Currently we only pass the name of the SQLCMD variable, but no value for it, so we'll probably need to change the DacpacTool to set the value as well. We do something similar in the recently added publish support (#33) so that shouldn't be too hard to do.

The other thing is that the examples provided use ProjectReference instead of PackageReference which is not something we currently support. Since this is a separate concern I've filed a separate issue (#40) to solve that. I'll rename this issue to reflect what's actually going on.

lewism36 commented 4 years ago

Thanks - I forgot to mention that Database1 and Database2 were the examples using the old sqlproj style. Database3 and Database4 are the two where I was replicating the same setup with this sdk.

lewism36 commented 4 years ago

I think ProjectReference would be useful too though

ActivationRequired commented 4 years ago

@jmezach This enhancement is needed if a database project has a reference to a linked server and SQLCMD variables are being used. The build will fail for instance when creating a view. It seems this would be easy to implement by just extending the AddSqlCmdVariables extension method on TSqlModel and updating how Sdk.targets build processes the SqlCmdVariableArguments. Also, what about using appsetting.json files and IConfigurationBuilder to load the BuildOptions and DeployOptions.

jmezach commented 4 years ago

Yeah, I don't think it is very hard to implement, just haven't gotten around to it yet. We'd have to do some gymnastics to resolve the values for the variables though. As shown in the example earlier there's a value and a default value item metadata where the value is referencing an MSBuild property. We'll probably have to do something like if the Value metadata doesn't have a value, then pass the default value and pass that to the command line tool.

I'm not sure I understand your other remark about using the IConfigurationBuilder. How would that help us? Passing the necessary inputs as command line parameters has worked quite well so far, except for #43 but that has been fixed already.

ActivationRequired commented 4 years ago

I also ran into the limitation of #43. I guess my thought about using the appsettings.json files could reduce all the parameters being set in the .csproj file. This could be used similarly to a publish profile.

I referenced the IConfigurationBuilder because I was thinking about the possibility of using the secrets manager, environment variables, and the AWS Parameter store middle-ware to add SQLCMD variable values. Which probably is not suited for a generic MSBuild project SDK. Would it be possible to publish a Core NuGet package that would provide the base methods DeployDacpac and BuildDacpac?

jmezach commented 4 years ago

Of course we could extract the core logic into a separate NuGet package and ship that, but I don't think it would add much on top of what Microsoft.SqlServer.DACFX already provides so I don't see much value in doing that.

Perhaps I'm struggling to understand what you're trying to achieve. Could you elaborate a little bit on that? I don't think there's a lot of parameters you'd need to put into the project file since we have sane defaults for most of them. I guess I can see a need for having something similar to publishing profiles, although to be honest the whole publishing pipeline that we introduced in 1.2.0 is mostly designed around the developers inner-loop and not geared towards actual production deployments. For that you should have a look at SqlPackage.exe.

kvchernyaev commented 3 years ago

Hello! Is it right that i cannot use select * from [$(OtherDatabase)].dbo.AnotherTable now? And that this issue is about that?

jmezach commented 3 years ago

@kvchernyaev That depends on where you're using it and what error message you receive. Could you elaborate on that?

kvchernyaev commented 3 years ago

I need to create a view that consolidates data from tables in my DB and other DB - it is a report in microservises architecture. In .sqlproj i wrote

OtherDatabaseName $(SqlCmdVar__3) OtherDatabase {c94fe83e-2ad1-4d30-afdf-5bd9b39a3805} True False OtherDatabase

And in rep.myreport.sql: create view rep.myreport as select * from [$(OtherDatabase)].dbo.AnotherTable

But for <Project Sdk="MSBuild.Sdk.SqlProj/1.10.0"> i can not do such way, yes?

kvchernyaev commented 3 years ago

And DB name must be in a variable - it is not the same for different servers (test, prod etc)

jmezach commented 3 years ago

Yes that sounds like this issue. We're not currently working on this. If you feel this is important to you please consider submitting a PR.

jeffrosenberg commented 3 years ago

@jmezach I think this is resolved by #131?

jmezach commented 3 years ago

@jeffrosenberg Yes, that does seem to be the case. I'll go ahead and close this. @lewism36 Feel free to re-open if you still think there's something not working here.