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

Add support for deploying dacpacs from referenced NuGet packages #334

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

jmezach
Copy link
Contributor

@jmezach jmezach commented Dec 16, 2024

Closes #327

This adds support to the SQL Database Projects integration to deploy a .dacpac from a referenced NuGet package. It works by including some MSBuild magic to create classes that inherit from the new IPackageMetadata interface. This interface exposes the Package ID, Version and the physical path to the package in the users NuGet package cache.

We then leverage this metadata at runtime to find the correct path to the .dacpac and use the existing infrastructure for deploying said .dacpac to a target SQL Server.

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • [] New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

@jmezach jmezach marked this pull request as ready for review December 17, 2024 15:30
@jmezach jmezach requested a review from ErikEJ as a code owner December 17, 2024 15:30
Signed-off-by: Jonathan Mezach <[email protected]>
Move targets file into package

Signed-off-by: Jonathan Mezach <[email protected]>
Signed-off-by: Jonathan Mezach <[email protected]>
Add integration test

Signed-off-by: Jonathan Mezach <[email protected]>
@jmezach jmezach force-pushed the feature/sqldbproj-packages branch from 3106cb5 to 7f72f70 Compare December 17, 2024 15:31
@jmezach
Copy link
Contributor Author

jmezach commented Dec 17, 2024

@aaronpowell @ErikEJ I think this is ready for review now :).

Copy link
Contributor

@ErikEJ ErikEJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the slightest nitpick in formatting

Signed-off-by: Jonathan Mezach <[email protected]>
@jmezach
Copy link
Contributor Author

jmezach commented Dec 18, 2024

@aaronpowell Do we publish NuGet packages from these PR's? I wanted to make sure that the NuGet package actually contains the MSBuild logic so this will work end-to-end in a real project ;)

@jmezach
Copy link
Contributor Author

jmezach commented Dec 18, 2024

@aaronpowell Never mind, I've downloaded the NuGet packages from the CI pipeline artifacts and added those to a sample project and it all seems to work as expected. So I guess we're good to go here :).

@jmezach
Copy link
Contributor Author

jmezach commented Dec 19, 2024

Now that I've tried this out on my own I'm wondering if we should make a change. Specifically related to the resource being returned by AddSqlPackage. This currently returns a SqlProjectResource which I guess can be confusing for some. But more importantly it is not easy to distinguish in code between different SqlPackageResources (if we call it that). That kind of makes it hard to further customize those resources.

For example, one of the scenarios I'm thinking of using this for is for our "ConfigurationDatabase". This is a central databased used by many of our services which contains a registry of tenants and their associated database(s). So, what I would like to provide is something like this:

var builder = DistributedApplication.CreateBuilder(args);

var sqlServer = builder.AddSqlServer("sql");
var configurationDatabase = sqlServer.AddDatabase("ConfigurationDatabase");
var tenantDatabase1 = sqlServer.AddDatabase("Tenant1");
var tenantDatabase2 = sqlServer.AddDatabase("Tenant2");

builder.AddSqlPackage<Packages.ConfigurationDatabase>("ConfigurationDatabase-Schema")
       .WithReference(configurationDatabase)
       .WithTenantDatabase("Tenant1", tenantDatabase1)
       .WithTenantDatabase("Tenant2", tenantDatabase2)

builder.Build().Run();

Obviously this would require implementing a WithTenantDatabase extensions method. But currently I'd add have to make that an extension method on IResourceBuilder<SqlProjectResource>, meaning you could call this on any database. But that won't work, since the extension method will make assumptions about the underlying schema of the database.

So perhaps we should have AddSqlPackage<TPackageMetadata> return a IResourceBuilder<SqlPackageResource<TPackageMetadata>> instead of a IResourceBuilder<SqlProjectResource>?

@aaronpowell
Copy link
Member

@aaronpowell Do we publish NuGet packages from these PR's? I wanted to make sure that the NuGet package actually contains the MSBuild logic so this will work end-to-end in a real project ;)

I see you found it, but also, they are published to the Azure Pipeline feed on PR (main will land on NuGet).

@aaronpowell
Copy link
Member

If there's the potential that you'd want to do different things depending on whether it's a Project or Package based setup, then I'd suggest that we want to return a different type.

But if it's possible that there would be methods that would make sense whether it's a Project or Package, should there be an interface that both SqlPackageResource and SqlProjectResource could implement (even if it's just an empty interface) so a method could have a constraint of where TResource : ISqlDeployResource (or whatever it gets called).

@jmezach
Copy link
Contributor Author

jmezach commented Dec 21, 2024

Technically I don't think there is a reason to differentiate between packages and projects. In the end both are based on a .dacpac, it is just a matter of where that .dacpac is being sourced from. So I guess it should have been a DacpacResource but that would require doing a breaking change at this point. Perhaps we could do this in a non-breaking way by adding an IDacpacResource interface that both SqlProjectResource and SqlPackageResource would implement?

My main point though was about being able to differentiate between multiple SqlPackageResources in code. If you're sourcing multiple different NuGet packages with different schemas I think you'd want to be able to target the different resources in your project.

@github-actions github-actions bot added the Stale label Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support deploying .dacpac from referenced NuGet packages
3 participants