-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[AKS]: Remove 200/OK response from reset SP/AAD api #4948
Conversation
If you're a MSFT employee, click this link |
Automation for azure-sdk-for-jsThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-rubyNothing to generate for azure-sdk-for-ruby |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-javaNothing to generate for azure-sdk-for-java |
Automation for azure-sdk-for-nodeThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-goThe initial PR has been merged into your service PR: |
Can one of the admins verify this patch? |
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.
Need confirmation from @ravbhatnagar that returning 202 from a synchronous operation is ok.
.../resource-manager/Microsoft.ContainerService/preview/2018-08-01-preview/managedClusters.json
Outdated
Show resolved
Hide resolved
.../resource-manager/Microsoft.ContainerService/preview/2018-08-01-preview/managedClusters.json
Outdated
Show resolved
Hide resolved
Hello!! The Rest API Specs team wishes everyone a happy holiday and would like to kindly inform you that responses and review to Pull request will be delayed during the holiday period (now -> 2nds of January) to allow for Awesome reviewers to have an awesome time with their families during the holidays! Thanks and Have a WONDERFUL HOLIDAY |
@zqingqing1 can you please explain why this is being removed? It's introducing new SDK validation failures, see here. Current requirement is to model the terminal LRO state in the swagger. |
The reason why it is been removed, because 200 is not in the response status of these 2 APIs. So right now, I am thinking maybe we can return operation status as 200 after polling header, if we wanna resolve this error. |
@lmazuel I was told you asked to have this validation rule put in place. Can you please explain the details behind it? |
@jhendrixMSFT, I added 200 response to resolve the error. Please review :) |
@zqingqing1 there is a merge conflict, can you please resolve it? |
@jhendrixMSFT have time to review again? |
@AutorestCI regenerate azure-sdk-for-go |
With these changes the generated code no longer returns a |
Yes, it is expected. |
So when the polling completes the user needs to make a separate API call then to retrieve the result? |
Yes, that is right. |
* add operation for 2018-03-31 and 2018-08-01-preview * correct example fileName * add default response and 202, correct typo * remove 200 response * long running operation * return 202 with no body * forget to remove 202 body from 2018-08-01-preview api * add 200 without schema
Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.