-
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
Microsoft.AzureStackHCI new api version 2020-10-01 #10953
Microsoft.AzureStackHCI new api version 2020-10-01 #10953
Conversation
…020-03-01-preview to version 2020-10-01
[Staging] Swagger Validation Report
️✔️ |
Azure Pipelines successfully started running 1 pipeline(s). |
Azure CLI Extension Generation - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-net - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-go - Release
|
azure-sdk-for-python - Release
- Breaking Change detected in SDK
|
azure-sdk-for-python-track2 - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-resource-manager-schemas - Release
|
Trenton Generation - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-js - Release
|
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.
add "WaitForARMFeedback" since adding new API version
"properties": { | ||
"description": "Cluster properties.", | ||
"$ref": "#/definitions/ClusterProperties", | ||
"x-ms-client-flatten": true |
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.
just curious, why need flatten?
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.
I'm not sure. I just followed the validation tool suggestion. Is there any guidance for this option?
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.
Yup, it's an SDK generation recommendation. It prevents autorest from generating the properties
property in the SDK models, which makes it easier to use a little bit.
In reply to: 496112091 [](ancestors = 496112091)
"ConnectedRecently", | ||
"NotConnectedRecently", | ||
"Disconnected", | ||
"Error" |
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.
Unknown?
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.
I'm not sure what you mean. Can you elaborate? Statuses follow the cluster lifecycle, we don't have "unknown".
}, | ||
"readOnly": true | ||
}, | ||
"cloudId": { |
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.
if it's resource id, why not property name as "resourceId"?
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.
It was in the original design but ARM reviewer said that it would confuse users with the actual resource Id.
"description": "Properties reported by cluster agent.", | ||
"$ref": "#/definitions/ClusterReportedProperties" | ||
}, | ||
"trialDaysRemaining": { |
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.
does this property has constraint? If yes, could you pls add it in Swagger?
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.
I could add "minimum: 0"? I don't want to define upper bound.
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.
Signed off from ARM side. Thank you from making the first commit in the new folder the original API version!
hi @mbialecka is the change on prod already? |
@yungezz No, the new api version endpoints have not been setup in prod yet. |
@yungezz Can I have this merged? |
"tags": [ | ||
"Clusters" | ||
], | ||
"operationId": "Clusters_List", |
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.
Could you update operation id to Clusters_ListBySubscription
? Thanks!
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.
Isn't this a breaking change to a stable swagger?
Azure Pipelines successfully started running 1 pipeline(s). |
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If there are following updates in the PR, ensure to request an approval from API Review Board as defined in the Breaking Change Policy.
Please follow the link to find more details on PR review process.