-
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
Add sql managed instance #9164
Add sql managed instance #9164
Conversation
Azure Pipelines successfully started running 1 pipeline(s). |
azure-sdk-for-go - Release
|
azure-cli-extensions
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-python - Release
|
azure-sdk-for-trenton
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
|
Can one of the admins verify this patch? |
Hi @raych1 , have you had a chance to look at this PR? |
...ion/azuredata/resource-manager/Microsoft.AzureData/preview/2019-07-24-preview/azuredata.json
Show resolved
Hide resolved
...ion/azuredata/resource-manager/Microsoft.AzureData/preview/2019-07-24-preview/azuredata.json
Outdated
Show resolved
Hide resolved
Azure Pipelines successfully started running 1 pipeline(s). |
[Staging] Swagger Validation Report️ ~[Staging] BreakingChange [Detail]Posted by Swagger Pipeline |
No commit pushedDate could be found for PR 9164 in repo Azure/azure-rest-api-specs |
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-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
|
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
|
@raych1 @anthony-c-martin , I have this PR from some time ago that was never merged, I just solved a conflict from recent changes from master and all checks passed. Can you guys take a look at it? Let me know if you need something from me to get it merged. Thank you for your help. |
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.
Please take a look at the comments.
"modelAsString": true | ||
} | ||
}, | ||
"HandshakeRequest": { |
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.
There are no properties in this object?
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.
Correct. There are some objects that still don't have properties. Most objects are just "shadow" objects that map to some objects on the customer's premises, but many functionality is still not defined/final yet.
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 there are changes expected I would recommend you to publish to internal repo as per
https://github.com/Azure/adx-documentation-pr/wiki/The-review-process
"responses": { | ||
"200": { | ||
"body": { | ||
"properties": {}, |
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.
Good to provide example of how this will be used? It's strange that there are no required parameters for this type.
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.
For sure, this is for now just a copy of "hybridDataControllers" type which is being renamed to dataControllers. These objects are currently just used by a CLI tool that creates these "shadow" resources that map to onpremise objects.
We plan on updating the examples and properties on these "shadow" resources as they work on premises progresses.
"responses": { | ||
"200": { | ||
"body": { | ||
"properties": {}, |
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.
Same as above no required parameters? Not sure how clients would be able to use this or build any applications based on such definition?
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.
This is just a copy of sqlInstances which is being renamed to sqlManagedInstances. We are working on what properties will be there and we'll add those once the work progresses. For now this is just a "shadow" resource that maps to some objects on premise.
"responses": { | ||
"200": { | ||
"body": { | ||
"properties": {}, |
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.
Please provide real world 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.
Same here. Will update the examples as the work on the premise side progresses.
.../Microsoft.AzureData/preview/2019-07-24-preview/examples/ListSubscriptionDataController.json
Show resolved
Hide resolved
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.
Satisfied with the responses, approving.
* Adding Microsoft.AzureData/dataControllers (mostly a copy of Microsoft.AzureData/hybridDataManagers) * Fixing issue. * Fixed response body * Use camelCase style * Updated DataControllerResource * Removed invalid format * Adding sqlManagedInstances * Added simple comment. * Adding default responses
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.