Azure Machine Learning WebServicesRP new API version#1019
Conversation
sergey-shandar
left a comment
There was a problem hiding this comment.
I'm not sure why the PR doesn't have a standard check list as other PRs do, for example #1045 (comment), but we still need to go through the list.
According to https://github.com/Azure/azure-rest-api-specs/blob/master/.github/CONTRIBUTING.md#filenames-and-folder-structure we should have a commit with the old version of the swagger file so we can see the changes. As a workaround, I've created a dummy branch to see the changes https://github.com/sergey-shandar/azure-rest-api-specs/pull/1/files.
| ], | ||
| "consumes": [ | ||
| "application/json", | ||
| "text/json" |
There was a problem hiding this comment.
AutoRest doesn't support "text/json".
From https://travis-ci.org/Azure/azure-rest-api-specs/jobs/209869904#L974
| ], | ||
| "produces": [ | ||
| "application/json", | ||
| "text/json" |
There was a problem hiding this comment.
AutoRest doesn't support "text/json".
| "tags": [ | ||
| "WebServices" | ||
| ], | ||
| "operationId": "WebServices_CreateOrUpdate", |
There was a problem hiding this comment.
Could you provide examples as described here https://github.com/Azure/azure-rest-api-specs-pr/blob/master/documentation/swagger-checklist.md#validation-tools-for-swagger-checklist ?
| "application/json", | ||
| "text/json" | ||
| ], | ||
| "paths": { |
There was a problem hiding this comment.
This service should also implement Operation API. It's an operation which returns a list of all service operations. See the example
https://github.com/Azure/azure-rest-api-specs/blob/master/arm-cdn/2016-10-02/swagger/cdn.json#L1578
From https://travis-ci.org/Azure/azure-rest-api-specs/jobs/209869904#L996
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "Success. This response is returned for an update web service operation. The response payload is identical to the response payload that is returned by the GET operation. The response includes the Provisioning State and the Azure-AsyncOperation header. To get the progress of the operation, call GET operation on the URL in Azure-AsyncOperation header field. For more information about Asynchronous Operations, see https://msdn.microsoft.com/en-us/library/mt742920.aspx.", |
There was a problem hiding this comment.
Avoid MSDN references. For better generated code quality, remove all references to "msdn.microsoft.com".
From https://travis-ci.org/Azure/azure-rest-api-specs/jobs/209869904#L1007
| "description": "Specifies the name of the resource.", | ||
| "type": "string" | ||
| }, | ||
| "location": { |
There was a problem hiding this comment.
The name property of the Resource must be read-only.
| "type": "string", | ||
| "readOnly": true | ||
| }, | ||
| "tags": { |
There was a problem hiding this comment.
The tags property of the Resource must be read-only.
| "x-ms-pageable": { | ||
| "nextLinkName": "nextLink" | ||
| }, | ||
| "operationId": "WebServices_List", |
There was a problem hiding this comment.
A Tracked Resource must have a ListBySubscriptionId operation with x-ms-pageable extension. I assume, the operationId should be renamed to "WebServices_ListBySubscriptionId".
From https://travis-ci.org/Azure/azure-rest-api-specs/jobs/209869904#L1128
| "type": "boolean", | ||
| "description": "When set to true, indicates that the payload size is larger than 3 MB. Otherwise false. If the payload size exceed 3 MB, the payload is stored in a blob and the PayloadsLocation parameter contains the URI of the blob. Otherwise, this will be set to false and Assets, Input, Output, Package, Parameters, ExampleRequest are inline. The Payload sizes is determined by adding the size of the Assets, Input, Output, Package, Parameters, and the ExampleRequest." | ||
| }, | ||
| "PayloadsLocation": { |
There was a problem hiding this comment.
the name should follow camel case style.
From https://travis-ci.org/Azure/azure-rest-api-specs/jobs/209869904#L1139
| } | ||
| }, | ||
| "definitions": { | ||
| "Resource": { |
There was a problem hiding this comment.
The definition should have "description".
From https://travis-ci.org/Azure/azure-rest-api-specs/jobs/209869904#L1172
|
Spec used to generate Azure/azure-sdk-for-net#2921 |
|
Can one of the admins verify this patch? |
|
No modification for Ruby |
|
No modification for NodeJS |
|
No modification for Python |
Azure Machine Learning is going to publish a new version 2017-01-01. This is the swagger for the new version. Most of the part is similar to the previous version in https://github.com/Azure/azure-rest-api-specs/blob/master/arm-machinelearning/2016-05-01-preview/swagger/webservices.json. I would recommend reviewers comparing with that file.