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

Aspire.Npgsql component does not create database #1170

Open
alexwolfmsft opened this issue Dec 1, 2023 · 28 comments
Open

Aspire.Npgsql component does not create database #1170

alexwolfmsft opened this issue Dec 1, 2023 · 28 comments
Labels
area-integrations Issues pertaining to Aspire Integrations packages postgres Issues related to Postgres integrations
Milestone

Comments

@alexwolfmsft
Copy link

When providing a custom name for a database, the builder.AddPostgresContainer("pg").AddDatabase("postgresdb") methods do not seem to create the database in the Postgres container. For example, with the following code:

var postgresdb = builder.AddPostgresContainer("catalog").AddDatabase("catalogdb");

var postgres = builder.AddProject<Projects.AspireSQL>("aspirepostgres")
                       .WithReference(postgresdb);

And:

builder.AddNpgsqlDataSource("catalogdb");

I receive this error:
PostgresException: 3D000: database "catalogdb" does not exist

When I use psql commands in the container to view the list of databases, it looks like this:

image

Should I expect to see a "catalogdb" database listed here?

The connection and query does work if I switch to AddDatabase("postgres") to use the default 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 Dec 1, 2023
@DamianEdwards
Copy link
Member

This is by design. The database name is to ensure the appropriate connection information is provided but it's not intended that the database will be auto-created by the hosting APIs given all the variables that can impact database creation.

@eerhardt
Copy link
Member

eerhardt commented Dec 4, 2023

The database name is to ensure the appropriate connection information is provided but it's not intended that the database will be auto-created by the hosting APIs given all the variables that can impact database creation.

I think this is confusing, and not consistent with our other hosting APIs. The first call is "AddPostgresContainer", which creates a Postgres container. The second call is "AddDatabase", which doesn't create a database? That seems wrong that the same verb doesn't do the same thing.

@alexwolfmsft
Copy link
Author

alexwolfmsft commented Dec 4, 2023

@eerhardt It's interesting you mention this because that's the exact reason I assumed it auto generated the db - it autogenerates a redis setup for you using similar methods/naming.

@eerhardt
Copy link
Member

eerhardt commented Dec 4, 2023

Re-opening to discuss this a bit more. I'm not sure the current behavior is the best we can do.

@eerhardt eerhardt reopened this Dec 4, 2023
@alexwolfmsft
Copy link
Author

Re-opening to discuss this a bit more. I'm not sure the current behavior is the best we can do.

I know you've seen #1168 as well, but just wanted to point out the other db components don't autogenerate the database either - this seems to be a Redis vs Databases inconsistency.

Some .NET tools have historically autogenerated databases for you locally, though I think that was only when using EF.

@DamianEdwards
Copy link
Member

Redis is more akin to a service than a database, in that you get a Redis instance and connect to that. With databases, you typically get a server which hosts multiple databases, and the database is part of the connection detail. All that said, I'm more than happy to discuss further whether we think the Aspire.Hosting layer should do more to ensure the configured database is actually available.

/Cc @davidfowl @roji for their thoughts.

@mitchdenny
Copy link
Member

The moment you start auto creating the database you need to start specifying things like collations and that kind of stuff.

The other part of the problem is that you have to figure out where you want to provision the database. If you say you want to do it in the AppHost then suddenly the app host needs to have client drivers for each database which could be dependency hell.

Then you get into lifetime considerations such as the AppHost not being around during deployment so you still need something else to create the database anyway.

Better off to face these considerations up front.

It is possible for a deployment tool like AZD to be enhanced to support creating the database since in the Azure ARM world a database is actually a resource in the resource provider. But this is certainly cloud platform dependent.

@IEvangelist
Copy link
Member

IEvangelist commented Dec 5, 2023

Perhaps, we should consider renaming the AddDatabase API. It make more sense as AddDatabaseConnection to make it clearer, that it's not in fact adding a database, but rather a connection to one?

@mitchdenny
Copy link
Member

LOL ... it is funny how we keep recycling nouns :) We just removed AddPostgesConnection(...) ... and now looking to add AddPostgres(...).AddDatabaseConnection(...) which does something different.

I've never been super enthused about AddDatabase(...) myself so I'm more than open to renaming it. I'm not sure AddDatabaseConnection(...) is any better. Perhaps we need something like SelectDatabase(...).

@DamianEdwards
Copy link
Member

There's a PR out that adds a sample on how to initialize databases when using database containers now: dotnet/aspire-samples#61

@mitchdenny mitchdenny self-assigned this Jan 11, 2024
@mitchdenny mitchdenny added this to the Backlog milestone Jan 11, 2024
@mitchdenny
Copy link
Member

Flagging for offsite because this has come up a bit.

@mitchdenny
Copy link
Member

I think we are comfortable with where this is for now. When provisioning you can get the database created for you because the permission set for deployed resources is usually a bit stricter.

For local development a worker app to manage database creation is our current guidance.

We may revisit this when we look at commanding/tasking.

@mitchdenny mitchdenny removed their assignment Feb 23, 2024
@jakoss
Copy link

jakoss commented Apr 16, 2024

I agree that AddDatabase is confusing in this sense. I just wasted too much time getting this information. Renaming it would be nice, but i think more important would be to add information about that to the documentation. It's not mentioned anywhere here https://learn.microsoft.com/en-us/dotnet/aspire/database/sql-server-entity-framework-component?tabs=dotnet-cli . It's only briefly mentioned in the seed data https://learn.microsoft.com/en-us/dotnet/aspire/database/seed-database-data?tabs=sql-server . I didn't got to this part of tutorial since i didn't succeed in connecting in the first place.

This is worse in the case of SqlServer i believe, since i got an exception when trying to connect to the database. SqlServer is verifying the Database=name part of connection string. If the database does not exist it throws with authentication exception which is misleading too. So even if i wanted to create the database from code - i can't, the connection string will not work.

I think it would be even better to create those databases as part of the AppHost configuration though

@DamianEdwards
Copy link
Member

This is worse in the case of SqlServer i believe

I'm fairly certain this happens for other database engines too like PostgreSQL.

So even if i wanted to create the database from code - i can't, the connection string will not work

Entity Framework Core actually handles this specifically in its database creation routines and reconnects to the database with a modified connection string (by stripping off the database name). There's no reason your apps can't do something similar (assuming you're not using EF Core).

I think it would be even better to create those databases as part of the AppHost configuration though

The resistance to this is that that requires having the database client referenced by the AppHost project and the logic to check for and if not present create the database needs to be specifically implemented for every different database hosting extension (as it's not universal). This is exactly what provider-based ORM libraries like Entity Framework Core exist to do and we're hesitant to reimplement that in the Aspire hosting layer. All that said, I acknowledge that it still might be the right thing to do as this has been a consistent piece of feedback we've had since the first preview.

@jakoss
Copy link

jakoss commented Apr 16, 2024

Entity Framework Core actually handles this specifically in its database creation routines and reconnects to the database with a modified connection string (by stripping off the database name). There's no reason your apps can't do something similar (assuming you're not using EF Core).

That does explain my issue. I have one service with normal DbContext using postgres and it's working fine. Other service is using SqlServer and it's a Identity Server. I'm trying to set persistence to Identity Server and it's not using DbContext, hence the error i guess.

The resistance to this is that that requires having the database client referenced by the AppHost project and the logic to check for and if not present create the database needs to be specifically implemented for every different database hosting extension (as it's not universal). This is exactly what provider-based ORM libraries like Entity Framework Core exist to do and we're hesitant to reimplement that in the Aspire hosting layer. All that said, I acknowledge that it still might be the right thing to do as this has been a consistent piece of feedback we've had since the first preview.

Understandable, I guess it's not really that bad. But I'm quite sure it should be documented better, both in the learn.microsoft.com and in the code docs :)

@holytshirt
Copy link

I just hit this and though I was doing something wrong for hours. I did not realise the database was not created.
In docker compose

    image: postgres:16-alpine
    environment:
      - POSTGRES_USER=postgres
      - POSTGRES_PASSWORD=postgres
      - POSTGRES_DB=zitadel

And database is there ,,,

@DamianEdwards
Copy link
Member

@holytshirt yes the PostgreSQL container supports changing the name of the default database via the POSTGRES_DB environment variable, as does the MySQL container. The DatabaseContainers sample shows this. However not all database containers have this mechanism, including the SQL Server container.

We're still considering what we might do to have databases specified via AddDatabase be automatically created if they're not present but haven't settled on a plan as yet.

@ozzioma
Copy link

ozzioma commented Jun 5, 2024

Is there a scenario for connecting to an existing database?
That is without provisioning a container for one?

@DamianEdwards
Copy link
Member

Yes, you can use builder.AddConnectionString to model an existing database instance

@atrauzzi
Copy link

atrauzzi commented Jul 9, 2024

@mitchdenny -- is #4160 an evolution to this conversation? 🙏

@mitchdenny
Copy link
Member

No we provably won't go that direction. We are considering allowing the hosting packages to reference client libraries to support doing things like creating databases for local development.

@atrauzzi
Copy link

atrauzzi commented Jul 9, 2024

Sounds good, I'm sure you guys will sort out the best approach.

Any good options in the interim? I'd really like to champion the gradual adoption of Aspire in my group, but I hope I'm not jumping into another hurry-up-and-wait. 😰

@manojlds
Copy link

So how am I supposed to handle this? This is trivial with existing docker-compose setups.

@eerhardt
Copy link
Member

This is trivial with existing docker-compose setups.

How do you do it with docker-compose?

@Matthewsre
Copy link

I encountered the same confusion with .AddDatabase(...) not actually creating the database. I see the value in supporting both creating the database and just referencing it.

Here are a few options that make sense to me:

Option 1: Prefix Conventions (Add, With, ...)

  • Modify .AddDatabase() to create the database. This aligns with user expectations where "Add" implies creation, even though it's a breaking change.
  • Introduce .WithDatabase() (or .WithDatabaseReference()) to include database details without creating it, providing clear distinction. Might break expectations around "With" prefix usage for chaining expectations.
    (Any good alternatives for a prefix that makes more sense?)

Option 2: Suffix Conventions (Reference, ReferenceOnly, ...)

  • Same as Option 1, but introduce .AddDatabaseReference() (or .AddDatabaseReferenceOnly()). This is really just an alternative name to the proposal by @IEvangelist. This might make sense since the “Add” prefix also seems to have an expected behavior of providing a new output compared to the “With” prefix used for chaining.

Option 3: Optional Argument (create = true)

  • Add an optional argument to .AddDatabase(…, bool create = true). This avoids breaking changes by defaulting to the current behavior, offering a short-term solution outside major version releases.

Additionally, I would like to see these same considerations made for containers/collections for resources such as CosmosDB and MongoDB.

For context, I use Terraform to build environments (dev, test, ppe, prod), which creates databases and sometimes containers/collections. I'm transitioning to Aspire to simplify dev environment configurations and reduce/remove Terraform. Beyond the dev environment, I plan to use Aspire for generating manifests compatible with Terraform, Pulumi, Bicep, etc. (something similar to one of my examples here: terraform-azurerm-microservices/examples/exampleB/main.tf at master · Matthewsre/terraform-azurerm-microservices (github.com))

@rsalus
Copy link

rsalus commented Aug 6, 2024

I will admit I also found this confusing when I was starting out, but I also did not have much familiarity with EF core at the time. Looking back, I think this is more of a documentation issue than an architectural one. Covering the basics of actually interacting with the DB (seeding, migrations, etc) when introducing the Aspire functionality would have gone a long way IMO.

@Alirexaa
Copy link
Contributor

Alirexaa commented Aug 6, 2024

My proposed API is to introduce WithDatabaseCreation in another layer that referenced two projects; Aspire.Hosting.PostgreSQL and Aspire.Npgsql or Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.
It could be more helpful to have this layer to do more things like WithDatabaseRestore
and also we could provide settings to completely configure Database properties some collation with WithDatabaseCreation as @mitchdenny says in #1170 (comment)

@davidfowl
Copy link
Member

@mitchdenny Is working on infrastructure that will make this possible.

@davidfowl davidfowl added area-integrations Issues pertaining to Aspire Integrations packages feature postgres Issues related to Postgres integrations and removed area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication area-app-model-offsite labels Sep 7, 2024
@davidfowl davidfowl removed the feature label Oct 16, 2024
@eerhardt eerhardt marked this as a duplicate of #4813 Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-integrations Issues pertaining to Aspire Integrations packages postgres Issues related to Postgres integrations
Projects
None yet
Development

No branches or pull requests