#ApplicationGateway Ssl Policy, Get AvailableSslOption, Redirect, Azure Websites and backendhealth nic fix#1285
Conversation
…ion, Redirect, Azure Websites and backendhealth nic fix
|
@akshaysngupta, |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
|
Closing and reopening on behalf of Ben Wanner. Thanks |
|
@akshaysngupta, |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
|
@akshaysngupta, |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
| "parameters": { | ||
| "routeFilterName": "filterName", | ||
| "resourceGroupName": "rg1", | ||
| "api-version": "2017-03-01", |
There was a problem hiding this comment.
You should update version in the example to 2017-06-01
| "tags": [ | ||
| "ApplicationGateways" | ||
| ], | ||
| "operationId": "ApplicationGateways_ListAvailableSslPredefinedPolicies", |
| "description": "Client API version." | ||
| }, | ||
| "PredefinedPolicyNameParameter": { | ||
| "name": "predefined-poicy-name", |
There was a problem hiding this comment.
This does not match the name defined in the path
/subscriptions/{subscriptionId}/providers/Microsoft.Network/applicationGatewayAvailableSslOptions/default/predefinedPolicies/{predefinedPolicyName}
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/241395216#L1165
| } | ||
| }, | ||
| "CipherSuitesEnum":{ | ||
| "type": "string", |
There was a problem hiding this comment.
missing description.
Please add for better documentation. https://travis-ci.org/Azure/azure-rest-api-specs/jobs/241395215#L1579
| } | ||
| } | ||
| }, | ||
| "/subscriptions/{subscriptionId}/providers/Microsoft.Network/applicationGatewayAvailableSslOptions/default": { |
There was a problem hiding this comment.
We'd encourage you to please provide x-ms-examples for the newly introduced operations to help us with model validation.
|
Please also fix the merge conflict for |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
vishrutshah
left a comment
vishrutshah
left a comment
There was a problem hiding this comment.
Please make sure that swagger is semantically correct. https://travis-ci.org/Azure/azure-rest-api-specs/jobs/243447018#L1168
| "type": "string", | ||
| "description": "Host header to be sent to the backend servers." | ||
| }, | ||
| "pickHostNameFromBackendAddress": { |
There was a problem hiding this comment.
Consider renaming property to hostNameSource and the values could be BackendAddress or Default. Default would be default value. Bools are generally not preferable.
| "type": "string", | ||
| "description": "Cookie name to use for the affinity cookie." | ||
| }, | ||
| "probeEnabled": { |
There was a problem hiding this comment.
Consider making this a string enum.
| ], | ||
| "description": "Backend Address Pool of an application gateway." | ||
| }, | ||
| "ApplicationGatewayBackendHttpSettingsPropertiesFormat": { |
There was a problem hiding this comment.
ConnectionDraining property is missing in this version. What happens when a resource which was created using this property (old api version), is GET using the new API-version which does not have this property? What will the service return?
There was a problem hiding this comment.
Connection draining is still there. I just moved it down.
| "format": "int32", | ||
| "description": "The probe retry count. Backend server is marked down after consecutive probe failure count reaches UnhealthyThreshold. Acceptable values are from 1 second to 20." | ||
| }, | ||
| "pickHostNameFromBackendHttpSettings":{ |
There was a problem hiding this comment.
consider using string enum
| }, | ||
| "description": "Properties of request routing rule of the application gateway." | ||
| }, | ||
| "ApplicationGatewayRequestRoutingRule": { |
There was a problem hiding this comment.
"type" property is missing. Proxy or non-tracked resources should have the standard name, id and type properties. I th
There was a problem hiding this comment.
These are child resources for Application Gateway.
| }, | ||
| "description": "Properties of redirect configuration of the application gateway." | ||
| }, | ||
| "ApplicationGatewayRedirectConfiguration": { |
There was a problem hiding this comment.
type property missing. This is a child resource of ApplicationGateway. Even proxy resources should have the standard name, id and type properties.
| "info": { | ||
| "title": "NetworkManagementClient", | ||
| "description": "The Microsoft Azure Network management API provides a RESTful set of web services that interact with Microsoft Azure Networks service to manage your network resources. The API has entities that capture the relationship between an end user and the Microsoft Azure Networks service.", | ||
| "version": "2017-06-01" |
There was a problem hiding this comment.
Are you only changing the api-version of this file? For all other files, its still 2017-03-01 even though they are moved to the 2017-06-01 folder? It would be difficult to review if you add the all the files to a new folder even though nothing has changed.
There was a problem hiding this comment.
version is updated by the team maintaining the resource.
|
ARM signs off on the new changes. |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
vishrutshah
left a comment
vishrutshah
left a comment
There was a problem hiding this comment.
@ravbhatnagar could you please re-review and make sure things looks good from the ARM perspective. Thanks
| "description": "The prefixes that the bgp community contains." | ||
| }, | ||
| "isAuthorizedToUse": { | ||
| "isAuthorizedToUse": { |
There was a problem hiding this comment.
Are you sure this is a boolean. Because the example depicts it as a string "True" instead of boolean true. https://travis-ci.org/Azure/azure-rest-api-specs/jobs/243501989#L1174
There was a problem hiding this comment.
These changes came in from master.
There was a problem hiding this comment.
@akshaysngupta If the spec is correct, you may need to update the example, if the example is correct, then it's mismatching the spec.
There was a problem hiding this comment.
This was already in master. I just fixed the indentation.
There was a problem hiding this comment.
@akshaysngupta I see your point but as @veronicagg mentioned, if you know which one is correct example / service or if you know who is the owner of the service, let's make sure that we understand which one is correct and let's fix that while we have an opportunity here :). Thanks!
There was a problem hiding this comment.
corrected the example. :)
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: File: File: File: File: File: File: File: File: File: File: File: File: File: Thanks for your co-operation. |
|
Thanks a lot @akshaysngupta for the fix. Changes LGTM! I'd still wait until @ravbhatnagar approves this PR :). |
|
LGTM |
|
No modification for AutorestCI/azure-sdk-for-node |
|
No modification for AutorestCI/azure-sdk-for-python |
|
@vishrutshah @akshaysngupta Was |
12 participants
Summary of Changes
Information for Validation Issues
We discussed the validation issues for AGW in a previous PR (#1052 (comment)). We are aware of the style issues and will address them in the near future.
PR information
api-versionin the path should match theapi-versionin the spec).Quality of Swagger