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

Add support for status 200, 201 for LRO delete operation #2255

Open
4 tasks done
AllyW opened this issue Feb 24, 2025 · 1 comment
Open
4 tasks done

Add support for status 200, 201 for LRO delete operation #2255

AllyW opened this issue Feb 24, 2025 · 1 comment
Assignees
Labels
cli/psh Issues for Azure CLI/PSH features

Comments

@AllyW
Copy link
Member

AllyW commented Feb 24, 2025

Clear and concise description of the problem

For now, autorest asks for 202, 204 and default error response for long-running delete operation (according to openapi validator here: https://github.com/Azure/azure-openapi-validator/blob/main/docs/delete-response-codes.md#output-message), conceptually. And tsp compiler also emits only 202, 204 and default error response for LRO delete operation.

However, in execution level, all sdks and other autogen tools need to append an additional operation for 200 or 201 status after getting a successful 202 polling result, which normally is in status 200.

And, there remains some apis whose LRO delete operation contains a 200 response for a successful 202 polling result (like this one: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/monitor/resource-manager/Microsoft.Insights/PrivateLinkScopes/preview/2023-06-01-preview/privateLinkScopes_API.json#L543-L599),
If 200 is dropped for these apis, I believe this might be a breaking change from swagger side?

Also, as a new spec language, it should be better to fill the concept and practice gap on LRO delete operation, and to simplify all the downstream's emitters' code base with no redundance on these field.

Checklist

  • Follow our Code of Conduct
  • Check that this issue is about the Azure libraries for typespec. For feature request in the typespec language or core libraries file it in the TypeSpec repo
  • Read the docs.
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.
@AllyW AllyW added the cli/psh Issues for Azure CLI/PSH features label Feb 24, 2025
@AllyW AllyW pinned this issue Feb 24, 2025
@AllyW AllyW unpinned this issue Feb 24, 2025
@markcowl
Copy link
Member

markcowl commented Feb 24, 2025

The 200 response after the initial 202 is implied by the lro information provided by lro APIs in Azure.Core. In most brownfield APIs that have a 200 response, there is not actually a 200 response returned by delete, but a 200 returned by the subsequent 'get' of the location or Azure-AsyncOperation header. We should not represent operation responses that are not returned by the operation being documented, even though we did this in swagger to make client generation easier. Equivalent information is available through APIs.

The purpose of the templates in the ARM library is not to represent every possible API, but to encourage new APIs to use best practices, which includes only returning 202 from delete operations, as enforced by lintdiff rules. For brownfield operations that are outside current best practices, specifications should use lower level templates. Note that there is an async delete operation template that returns a 200 as shown here, but this should only be used if the delete operation can actually return a 200 itself. If there really are APIs that return a 201 (created) from delete, and this isn't a spec mistake, then these would need to be represented using lower level templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli/psh Issues for Azure CLI/PSH features
Projects
None yet
Development

No branches or pull requests

5 participants