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

SqlConnection.OpenWithCreate()? #4160

Open
mitchdenny opened this issue May 13, 2024 · 8 comments
Open

SqlConnection.OpenWithCreate()? #4160

mitchdenny opened this issue May 13, 2024 · 8 comments
Labels
area-components Issues pertaining to Aspire Component packages
Milestone

Comments

@mitchdenny
Copy link
Member

Context

One of the friction points that we have around databases in .NET Aspire is that for local development we don't automatically create the databases for the user. In deployment scenarios we often do create the database because we emit (in the case of AZD) Bicep that creates the database resource for the various databases that we support in the tree.

I've been looking at various options to decorate the app model with some code to support creating the database but we need to be careful to not suddenly make the app host require client side dependencies just to support this scenario (which could lead to dependency management issues since the AppHost would logically have a dependency thing on everything).

We could create a special package for each database resource type which includes this creation logic but that seems like a sledge hammer.

Proposal

Stepping back a bit the main issue we have is that the database doesn't exist when you first got to connect to it. Tools like EFCore have some extensive plumbing to work around this where you can ensure that the database exists - but we don't have that if say you just want to use SqlConnection directly. The SqlConnection.Open(...) method doesn't allow us to specify that we want the database to be created if it doesn't exist.

But what if it did?

What if we had an API like this:

SqlConnection.OpenWithCreate()

We could potentially provide an extension method in the Aspire application libraries to plaster over this issue. It would mirror what is done in EF today where if the database does not exist it connects to the same server using the default database (it parses the connection string using the builder) and then calls CREATE DATABASE.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label May 13, 2024
@mitchdenny mitchdenny added this to the Backlog milestone May 13, 2024
@mitchdenny mitchdenny added area-components Issues pertaining to Aspire Component packages and removed area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication labels May 13, 2024
@mitchdenny
Copy link
Member Author

@eerhardt
Copy link
Member

I don't think this is the right level to provide this API. If this is generally valuable, it should be in SqlConnection itself. Aspire components generally don't try to wrap or augment the underlying library's APIs, but rather provide the glue necessary to augment an existing client library with the missing pieces to satisfy the "Aspire requirements" (telemetry, config, health checks, etc).

We should be asking ourselves "What about Aspire is making this a problem? Why don't others hit it?" I think the answer is:

By default, .NET Aspire starts the database server from scratch on every F5. Typical application development experiences against databases don't do this. Outside of the "app" someone creates the database server, the database, and the tables. This happens once and doesn't need to happen again on every F5.

We could create a special package for each database resource type

We did this already in #2986.

@davidfowl
Copy link
Member

@roji Thoughts?

@mitchdenny
Copy link
Member Author

If this is generally valuable, it should be in SqlConnection itself

I actually agree. And part of filing this issue and putting up the PR is to facilitate the conversation. What we'd basically be asking here is for the various ADO.NET providers to implement an overload for the Open() and OpenAsync() methods.

It would take some time for that to filter through the ecosystem, so the question is are we happy with folks having to implement this themselves in the meantime?

For example if https://github.com/dotnet/sqlclient decided to take this on, would we need to wait until .NET 9.0 for it?

@eerhardt
Copy link
Member

so the question is are we happy with folks having to implement this themselves in the meantime?

I don't think they need to implement this themselves. I think there are other things that can be done in the AppHost that would make this experience better:

  1. builder.AddSqlServer("server").AddDatabase("database"); actually creates the database.
  2. Databases use persisted data by default. So the DB isn't wiped away between F5s
  3. Optionally, database servers stay running and are re-connected between F5s, so I can look at and modify the data in the database without the whole app running.

@eerhardt
Copy link
Member

Note also that "database creation" is only the tip of the issue. The app also needs the tables created, and optionally seeded with data.

@davidfowl
Copy link
Member

The reason database creation comes up often is because it looks like the apphost does create the database (AddDatabase). The user didn't call AddTable so there shouldn't be an expectation that the tables would be created.

@mitchdenny
Copy link
Member Author

  • builder.AddSqlServer("server").AddDatabase("database"); actually creates the database.

I think this is probably the way customers expect it to work for local development. There are really two broad approaches to make this work:

  1. Add a dependency on Microsoft.Data.SqlClient to Aspire.Hosting.SqlServer and add a hook that creates the database once the server is online.
  2. Inject some script/configure an initialization container to create the database.

I've tried option 2 above but its so clunky that it makes me want to just do option 1 ... but I don't like that because it'll end up pulling all the client libraries into the AppHost as a dependency which could create a bit of a socialisation issue. Perhaps we can do it this way but provided we have enough abstraction we can change the implementation down the track.

  • Databases use persisted data by default. So the DB isn't wiped away between F5s

There is always a first run. I think if we don't provide people the ability to git clone then F5 then we've missed a big part of the promise of Aspire.

  • Optionally, database servers stay running and are re-connected between F5s, so I can look at and modify the data in the database without the whole app running.

I think we'll need to make this work for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-components Issues pertaining to Aspire Component packages
Projects
None yet
Development

No branches or pull requests

3 participants