-
Notifications
You must be signed in to change notification settings - Fork 542
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Support Existing Azure AppInsights, OpenAI, and CosmosDB #7677
Conversation
These resources don't support existing resources yet. Adding support to finish all the Azure resources for existing. Note that for CosmosDB, we only explicitly set DisableLocalAuth if WithAccessKeyAuthentication is called. We don't want to disable local auth on an existing resource if it is already enabled as that might break other uses of the resource. Fix dotnet#7665
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (10)
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:71
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:204
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:332
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:505
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:566
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:627
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:688
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:885
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:1154
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs:1278
- The 'output.WriteLine(BicepText);' statement is redundant and should be removed.
output.WriteLine(BicepText);
…d Redis Existing Azure resources can't be directly updated in bicep. When trying to deploy these changes it raises an error "Error BCP173: The property "properties" cannot be used in an existing resource declaration. [https://aka.ms/bicep/core-diagnostics#BCP173]"
Tags = { { "aspire-resource-name", infrastructure.AspireResource.Name } } | ||
}; | ||
infrastructure.Add(cosmosAccount); | ||
DisableLocalAuth = disableLocalAuth, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to this. Setting this directly on the resource instead of after the fact reads better as well.
…e added to the infrastructure in the opposite order now.
/backport to release/9.1 |
Started backporting to release/9.1: https://github.com/dotnet/aspire/actions/runs/13422889047 |
Description
These resources don't support existing resources yet. Adding support to finish all the Azure resources for existing.
Note that for CosmosDB and Redis, we cannot explicitly set DisableLocalAuth on an existing resource because bicep doesn't support it.
Fix #7665
Checklist