Use CustomPatch for EdgeActions Service#39517
Conversation
Next Steps to Merge✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.Comment generated by summarize-checks workflow run. |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews
|
| get is ArmResourceRead<EdgeAction>; | ||
| create is ArmResourceCreateOrUpdateAsync<EdgeAction>; | ||
| update is ArmResourcePatchAsync<EdgeAction, EdgeActionProperties>; | ||
| update is ArmCustomPatchAsync<EdgeAction, EdgeActionUpdate>; |
There was a problem hiding this comment.
Looks like we are updating the input (request object) from EdgeActionProperties to EdgeActionUpdate
@kazrael2119 I think that would impact not just the client but also our api, we would have to update our model classes and the api.
Do we need to change the request object?
same for the other object
There was a problem hiding this comment.
@tundwed This change to ArmCustomPatchAsync will not change the input model and you could compare the main update with this commit change.
Actually this change would not update any REST-level stuff. We just switched the template from ArmResourcePatchAsync to ArmCustomPatchAsync which is our recommandation in guide so that client side could better handle this operation.
The same for the other object.
|
/azp run |
|
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
|
/azp run |
|
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
|
Add some labels consdiering there is no real REST change and these labels are false alarmed. |
specification/cdn/resource-manager/Microsoft.Cdn/EdgeActions/edgeactionsbasemodels.tsp
Show resolved
Hide resolved
| /** The type used for update operations of the EdgeAction. */ | ||
| model EdgeActionUpdate { | ||
| @doc("The resource-specific properties for this resource.") | ||
| properties?: EdgeActionPropertiesUpdate; |
There was a problem hiding this comment.
this is to avoid breaking change in generated swagger
origin swagger: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cdn/resource-manager/Microsoft.Cdn/EdgeActions/preview/2025-12-01-preview/openapi.json#L1605-L1608
specification/cdn/resource-manager/Microsoft.Cdn/EdgeActions/edgeactionsbasemodels.tsp
Show resolved
Hide resolved
| } | ||
|
|
||
| /** Represents an edge action properties */ | ||
| #suppress "@azure-tools/typespec-azure-resource-manager/no-empty-model" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details" |
@weidongxu-microsoft We do not need this type, it was not present before. @kazrael2119 why was this type added? |
The type was in openapi.json before this PR I think it was generated due to the I think we can clean up it a bit in this PR, as long as service is good with it. As change to existing Swagger schema (e.g. remove the |
We currently have no dependency on |
I can't use skuType directly for an Update model as it will fail the lint check with this error message |
I see, looks like ARM enforces this. Since it maintains the current state. I am okay with leaving it as is.
@weidongxu-microsoft let me know if you have a different thought |
tundwed
left a comment
There was a problem hiding this comment.
The changes do not impact api logic.
Good to me, if we keep current state. |
…tion Co-authored-by: tundwed <55289657+tundwed@users.noreply.github.com>
…59847 (#37048) Configurations: 'specification/cdn/resource-manager/Microsoft.Cdn/EdgeActions/tspconfig.yaml', API Version: 2025-12-01-preview, SDK Release Type: beta, and CommitSHA: '3fc43b6c80d8fce282a242b1f34fecced80ddcb5' in SpecRepo: 'https://github.com/Azure/azure-rest-api-specs' Pipeline run: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5759847 Refer to https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites to prepare for SDK release. **Release plan link:** [https://web.powerapps.com/apps/821ab569-ae60-420d-8264-d7b5d5ef734c?release-plan-id=9e03d787-20ec-f011-8544-000d3a5b5a24](https://web.powerapps.com/apps/821ab569-ae60-420d-8264-d7b5d5ef734c?release-plan-id=9e03d787-20ec-f011-8544-000d3a5b5a24) **Submitted by**: dannytundwe@microsoft.com ## Release Plan Details - Release Plan: https://aka.ms/sdk-release-planner?release-plan-id=9e03d787-20ec-f011-8544-000d3a5b5a24 Spec pull request: Azure/azure-rest-api-specs#39517 Spec API version: 2025-12-01-preview --------- Co-authored-by: tundwed <55289657+tundwed@users.noreply.github.com> Co-authored-by: Danny Tundwe (from Dev Box) <dannytundwe@microsoft.com>




This is the change only impacting the client generation no REST API change.
Update the ArmResourcePatchSync to ArmCustomPatchSync and this is supposed to only impact the client generation and have no impact for REST API.
Guidance doc: https://azure.github.io/typespec-azure/docs/howtos/arm/resource-operations/#resource-update-operations-patch
2025-09-01-preview and 2025-12-01-preview have not released SDKs before.