Emit database names in manifest. #98
Conversation
| return builder.WithEnvironment((context) => | ||
| { | ||
| if (context.PublisherName == "manifest") | ||
| { | ||
| context.EnvironmentVariables[$"{ConnectionStringEnvironmentName}{connectionName}"] = $"{{{postgresBuilder.Component.Name}.connectionString}}"; | ||
| var manifestConnectionString = databaseName == null ? $"{{{postgresBuilder.Component.Name}.connectionString}}" : $"{{{postgresBuilder.Component.Name}.databases.{databaseName}.connectionString}}"; |
There was a problem hiding this comment.
IMO - I wouldn't make it this complicated. I would think this should be good enough:
"env": {
"ConnectionStrings__postgres": "{postgres.connectionString};Database=catalogdb"
},
That way there's no need for a new annotation, and no need to write the list of databases into the "type", "postgres.v1" object.
@ellismg - will string interpolation like I have above work correctly in azd? Get the connectionString from the postgres resource, and then append ;Database=catalogdb to it.
There was a problem hiding this comment.
@ellismg - will string interpolation like I have above work correctly in azd? Get the connectionString from the postgres resource, and then append ;Database=catalogdb to it.
Not today - but we could make it work. The reason we didn't allow this was because we felt we'd need to come up with a way to escape the {. If we want to say you put in a literal { or } with a \{ or \} and escape a literal \ with \\, it would be easy enough to implement.
There was a problem hiding this comment.
There are two parts to making this emission work. One part is emitting the connection string, but the other part is emitting the list of databases on the postgres component.
There was a problem hiding this comment.
but the other part is emitting the list of databases on the postgres component.
why do we need that?
There was a problem hiding this comment.
So that the thing that uses the manifest can create the database. You don't generally give apps that level of privs to the database server. In fact even DDL access is a bit too much.
|
Yeeting this PR into void due to some upcoming changes in the way we are going to model sub database resources. |
Minimal implementation of emitting the database names postgres components. This should go in AFTER #84 (will need to react to changes there).
Fix #104