-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[Bug fix] Remove read-only constraint for management policy, and change properties types from integer to number #6551
Conversation
SDK Automation [Logs] (Generated from 0e6f149, Iteration 6)
|
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-javaThe 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? |
…onGreaterThan to float type
specification/storage/resource-manager/Microsoft.Storage/stable/2019-04-01/storage.json
Outdated
Show resolved
Hide resolved
"type": "integer", | ||
"minimum": 0, | ||
"type": "float", | ||
"minimum": 0.0, | ||
"description": "Integer value indicating the age in days after last modification" |
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.
Description is still calling it an integer
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.
In the server, the property is defined as float type for testing purpose. But in the real use case, we expect the input to be an integer-like number. I added a constraint in the update 0e6f149, which will only allow number that is the multiple of 1.
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.
Yeah, but since it's no longer an actual int, the docs shouldn't call it that. Maybe just change "Integer value" to "Value"
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.
Changed "Integer value" to "Value" 870609e
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.
Approved from ARMs side
@myronfanqiu Please have a look at Ruby SDK. |
@zfchen95 Please have a look at checklist and try to fix the error in public.rest-api-specs. |
It's a known issue in the SDK Automation. It doesn't block the real SDK generation process. Currently, we are working on it. |
Hi @Juliehzl, I looked at the errors in public.rest-api-specs (model validation) test. None of them are caused by the change in this PR. Actually, all the errors are caused by "missing" schema in the storage swagger. I will try to fix them in another PR, not in this one because it will require an additional review..
|
I have fixed the errors in #6611 |
Yes. Please make changes in this PR. |
Updated. All the checks pass. |
@Juliehzl Could you approve it? |
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.