[Azure Automation] Swagger json files for rest of 15 resources and and changes to #1045
Conversation
Starting with these 4 resources. json files are split by each resource. rest of resources to come subsequently.
…d changes to account.json
| "$ref": "#/definitions/AutomationAccount" | ||
| } | ||
| }, | ||
| "default": { |
There was a problem hiding this comment.
all indentations are corrected now.
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "OK", |
There was a problem hiding this comment.
please provide descriptive descriptions
There was a problem hiding this comment.
Hi Vishrut,
We have received exceptions for the below issues:
[Azure Automation] validations (size, range, regexes) on properties and parameters missing #1020
#1020
[Azure Automation] Descriptions need to be corrected in all Azure Automation resources #1021
#1021
Samer is aware of above issues. The two issues above will be revisited. These descriptions were inherited from Hyak specs and we won't be changing descriptions or validation regex at the moment
Leave a comment
| "in": "body", | ||
| "required": true, | ||
| "schema": { | ||
| "$ref": "#/definitions/AutomationAccountCreateOrUpdateParameters" |
| } | ||
| } | ||
| }, | ||
| "put": { |
There was a problem hiding this comment.
Is this a long-running operation?
| "$ref": "#/definitions/AutomationAccount" | ||
| } | ||
| }, | ||
| "default": { |
| }, | ||
| "Usage": { | ||
| "properties": { | ||
| "id": { |
| }, | ||
| "unit": { | ||
| "type": "string", | ||
| "description": "Gets or sets the usage unit name." |
There was a problem hiding this comment.
please provide descriptive description so that users can understand this property. Applies for entire document.
There was a problem hiding this comment.
Leave a commentHi Vishrut,
We have received exceptions for the below issues:
[Azure Automation] validations (size, range, regexes) on properties and parameters missing #1020
#1020
[Azure Automation] Descriptions need to be corrected in all Azure Automation resources #1021
#1021
Samer is aware of above issues. The two issues above will be revisited. These descriptions were inherited from Hyak specs and we won't be changing descriptions or validation regex at the moment
| "description": "Gets or sets the current usage value." | ||
| }, | ||
| "limit": { | ||
| "type": "integer", |
There was a problem hiding this comment.
you should use Minimum and Maximum constrains here
There was a problem hiding this comment.
Leave a commentHi Vishrut,
We have received exceptions for the below issues:
[Azure Automation] validations (size, range, regexes) on properties and parameters missing #1020
#1020
[Azure Automation] Descriptions need to be corrected in all Azure Automation resources #1021
#1021
Samer is aware of above issues. The two issues above will be revisited. These descriptions were inherited from Hyak specs and we won't be changing descriptions or validation regex at the moment
| "in": "path", | ||
| "required": true, | ||
| "type": "string", | ||
| "description": "Gets subscription credentials which uniquely identify Microsoft Azure subscription. The subscription ID forms part of the URI for every service call." |
There was a problem hiding this comment.
Hi Vishrut,
We have received exceptions for the below issues:
[Azure Automation] validations (size, range, regexes) on properties and parameters missing #1020
#1020
[Azure Automation] Descriptions need to be corrected in all Azure Automation resources #1021
#1021
Samer is aware of above issues. The two issues above will be revisited. These descriptions were inherited from Hyak specs and we won't be changing descriptions or validation regex at the moment
There was a problem hiding this comment.
Yes @vishrutshah they have exceptions for fixing descriptions
| "in": "path", | ||
| "required": true, | ||
| "type": "string", | ||
| "pattern": "^[-\\w\\._]+$", |
|
I'd request you please provide x-ms-example for your APIs. https://github.com/Azure/azure-rest-api-specs/blob/master/documentation/x-ms-examples.md |
|
Hi Vishrut, Samer is aware of above issues. The two issues above will be revisited. These descriptions were inherited from Hyak specs and we won't be changing descriptions or validation regex at the moment |
|
Issue 2: The issue is AutoRest command that generates “Generated” files adds member Credentials (Credentials needed for the client to connect to Azure.) Co-Incidentally, we also have an API “Credentials” due to which two members have name conflicts. Could you let us know how to resolve this issue? I’ve attached the concerned files for your reference. (Forwarded the .cs files in reference to Vishrut). There is a name conflict. Two "Credentials" can be found |
|
#Issue 2 is coming from https://github.com/Azure/azure-rest-api-specs/pull/1045/files#diff-bf0e1b6e54180951dd6138234a55f3c4R239 : |
|
Issue 1: Yes AutoRest should generate guid for you. Ref: Azure/autorest#784 |
|
@vivlingaiah just to be clear xms-examples are not covered in the exceptions, just wanted to make sure that we're on the same page here |
vishrutshah
left a comment
There was a problem hiding this comment.
Reviewed arm-automation/2015-10-31/swagger/certificate.json. Please fix indentation across the swagger files
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "OK" |
There was a problem hiding this comment.
The PUT/GET/PATCH operations must have same schema response - I see patch is not returning Certificate.
| "tags": [ | ||
| "Certificates" | ||
| ], | ||
| "operationId": "Certificates_List", |
There was a problem hiding this comment.
_ListByAutomationAccounts?
| "properties": { | ||
| "base64Value": { | ||
| "type": "string", | ||
| "description": "Gets or sets the base64 encoded value of the certificate." |
There was a problem hiding this comment.
Gets or Sets MUST not appear in entire swagger.
| "type": "string", | ||
| "description": "Gets or sets the thumbprint of the certificate." | ||
| }, | ||
| "isExportable": { |
| "type": "string", | ||
| "description": "Gets or sets the description of the certificate." | ||
| }, | ||
| "thumbprint": { |
| "type": "string", | ||
| "description": "Gets or sets the thumbprint of the certificate." | ||
| }, | ||
| "expiryTime": { |
| "type": "boolean", | ||
| "description": "Gets or sets the is exportable flag of the certificate." | ||
| }, | ||
| "creationTime": { |
| "format": "date-time", | ||
| "description": "Gets or sets the creation time." | ||
| }, | ||
| "lastModifiedTime": { |
| }, | ||
| "lastModifiedTime": { | ||
| "type": "string", | ||
| "format": "date-time", |
There was a problem hiding this comment.
What is the date-time format? ISO8601?
| }, | ||
| "description": "Properties of the certificate." | ||
| }, | ||
| "Certificate": { |
There was a problem hiding this comment.
Is this a tracked resource?
vishrutshah
left a comment
There was a problem hiding this comment.
Reviewed arm-automation/2015-10-31/swagger/connection.json. Please update indentation and provide accurate descriptions.
Also please provide x-ms-example for each of the operation
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "OK" |
There was a problem hiding this comment.
Patch MUST return #/definitions/Connection schema
| "tags": [ | ||
| "Connections" | ||
| ], | ||
| "operationId": "Connections_List", |
There was a problem hiding this comment.
_ListByAutomationAccounts
| } | ||
| } | ||
| }, | ||
| "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Automation/automationAccounts/{automationAccount}/connections": { |
There was a problem hiding this comment.
Don't you have
List - BySubscription &
ListByResourceGroup for this tracked resource?
There was a problem hiding this comment.
this is not a tracked resource. corrected naming of methods
| }, | ||
| "connectionType": { | ||
| "$ref": "#/definitions/ConnectionTypeAssociationProperty", | ||
| "description": "Gets or sets the connectionType of the connection." |
There was a problem hiding this comment.
Gets or Sets MUST not appear in the swagger. Documentation will not be good.
| "description": "Gets or sets the field definition values of the connection." | ||
| }, | ||
| "creationTime": { | ||
| "type": "string", |
| "format": "date-time", | ||
| "description": "Gets or sets the creation time." | ||
| }, | ||
| "lastModifiedTime": { |
| }, | ||
| "description": "Definition of the connection properties." | ||
| }, | ||
| "Connection": { |
There was a problem hiding this comment.
Is this a tracked/Proxy Resource? Should it inherit from the common Resource model having id, name, type as readOnly?
There was a problem hiding this comment.
this is not a tracked resource
| "Connection": { | ||
| "properties": { | ||
| "id": { | ||
| "type": "string", |
| } | ||
| } | ||
| }, | ||
| "/{subscriptionId}/operations/{requestId}": { |
There was a problem hiding this comment.
Is this the API supported & maintained by your team?
There was a problem hiding this comment.
it's not supported. I removed it
| } | ||
| ], | ||
| "responses": { | ||
| "201": { |
There was a problem hiding this comment.
when do you return 201 v/s 200? Seems like both return the same model?
|
The following operation names are violating the rules:
@vivlingaiah Can you confirm if you have exceptions approved for all of these? |
|
A body parameter must be named 'parameters'. The following paths are violating the rule:
|
|
One of the rule is that "Parameters "subscriptionId" and "api-version" are not allowed in the operations section, define these in the global parameters section instead". There are several paths which violate the rule: https://gist.github.com/sarangan12/e5ffe03d76cbaccbcceba2060c2dec8d#file-gistfile1-txt |
|
ERROR: A 'POST' operation 'RunbookDraft_Publish' with x-ms-long-running-operation extension must have a valid terminal success status code 200 or 201 or 204. |
|
ERROR: The property 'odata.nextLink' specified by nextLinkName does not exist in the 200 response schema. |
|
ERROR: Tracked Resource failing validation is: "DscConfiguration". Validation Failed: 2. |
0552348 to
e4dc90d
Compare
|
Hi Sarangan,
|
|
No modification for Ruby |
1 similar comment
|
No modification for Ruby |
|
No modification for Python |
|
No modification for NodeJS |
Re-Creating PR for @vivlingaiah
@vivlingaiah -- You need to complete the following checklist, and fix the conflicts between the master branch and this PR (Pull from master into your local branch, fix the changes and push back into your fork on
AzureAutomationTeam:masterThis 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