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

Use InputReference in more places #2642

Merged
merged 2 commits into from
Mar 5, 2024
Merged

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Mar 5, 2024

Model the Password inputs on Containers and the Value input on Parameters as an InputReference and use their ValueExpressions instead of hard-coding the strings.

Microsoft Reviewers: Open in CodeFlow

Model the Password inputs on Containers and the Value input on Parameters as an InputReference and use their ValueExpressions instead of hard-coding the strings.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Mar 5, 2024
@@ -36,7 +36,7 @@ public static IResourceBuilder<MySqlServerResource> AddMySql(this IDistributedAp
{
if (context.ExecutionContext.IsPublishMode)
{
context.EnvironmentVariables.Add(PasswordEnvVarName, $"{{{resource.Name}.inputs.password}}");
context.EnvironmentVariables.Add(PasswordEnvVarName, resource.PasswordInput.ValueExpression);
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of this if statement and stash the input reference directly in the env variable?

Suggested change
context.EnvironmentVariables.Add(PasswordEnvVarName, resource.PasswordInput.ValueExpression);
context.EnvironmentVariables[PasswordEnvVarName] = resource.PasswordInput;

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't get rid of the if statement unless InputReference implements IValueProvider. Right now we don't actually have the value (i.e. the password) stored on the InputAnnotation.

I'll see if I can make this change in the next PR.

@eerhardt eerhardt enabled auto-merge (squash) March 5, 2024 19:47
@eerhardt eerhardt merged commit a80c5df into dotnet:main Mar 5, 2024
8 checks passed
@eerhardt eerhardt deleted the UseInputReference branch March 5, 2024 22:44
@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2024
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 this pull request may close these issues.

2 participants