Change DOTNET_ to ASPIRE_ in config variables #8336
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DamianEdwards
requested review from
ReubenBond,
eerhardt,
mitchdenny and
radical
as code owners
Contributor
There was a problem hiding this comment.
Copilot reviewed 58 out of 78 changed files in this pull request and generated no comments.
Files not reviewed (20)
- playground/AspireEventHub/EventHubs.AppHost/Properties/launchSettings.json: Language not supported
- playground/AzureContainerApps/AzureContainerApps.AppHost/Properties/launchSettings.json: Language not supported
- playground/AzureFunctionsEndToEnd/AzureFunctionsEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/AzureSearchEndToEnd/AzureSearch.AppHost/Properties/launchSettings.json: Language not supported
- playground/AzureServiceBus/ServiceBus.AppHost/Properties/launchSettings.json: Language not supported
- playground/AzureStorageEndToEnd/AzureStorageEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/BrowserTelemetry/BrowserTelemetry.AppHost/Properties/launchSettings.json: Language not supported
- playground/CosmosEndToEnd/CosmosEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/CustomResources/CustomResources.AppHost/Properties/launchSettings.json: Language not supported
- playground/DatabaseMigration/DatabaseMigration.AppHost/Properties/launchSettings.json: Language not supported
- playground/Elasticsearch/Elasticsearch.AppHost/Properties/launchSettings.json: Language not supported
- playground/HealthChecks/HealthChecksSandbox.AppHost/Properties/launchSettings.json: Language not supported
- playground/OpenAIEndToEnd/OpenAIEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/OracleEndToEnd/OracleEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/ParameterEndToEnd/ParameterEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/PostgresEndToEnd/PostgresEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/ProxylessEndToEnd/ProxylessEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
- playground/Qdrant/Qdrant.AppHost/Properties/launchSettings.json: Language not supported
- playground/Redis/Redis.AppHost/Properties/launchSettings.json: Language not supported
- playground/SqlServerEndToEnd/SqlServerEndToEnd.AppHost/Properties/launchSettings.json: Language not supported
github-actions
bot
added
the
area-app-model
eerhardt
reviewed
Mar 27, 2025
src/Aspire.ProjectTemplates/templates/aspire-apphost/8.2/Properties/launchSettings.json
Outdated
Show resolved
Hide resolved
eerhardt
reviewed
Mar 27, 2025
src/Aspire.Dashboard/Configuration/PostConfigureDashboardOptions.cs
Outdated
Show resolved
Hide resolved
JamesNK
reviewed
Mar 27, 2025
JamesNK
reviewed
Mar 28, 2025
Co-authored-by: James Newton-King <[email protected]>
Co-authored-by: James Newton-King <[email protected]>
JamesNK
reviewed
Mar 28, 2025
Comment on lines
-110
to
+111
| startInfo.EnvironmentVariables["ASPIRE_BACKCHANNEL_PATH"] = socketPath; | ||
| startInfo.EnvironmentVariables[KnownConfigNames.UnixSocketPath] = socketPath; |
Member
There was a problem hiding this comment.
Not a change in this PR, but related to passed in parameters:
All dashboard parameters start with ASPIRE_DASHBOARD. Should these CLI args start with ASPIRE_CLI?
Member
There was a problem hiding this comment.
Edit: I see these aren't strictly related to just CLI. Still, structure of names is something to keep in mind. I think it's good that all dashboard parameters have a consistent prefix.
| private static void WaitForDebugger() | ||
| { | ||
| if (Environment.GetEnvironmentVariable("ASPIRE_WAIT_FOR_DEBUGGER") == "true") | ||
| if (Environment.GetEnvironmentVariable(KnownConfigNames.WaitForDebugger) == "true") |
Member
There was a problem hiding this comment.
Is there a reason Environment is used directly in a couple of places instead of IConfiguration? cc @mitchdenny
If IConfiguration not available yet? Should have a comment whereever Environment.GetEnvironmentVariable is used to explain why it doesn't follow the convention used everywhere else.
Co-authored-by: James Newton-King <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Changes from
DOTNET_toASPIRE_in all environment variable and config key names. In some cases it replaces it withASPIRE_. Old keys still work of course, but new ones are preferred.Fixes #8009
Checklist