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

The argument for AddDatabase represents a database and a connection string name at the same time #1199

Closed
sebastienros opened this issue Dec 5, 2023 · 8 comments · Fixed by #2346
Assignees
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Comments

@sebastienros
Copy link
Member

Let's take this example for PostgreSQL:

var catalogDb = builder.AddPostgresContainer("postgres").AddDatabase("catalogdb");

image

"catalogdb" is both the name of the database (a technical name), and the name of the resource (a logical name).

As a developer when I read AddDatabase(string name) I understand that is can be a database name, as in CREATE DATABASE "catalogdb".

But in the app when I do builder.AddNpgsqlDbContext<CatalogDbContext>("catalogdb") it's asking for a connectionName.

Though the documentation for AddDatabase(string name) is clear:

The name of the resource. This name will be used as the connection string name when referenced in a dependency.

It seems like we should be able to provide a database name and a resource name (or connection string name), because we might want them to be different, or change independently.

The name argument should also be changed to remove the ambiguity.

@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 5, 2023
@davidfowl
Copy link
Member

Are you going to make this change @sebastienros ?

@sebastienros
Copy link
Member Author

I am willing to, was waiting for some kind of triage before, and maybe comments.

@sebastienros
Copy link
Member Author

@davidfowl I don't think it an ship in next preview, right?

@ohroy
Copy link

ohroy commented Jan 8, 2024

any news ?
resouce name and db should not always same, at least we can choose it by optional param?

@DamianEdwards
Copy link
Member

We should do this for next preview. Thoughts @mitchdenny?

@mitchdenny
Copy link
Member

I think we need to do this (created an issue recently for the same thing). Next preview seems fine. I think the usage of IResource.Name is quite pervasive so each usage will have to be scrutinized, especially where connection strings are concerned.

There is a related aspect to this in the context of the proposed metadata field (#1619). For cloud deployment tools we may wish to have a different resource name than that specified in the app model - especially when approaching brownfields work.

@mitchdenny
Copy link
Member

@sebastienros are you still working on this? Loop in @ellismg for the AZD side of things.

@sebastienros
Copy link
Member Author

Yes I will work on that very soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants