update RA calls to new version by darshanhs90 · Pull Request #2031 · Azure/azure-rest-api-specs

darshanhs90 · 2017-11-17T23:53:23Z

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

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

… into current

mcardosos · 2017-11-18T00:47:34Z

If code is regenerated using this tag, code will have breaking changes. This problem can be avoided if a new tag for package-2017-10-preview is added, and the tag for package-2015-07 is left as it originally was.

Refers to: specification/authorization/resource-manager/readme.md:34 in 2996419. [](commit_id = 2996419, deletion_comment = False)

mcardosos · 2017-11-18T00:53:22Z

...ication/authorization/resource-manager/Microsoft.Authorization/2015-07-01/authorization.json

      },
      "description": "Provider operations metadata list"
    },
-    "RoleAssignmentPropertiesWithScope": {


RoleAssignmentPropertiesWithScope [](start = 5, length = 33)

Removing operations from an existing API version will cause breaking changes on generated code.
It is better to create a file with the operations that will be modified in the next API version, so the tag for 2015 version calls this two files (operations that wont change, and operations that will change), and the tag for 2017 version calls the other two files (the one with the unmodified operations and the one with the modified operations).

Since we would need the 2017 tag's[as per above] generated files for sdk-for-net.How would this work if we have two tags?
which generated files will be used by sdk-for net then?

In the Basic Information section you can specify which tag is the default one to generate.

thanks,will update the pr as per the comments then

darshanhs90 · 2017-11-18T01:13:46Z

@mcardosos Also since there wasnt any examples files before,is it mandatory in this pr to include those files for the build to pass?
or can i create a separate pr for this alone #Resolved

mcardosos · 2017-11-18T01:37:01Z

@darshanhs90 I would prefer for the examples to be included here.

darshanhs90 · 2017-11-20T21:40:36Z

@mcardosos Have added the examples for all,but there seems to be some error in the build stating message: 'Equivalent path already exists: /{roleDefinitionId}',even though there is only one path like that.
Could you let me know on what needs to be done to fix the build issue #Resolved

darshanhs90 · 2017-11-21T19:54:45Z

@mcardosos DO let me know if there is anything else that needs to be done for the PR to get merged #Resolved

mcardosos · 2017-11-22T00:22:43Z

...authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json

+      "description": "Role definition properties."
+    },
+    "RoleDefinition": {
+      "properties": {


Are id, name and type properties readonly?

no,made the changes in the specs

What I mean here, because of ARM rules on APIs, this 3 properties shouls be marked as "readonly": true

okay did that

mcardosos · 2017-11-22T00:23:56Z

...ation/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json

+      "description": "Role assignment properties with scope."
+    },
+    "RoleAssignment": {
+      "properties": {


Same here:
Are id, name and type properties readonly?

no,made the changes in the specs

mcardosos · 2017-11-22T00:29:28Z

Added some more comments about readonly properties.
There are model validation errors to be fixed. They can be fixed on the examples files.
This error on x-ms-paths can be safely igonred in this case :)

darshanhs90 · 2017-11-22T01:34:37Z

@mcardosos updated the examples,so that the model validation errors are fixed.Also the readonly field has been modified as per the comments

mcardosos · 2017-11-22T19:25:02Z

@darshanhs90
I am not sure exactly why, but CI didn't run the linter on the 2017-10-01-preview files.
I just ran the linter on your changes so we can actually take a look into this issues in the new API swagger.
If you also want to run the linter on your machine so you also can look at this issues locally, you can do it like this...

cd azure-rest-api-specs\specification\authorization\resource-manager
npx autorest@2.0.4152 --validation --azure-validator --message-format=json

I will add more comments on the new API swaggers based on this errors found by the linter.

{
  "type": "Error",
  "code": "OperationsAPIImplementation",
  "message": "Operations API must be implemented for '/providers/Microsoft.Authorization/operations'.",
  "id": "R3023",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:38: 2 ($.paths)",
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:38: 2 ($.paths)"
  ]
}
{
  "type": "Warning",
  "code": "NonApplicationJsonType",
  "message": "Only 'application/json' content-type is supported by ARM.",
  "id": "R2004",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:14: 4 ($.consumes[1])"
  ]
}
{
  "type": "Warning",
  "code": "NonApplicationJsonType",
  "message": "Only 'application/json' content-type is supported by ARM.",
  "id": "R2004",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:18: 4 ($.produces[1])"
  ]
}
{
  "type": "Warning",
  "code": "OperationIdNounConflictingModelNames",
  "message": "OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'ProviderOperationsMetadataModel'. Consider using the plural form of 'ProviderOperationsMetadata' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.",
  "id": "R2063",
  "validationCategory": "SDKViolation",
  "providerNamespace": "Microsoft.Authorization",
  "resourceType": "providerOperations",
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:191: 8 ($.paths[\"/providers/Microsoft.Authorization/providerOperations/{resourceProviderNamespace}\"].get.operationId)"
  ]
}
{
  "type": "Warning",
  "code": "OperationIdNounConflictingModelNames",
  "message": "OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'ProviderOperationsMetadataModel'. Consider using the plural form of 'ProviderOperationsMetadata' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.",
  "id": "R2063",
  "validationCategory": "SDKViolation",
  "providerNamespace": "Microsoft.Authorization",
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:237: 8 ($.paths[\"/providers/Microsoft.Authorization/providerOperations\"].get.operationId)"
  ]
}
{
  "type": "Error",
  "code": "RequiredPropertiesMissingInResourceModel",
  "message": "Model definition 'RoleDefinition' must have the properties 'name', 'id' and 'type' in its hierarchy and these properties must be marked as readonly.",
  "id": "R2020",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:709: 4 ($.definitions.RoleDefinition)"
  ]
}
{
  "type": "Error",
  "code": "RequiredPropertiesMissingInResourceModel",
  "message": "Model definition 'RoleAssignment' must have the properties 'name', 'id' and 'type' in its hierarchy and these properties must be marked as readonly.",
  "id": "R2020",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:529: 4 ($.definitions.RoleAssignment)"
  ]
}
{
  "type": "Warning",
  "code": "AvoidNestedProperties",
  "message": "Consider using x-ms-client-flatten to provide a better end user experience",
  "id": "R2001",
  "validationCategory": "SDKViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:524: 8 ($.definitions.ClassicAdministrator.properties.properties)"
  ]
}
{
  "type": "Warning",
  "code": "AvoidNestedProperties",
  "message": "Consider using x-ms-client-flatten to provide a better end user experience",
  "id": "R2001",
  "validationCategory": "SDKViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:600: 8 ($.definitions.ProviderOperation.properties.properties)"
  ]
}
{
  "type": "Warning",
  "code": "AvoidNestedProperties",
  "message": "Consider using x-ms-client-flatten to provide a better end user experience",
  "id": "R2001",
  "validationCategory": "SDKViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json:726: 8 ($.definitions.RoleDefinition.properties.properties)"
  ]
}
{
  "type": "Warning",
  "code": "EnumInsteadOfBoolean",
  "message": "Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined. Property: canDelegate",
  "id": "R3018",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:501: 8 ($.definitions.RoleAssignmentFilter.properties.canDelegate)"
  ]
}
{
  "type": "Warning",
  "code": "EnumInsteadOfBoolean",
  "message": "Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined. Property: canDelegate",
  "id": "R3018",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:522: 8 ($.definitions.RoleAssignmentPropertiesWithScope.properties.canDelegate)"
  ]
}
{
  "type": "Warning",
  "code": "AvoidNestedProperties",
  "message": "Consider using x-ms-client-flatten to provide a better end user experience",
  "id": "R2001",
  "validationCategory": "SDKViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:546: 8 ($.definitions.RoleAssignment.properties.properties)"
  ]
}
{
  "type": "Warning",
  "code": "EnumInsteadOfBoolean",
  "message": "Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined. Property: canDelegate",
  "id": "R3018",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:579: 8 ($.definitions.RoleAssignmentProperties.properties.canDelegate)"
  ]
}
{
  "type": "Warning",
  "code": "AvoidNestedProperties",
  "message": "Consider using x-ms-client-flatten to provide a better end user experience",
  "id": "R2001",
  "validationCategory": "SDKViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:588: 8 ($.definitions.RoleAssignmentCreateParameters.properties.properties)"
  ]
}
{
  "type": "Warning",
  "code": "NonApplicationJsonType",
  "message": "Only 'application/json' content-type is supported by ARM.",
  "id": "R2004",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:14: 4 ($.consumes[1])"
  ]
}
{
  "type": "Warning",
  "code": "NonApplicationJsonType",
  "message": "Only 'application/json' content-type is supported by ARM.",
  "id": "R2004",
  "validationCategory": "RPCViolation",
  "providerNamespace": null,
  "resourceType": null,
  "sources": [
    "file:///C:/azure-rest-api-specs/specification/authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json:18: 4 ($.produces[1])"
  ]
}

mcardosos · 2017-11-22T19:28:31Z

...ation/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json

+  ],
+  "consumes": [
+    "application/json",
+    "text/json"


"text/json" [](start = 4, length = 11)

According to the linter, ARM does not support consuming and producing "test/json"

mcardosos · 2017-11-22T19:36:33Z

...ation/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json

+          "readOnly":false,
+          "description": "The role assignment type."
+        },
+        "properties": {


properties [](start = 9, length = 10)

This model could provide a better user experience if this property used x-ms-client-flatten

mcardosos · 2017-11-22T19:39:11Z

...authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json

+          "type": "string",
+          "description": "The type of the administrator."
+        },
+        "properties": {


"properties": [](start = 8, length = 13)

Same comment here, if this property is marked with x-ms-client-flatten user experience on the SDK would improve

Added the x-ms-client-flatten in all the places the linter tool showed as warning

mcardosos · 2017-11-22T19:40:50Z

...authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json

+          "type": "string",
+          "description": "The operation origin."
+        },
+        "properties": {


"properties" [](start = 8, length = 12)

And here too, x-ms-client-flatten can improve generated code.

Added the x-ms-client-flatten in all the places the linter tool showed as warning

mcardosos · 2017-11-22T19:46:58Z

...ation/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json

+    },
+    "RoleAssignmentCreateParameters": {
+      "properties": {
+        "properties": {


properties [](start = 9, length = 10)

This property can also benefit from being flattened

Added the x-ms-client-flatten in all the places the linter tool showed as warning

mcardosos · 2017-11-22T19:50:21Z

...authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json

+  ],
+  "consumes": [
+    "application/json",
+    "text/json"


"text/json" [](start = 4, length = 11)

According to the linter, this is not supported by ARM

mcardosos · 2017-11-22T19:54:27Z

...ation/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization-RACalls.json

+          "type": "string",
+          "description": "Returns role assignment of the specific principal."
+        },
+        "canDelegate":{


canDelegate [](start = 9, length = 11)

Linter is suggesting using an enum instead of a bool here. What do you think?

since the values are either true or false which gets passed on to the service,bool is more apt in this case

mcardosos · 2017-11-22T20:00:05Z

...authorization/resource-manager/Microsoft.Authorization/2017-10-01-preview/authorization.json

+      }
+    },
+    "/providers/Microsoft.Authorization/providerOperations": {
+      "get": {


question for @ravbhatnagar
Is this allowed? The linter marks an error on this swaggers because they are not providing an operatin that lists all available operations in the API. I found this other operation that looks like does exactly that. Is it mandatory to use the exact same path that is provided by the linter error?

No, this is fine for this RP. For all other RPs, the path that linter points is correct.

ravbhatnagar · 2017-11-22T20:20:48Z

Looks good. Signing Off!!

azuresdkciprbot · 2017-11-22T20:36:55Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/authorization/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 4
After the PR: Warning(s): 0 Error(s): 1

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2017-11-22T20:43:31Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/authorization/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 4
After the PR: Warning(s): 0 Error(s): 1

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

darshanhs90 · 2017-11-22T20:44:53Z

@mcardosos Fixed all the above issues
Currently Checking once if there are no regressions on the sdk-side.
Do let me know if everything is fine

AutorestCI · 2017-11-22T21:20:48Z

No modification for AutorestCI/azure-sdk-for-node

AutorestCI · 2017-11-22T21:21:03Z

Azure/azure-sdk-for-python#1644

AutorestCI · 2017-11-22T21:21:15Z

Azure/azure-sdk-for-ruby#1144

darshanhs90 added 4 commits September 25, 2017 16:41

add can delegate flag to ra calls

cf77801

Merge branch 'current' of https://github.com/Azure/azure-rest-api-specs…

6c084f4

… into current

update the version of api

7632bb3

fix merge conflicts

2996419

azuresdkci assigned mcardosos Nov 18, 2017

darshanhs90 mentioned this pull request Nov 18, 2017

Add delegation support for RA calls Azure/azure-sdk-for-net#3872

Merged

9 tasks

add dummy example files-to be updated in next commit

50cfc69

mcardosos reviewed Nov 18, 2017

View reviewed changes

update as per the comments

b851045

darshanhs90 added 4 commits November 20, 2017 12:43

update examples

6c3a206

add new version support for go in readme

eb1a880

fix json parse issue

b5cee27

fix json parse issue

a3fdaa3

darshanhs90 added 2 commits November 21, 2017 11:09

update x-ms-paths

d52d2dd

update x-ms-paths

22db3d9

mcardosos reviewed Nov 22, 2017

View reviewed changes

update examples

d93f686

update as per comments

7deb908

mcardosos reviewed Nov 22, 2017

View reviewed changes

udpated as per the comments

1beb095

ravbhatnagar added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Nov 22, 2017

fix readonly

e351d1d

fix changed exmple files

a051453

mcardosos approved these changes Nov 22, 2017

View reviewed changes

mcardosos merged commit 9e9ae3c into Azure:current Nov 22, 2017

AutorestCI mentioned this pull request Nov 22, 2017

[Authorization] update RA calls to new version Azure/azure-sdk-for-python#1644

Closed

AutorestCI mentioned this pull request Nov 22, 2017

Automatic PR from RestAPI-PR2031 Azure/azure-sdk-for-ruby#1144

Closed

darshanhs90 mentioned this pull request Nov 22, 2017

Fix regression caused by previous merge #2043

Merged

10 tasks

Conversation

darshanhs90 commented Nov 17, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

mcardosos commented Nov 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darshanhs90 commented Nov 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcardosos commented Nov 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darshanhs90 commented Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darshanhs90 commented Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcardosos commented Nov 22, 2017

Uh oh!

darshanhs90 commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcardosos commented Nov 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravbhatnagar commented Nov 22, 2017

Uh oh!

azuresdkciprbot commented Nov 22, 2017

Uh oh!

azuresdkciprbot commented Nov 22, 2017

Uh oh!

darshanhs90 commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

darshanhs90 commented Nov 17, 2017 •

edited by azuresdkci

Loading

darshanhs90 commented Nov 18, 2017 •

edited

Loading

mcardosos commented Nov 18, 2017 •

edited

Loading

darshanhs90 commented Nov 20, 2017 •

edited

Loading

darshanhs90 commented Nov 21, 2017 •

edited

Loading

darshanhs90 commented Nov 22, 2017 •

edited

Loading

darshanhs90 commented Nov 22, 2017 •

edited

Loading