-
Notifications
You must be signed in to change notification settings - Fork 696
Fix missing 'name' property in TableService Bicep generation #10334
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
Conversation
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
This doesn't work because of Azure/azure-sdk-for-net#51210. Reverting back to the 9.3 behavior of not creating the bicep resource at all, which is fine for now since we don't have child table resources yet.
@copilot - can you fix the tests by following the Snapshot Testing with Verify instructions to run |
@copilot - fix the tests by following the "Snapshot Testing with Verify" instructions at https://github.com/dotnet/aspire/blob/main/.github/copilot-instructions.md#snapshot-testing-with-verify. Use |
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
Fixed by updating test snapshot files with |
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.
Pull Request Overview
This PR aims to fix the missing name
property in the generated TableService Bicep by removing the previous provisioning implementation and related TableStorageResource references. It also updates test snapshots to no longer expect a table service resource in Bicep output.
- Removed the
ToProvisioningEntity
method and its import inAzureTableStorageResource
- Dropped the
TableStorageResource
property and provisioning block inAzureStorageExtensions
- Updated snapshots by removing table service resource entries
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/Aspire.Hosting.Azure.Storage/AzureTableStorageResource.cs | Removed ToProvisioningEntity method and unused import |
src/Aspire.Hosting.Azure.Storage/AzureStorageResource.cs | Removed TableStorageResource property |
src/Aspire.Hosting.Azure.Storage/AzureStorageExtensions.cs | Removed provisioning logic for table services and TableStorageResource assignment |
tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureStorageExtensionsTests.*.verified.bicep | Removed expected table service resource entries in snapshots |
Comments suppressed due to low confidence (3)
src/Aspire.Hosting.Azure.Storage/AzureTableStorageResource.cs:42
- Removing the
ToProvisioningEntity
method prevents generating the TableService resource altogether. Restore this method and ensure it sets theName
property to "default" on the provisioning entity.
}
src/Aspire.Hosting.Azure.Storage/AzureStorageExtensions.cs:465
- Removing the assignment of
TableStorageResource
prevents downstream provisioning from detecting the table service. Restore this line so the table resource is registered for provisioning.
var resource = new AzureTableStorageResource(name, builder.Resource);
/backport to release/9.4 |
Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16231014033 |
…otnet#10334)" This reverts commit 059b8ff.
* update azure.provisioning.storage to 1.1.2 * Revert "Fix missing 'name' property in TableService Bicep generation (#10334)" This reverts commit 059b8ff. * Fix tests for latest updates. --------- Co-authored-by: Arcturus Zhang <dapzhang@microsoft.com> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
When using
AddTableService()
on Azure Storage, the generated Bicep was missing the requiredname
property, resulting in invalid Bicep that fails validation.Problem:
Generated invalid Bicep:
Root cause:
The
AzureTableStorageResource.ToProvisioningEntity()
method creates aTableService
instance but doesn't set theName
property. Unlike BlobService and QueueService which correctly generatename: 'default'
, TableService was missing this required property.Solution:
Reverted to previous behavior where we don't add a testServices resource to the bicep since we don't have table children (yet).
To fix this for real we need Azure/azure-sdk-for-net#51210
Fixes #10328.