-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Swagger Changes to Add DnsZonePartner and Collation into Managed Instance #3875
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
Conversation
|
Can one of the admins verify this patch? |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-rubyThe 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-goThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-nodeEncountered a Subprocess error: (azure-sdk-for-node)
Command: ['/usr/local/bin/autorest', '/tmp/tmpu78ps76g/rest/specification/sql/resource-manager/readme.md', '--license-header=MICROSOFT_MIT_NO_VERSION', '--node-sdks-folder=/tmp/tmpu78ps76g/sdk', '--nodejs', '[email protected]/[email protected]'] AutoRest code generation utility [version: 2.0.4283; node: v8.11.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
Loading AutoRest core '/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4283)
Loading AutoRest extension '@microsoft.azure/autorest.nodejs' (2.1.103->2.1.103)
Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
Shutting Down
Shutting Down
FATAL: nodejs/generate - FAILED
FATAL: Error: [Exception] AutoRest extension '@microsoft.azure/autorest.nodejs' terminated.
Process() cancelled due to exception : [Exception] AutoRest extension '@microsoft.azure/autorest.nodejs' terminated. |
| "subnetId" : "/subscriptions/20D7082A-0FC7-4468-82BD-542694D5042B/resourceGroups/testrg/providers/Microsoft.Network/virtualNetworks/vnet1/subnets/subnet1", | ||
| "state" : "Ready" | ||
| "state": "Ready", | ||
| "dnsZone": "1b4e2caff2530" |
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.
Is this a realistic-looking dns zone?
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.
Yes it is. We generate dns zone with the first part of a new Guid.
| "storageSizeInGB": 1024, | ||
| "licenseType": "Full" | ||
| "licenseType": "Full", | ||
| "collation": "SQL_Latin1_General_CP1_CI_AS" |
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.
Looks like dnsZone is missing from all these other examples
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.
Will address this now
| "collation": { | ||
| "description": "Collation of the managed instance.", | ||
| "type": "string", | ||
| "x-ms-mutability": [ |
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 thought this was made readonly now
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.
Yes, the Collation property is made read-only a three weeks ago. Mike, which sync point you've used for generating the swagger changes?
jaredmoo
left a comment
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.
Minor nitpicks, otherwise approved
| "readOnly": true | ||
| }, | ||
| "dnsZone": { | ||
| "description": "The Dns Zone taht the managed instance is in.", |
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.
Typo: taht -> that
| "licenseType": "Full" | ||
| "licenseType": "Full", | ||
| "collation": "SQL_Latin1_General_CP1_CI_AS", | ||
| "dnsZone": "1b4e2caff2530" |
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.
nit: please fix indentation
| "licenseType": "Full" | ||
| "licenseType": "Full", | ||
| "collation": "SQL_Latin1_General_CP1_CI_AS", | ||
| "dnsZone": "1b4e2caff2530" |
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.
nit: fix indentation
|
If @jaredmoo is good, I am too ;) |
Automation for azure-sdk-for-jsNothing to generate for azure-sdk-for-js |
|
Thanks! |
This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
PR information
api-versionin the path should match theapi-versionin the spec).Quality of Swagger