Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Post/Pre deployment scripts support #9

Closed
molszews opened this issue Apr 25, 2020 · 25 comments
Closed

Post/Pre deployment scripts support #9

molszews opened this issue Apr 25, 2020 · 25 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@molszews
Copy link

Seems like these are not supported.

I've played a little bit with appending such scripts, seems like simple appending of the .sql to resulting dacpac (zip) should work (https://the.agilesql.club/2019/03/how-can-we-merge-multiple-dacpacs-into-one/), however, I'm not sure how to tackle sqlcmd syntax support.
Running sqlcmd -i postdeployment_entryfile.sql -e -r >postdeploy.sql generates correct file, but bundling sqlcmd within the target project may be difficult?

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 3, 2020

Any thoughts about the design of this?

  <ItemGroup>
    <PostDeploy Include="Postdeployment\Script.PostDeployment.sql" />
    <PreDeploy Include="Predeployment\Script.PreDeployment.sql" />
  </ItemGroup>

If this is passed to the command line tool, maybe we can just expect sqlcmd to be present, and run it to create the consolidated script(s), and add it like this: https://github.com/GoEddie/DacpacMerge/blob/master/src/MergeEm/Program.cs#L118

(I have not verified the PreDeployment .sqlproj syntax)

@molszews
Copy link
Author

molszews commented May 3, 2020

well, using sort of naming convention won't be enough? there can be only one post/pre deploy script, we can simply assume it should be available under Post-deployment\postdeploy.sql

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 3, 2020

You can link multiple script with r: sqlcmd syntax - and we do.

@molszews
Copy link
Author

molszews commented May 3, 2020

yeah, but you specify only the entry point inside the .csproj, after that you can link every other file?
btw, sqlcmd could be embedded in a way this package embeds chromedriver binary https://github.com/jsakamoto/nupkg-selenium-webdriver-chromedriver/#where-is-chromedriverexe-saved-to

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 3, 2020

But for v1, we could of course just go by convention, and expect the script to be merged.

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 3, 2020

Not sure sqlcmd is simply a single .exe..

@jmezach
Copy link
Member

jmezach commented May 3, 2020

I wonder how SSDT treats those scripts. I'm guessing it may just be added to the model as is and the passed to sqlcmd at deployment time. Does anyone have an example .dacpac that is build by SSDT which includes a pre- and/or postdeployment script that they are willing to share so I can have a look?

@molszews
Copy link
Author

molszews commented May 3, 2020

Resulting .dacpac archive contains a single .sql file which looks like the result of sqlcmd command I've posted before. It looks like there is nothing in the model related to that postdeploy.sql, it is simply appended to the zip archive. I will try to provide some samples

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 3, 2020

If you email me, I can send you one privately.

@molszews
Copy link
Author

molszews commented May 4, 2020

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 4, 2020

I suggest that for initial implementation:

1: We assume the script is consolidated (the user can do this as a pipeline/build step)

2: We assume the folders follow the naming convention already specified here.

3: We assume the .sqlproj syntax will point to the consolidated script:

<ItemGroup>
    <PostDeploy Include="Post-Deployment\Script.PostDeployment.sql" />
    <PreDeploy Include="Pre-Deployment\Script.PreDeployment.sql" />
  </ItemGroup>

Happy to update the command line tool to support this, if we decide to go ahead!

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 4, 2020

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 5, 2020

@jmezach Can I start work on this? Or are you already full speed ahead?

@jmezach
Copy link
Member

jmezach commented May 5, 2020

Yeah, go for it :)

ErikEJ referenced this issue in ErikEJ/MSBuild.Sdk.SqlProj May 5, 2020
This was referenced May 5, 2020
@jmezach
Copy link
Member

jmezach commented May 6, 2020

@ErikEJ @molszews A new beta package is now up on NuGet for you to try out. This should have support for pre/post deployment scripts, if everything is in the same file. I'm still working on the merge logic.

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 6, 2020

@jmezach Fantastic! Will give it a try asap!

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 6, 2020

image

I am now able to build a "buddy" .dacpac to my .sqlproj project, and any differences in package content are not of importance. The postdeploy files even match!

(I am keeping the .sqlproj to get the good designer / IntelliSense experience in VS)

I will deploy the new .dacpac to DEV tomorrow.

My project file (not in the same folder as the .sqlproj) with linked items (so the .csproj is basically residing in an empty folder):

<Project Sdk="MSBuild.Sdk.SqlProj/1.1.0-beta.11">
  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <SqlServerVersion>SqlAzure</SqlServerVersion>
    <AnsiNulls>True</AnsiNulls>
    <QuotedIdentifier>True</QuotedIdentifier>
    <CompatibilityMode>150</CompatibilityMode>   
  </PropertyGroup>

  <ItemGroup>
    <SqlCmdVariable Include="DbReaderPassword">
      <DefaultValue>
      </DefaultValue>
      <Value>$(SqlCmdVar__2)</Value>
    </SqlCmdVariable>
    <SqlCmdVariable Include="DbUserPassword">
      <DefaultValue>
      </DefaultValue>
      <Value>$(SqlCmdVar__1)</Value>
    </SqlCmdVariable>
  </ItemGroup>

  <ItemGroup>
    <Content Include="..\..\Database\dbo\**\*.sql" />
  </ItemGroup>
 
  <ItemGroup>
    <PostDeploy Include="..\..\Database\Post-Deployment\Script.PostDeployment.sql" />
  </ItemGroup>

</Project>

@jmezach
Copy link
Member

jmezach commented May 6, 2020

Very nice. Glad to see it's coming together. Did you try deploying the resulting .dacpac yet? Curious to see if it actually runs at deployment time. @molszews Have you had time to give it a try yet?

I made some progress on the merging of scripts today. Interestingly it works fine on my dev machine, but fails on CI. For some reason it can't find the included file. Hope to do some more work on that soon.

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 6, 2020

Did you try deploying the resulting .dacpac yet?

No, I need PR review etc. before I can deploy to DEV - will do so tomorrow

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 7, 2020

I can now confirm, that deploying my package with both postdeploy and sqlcmd variables using Azure Pipelines to our DEV environment works as expected. I believe #6 can be closed now.

@jmezach
Copy link
Member

jmezach commented May 7, 2020

Great, thanks for reporting back. I'll close #6 when I've released a stable version. Do the pre/post-deployment scripts work as well?

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 7, 2020

Do the pre/post-deployment scripts work as well?

Yes, it does! (Only using post)

@jmezach
Copy link
Member

jmezach commented May 7, 2020

As I mentioned yesterday I did make some progress on the merging of SQLCMD files. I'm trying to rely on existing code from the Microsoft assemblies to do it, so that we don't have to reinvent the wheel, even though those API's are internal. Unfortunately it looks like the existing code isn't very cross-platform friendly. When you reference a script using something like :r ./Script1.sql it turns that into an absolute path, but also uppercases the entire path. Then it checks to see if that file exists, which obviously doesn't work on platforms that have a case sensitive filesystem.

If I run CI on Windows it will work just fine, but I would like for things to stay truly cross-platform. Would it be ok for you guys if I ship what we have right now as 1.1.0 and create a separate issue for the merging of files that may or may not come in a future version?

@ErikEJ
Copy link
Collaborator

ErikEJ commented May 7, 2020

Would it be ok for you guys if I ship what we have right now as 1.1.0

Absolutely, this has been my scope all the way - just document the limitation in the readme.

@jmezach
Copy link
Member

jmezach commented May 7, 2020

This has been released in 1.1.0. README has been updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants