update Role based calls to new api version to support dataActions by darshanhs90 · Pull Request #2356 · Azure/azure-rest-api-specs

darshanhs90 · 2018-01-29T21:58:07Z

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.

AutorestCI · 2018-01-29T22:36:25Z

Did a commit to Azure/azure-sdk-for-python:
Azure/azure-sdk-for-python@04e8425

AutorestCI · 2018-01-29T22:50:06Z

Did a commit to Azure/azure-sdk-for-go:
Azure/azure-sdk-for-go@da0d03b

darshanhs90 · 2018-01-29T23:19:50Z

@mcardosos what can be done to remove the semantic issue for this PR,as the path mentioned in the error isn't used anywhere before
Reason for /roleassignmentId and /roledefinitionid to be different is as per the example for each
/roleassignmentid example:
id=/subscriptions//providers/Microsoft.Authorization/roleAssignments/fa1a4d3b-2cca-406b-8956-6b6b32377641

/roledefinitionid example: id=/subscriptions//providers/Microsoft.Authorization/roleDefinitions/f0885aa8-0107-4e65-9a00-541286195838

mcardosos · 2018-01-31T00:00:56Z

...ager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-ClassicAdminCalls.json

+            "required": true,
+            "type": "string",
+            "description": "The API version to use for this operation."
+          },


Why not use the API version parameter already defined in the parameters section? #Resolved

mcardosos · 2018-01-31T00:04:20Z

...manager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-RoleBasedCalls.json

+            "description": "The namespace of the resource provider."
+          },
+          {
+            "name": "api-version",


api-version [](start = 21, length = 11)

Same here. If the parameter is already defined somewhere else why not use it?

mcardosos · 2018-01-31T00:05:54Z

...manager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-RoleBasedCalls.json

+        "tags": [
+          "ProviderOperationsMetadata"
+        ],
+        "operationId": "ProviderOperationsMetadata_Get",


ProviderOperationsMetadata_Get [](start = 24, length = 30)

Odata extension can be useful here

mcardosos · 2018-01-31T00:10:53Z

...manager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-RoleBasedCalls.json

+        "tags": [
+          "ProviderOperationsMetadata"
+        ],
+        "operationId": "ProviderOperationsMetadata_List",


ProviderOperationsMetadata_List [](start = 24, length = 31)

Odata extension is useful here too

mcardosos · 2018-01-31T00:13:37Z

...manager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-RoleBasedCalls.json

+        "description": "Gets all permissions the caller has for a resource group.",
+        "parameters": [
+          {
+            "name": "resourceGroupName",


resourceGroupName [](start = 21, length = 17)

If resource group name parameter is used in many operation, it also can be defined in the parameter section

mcardosos · 2018-01-31T00:28:39Z

Hello @darshanhs90 !
The main issue is that swagger does not allow to define the same path more than once (for swagger /{roleassignmentid} and /{roledefinitionid} are equivalents.
x-ms-paths can be used to avoid that.

mcardosos · 2018-01-31T00:34:03Z

On the other hand, we are moving away from current branch.

mcardosos · 2018-01-31T01:03:32Z

...manager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-RoleBasedCalls.json

+        "x-ms-pageable": {
+          "nextLinkName": "nextLink"
+        },
+        "x-ms-odata": "#/definitions/RoleAssignmentFilter",


RoleAssignmentFilter [](start = 37, length = 20)

Not just RoleAssignment ?

no it takes in the filter parameters as query parameter in the url,hence not the roleassignment

In reply to: 164927815 [](ancestors = 164927815)

darshanhs90 · 2018-01-31T01:28:55Z

@mcardosos when i added the x-ms-path,i got an error regarding that also
{
"type": "Error",
"code": "XmsPathsMustOverloadPaths",
"message": "Paths in x-ms-paths must overload a normal path in the paths section, i.e. a path in the x-ms-paths must either be same as a path in the paths section or a path in the paths sections followed by additional parameters.",

Not sure what it expects in x-ms-path

AutorestCI · 2018-01-31T01:53:09Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

AutorestCI · 2018-01-31T02:03:13Z

Did a commit to Azure/azure-sdk-for-python:
Azure/azure-sdk-for-python@eb3fbd5

mcardosos · 2018-01-31T02:22:41Z

@darshanhs90 It expects that paths in the x-ms-path section also have a query (a dumb query ), just for disambiguation/

AutorestCI · 2018-01-31T17:41:01Z

This commit was treated and no generation was made for Azure/azure-sdk-for-python

AutorestCI · 2018-01-31T17:41:53Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

darshanhs90 · 2018-01-31T20:00:46Z

@mcardosos does autorest expect me to have both /roleassignment and /roledefinition in paths and add another /roledefintion?dummy in x-ms paths?

mcardosos · 2018-01-31T21:41:03Z

No, just /roleassignment in paths, and /roledefinition with dummy in x-ms-paths

darshanhs90 · 2018-01-31T21:51:06Z

@mcardosos included as mentioned by you,but still the linter is showing an error in this PR

AutorestCI · 2018-02-01T01:09:49Z

Did a commit to Azure/azure-sdk-for-python:
Azure/azure-sdk-for-python@abba98d

AutorestCI · 2018-02-01T01:22:49Z

Did a commit to Azure/azure-sdk-for-go:
Azure/azure-sdk-for-go@daaa1ec

AutorestCI · 2018-02-01T01:33:25Z

Did a commit to Azure/azure-sdk-for-python:
Azure/azure-sdk-for-python@1647759

AutorestCI · 2018-02-01T01:35:32Z

Did a commit to Azure/azure-sdk-for-go:
Azure/azure-sdk-for-go@5972911

AutorestCI · 2018-02-01T19:08:05Z

Swagger to SDK encountered a Subprocess error: (Azure/azure-sdk-for-go)

Command: ['/usr/local/bin/autorest', '/tmp/tmp0j9huvcm/rest/specification/authorization/resource-manager/readme.md', '--perform-load=false', '--swagger-to-sdk', '--output-artifact=configuration.json', '--input-file=foo', '--output-folder=/tmp/tmpb3is22rb']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4244; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest

There is a new version of AutoRest available (2.0.4245).
 > You can install the newer version with with npm install -g autorest@latest

   Loading AutoRest core      '/tmp/.autorest/@microsoft.azure_autorest-core@2.0.4244/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4244)
ERROR: Syntax Error Encountered: Syntax error: can not read a block mapping entry; a multiline key may not be an implicit key
    - file:///tmp/tmp0j9huvcm/rest/specification/authorization/resource-manager/readme.md:181:4
[OperationAbortedException] Error occurred. Exiting.

AutorestCI · 2018-02-01T20:08:14Z

Swagger to SDK encountered a Subprocess error: (Azure/azure-sdk-for-python)

Command: ['/usr/local/bin/autorest', '/tmp/tmphnqjrlbt/rest/specification/authorization/resource-manager/readme.md', '--perform-load=false', '--swagger-to-sdk', '--output-artifact=configuration.json', '--input-file=foo', '--output-folder=/tmp/tmp20281fs2']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4245; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/tmp/.autorest/@microsoft.azure_autorest-core@2.0.4245/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4245)
ERROR: Syntax Error Encountered: Syntax error: can not read a block mapping entry; a multiline key may not be an implicit key
    - file:///tmp/tmphnqjrlbt/rest/specification/authorization/resource-manager/readme.md:181:4
[OperationAbortedException] Error occurred. Exiting.

AutorestCI · 2018-02-01T20:14:56Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

azuresdkciprbot · 2018-02-01T20:15:37Z

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:

💡 Please review potentially introduced Error(s)/Warning(s): Analysis Report 💡

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

AutorestCI · 2018-02-01T20:19:59Z

This commit was treated and no generation was made for Azure/azure-sdk-for-python

mcardosos · 2018-02-02T00:09:51Z

...manager/Microsoft.Authorization/preview/2018-01-01-preview/authorization-RoleBasedCalls.json

+  },
+  "x-ms-paths":{
+    "/{roleDefinitionId}?disambiguation_dummy": {
+      "get": {


To get rid of the linter error, both disambiguated paths should look exactly the same (and the one here with the disambiguation query). Example: /{hello} and /{hello}?waves. Not /{hello} and /{world}?waves

mcardosos · 2018-02-02T00:11:31Z

Since there are new operations, also adding @ravbhatnagar for review

mcardosos · 2018-02-02T00:14:12Z

On the other hand, we are moving away from current branch, please poke @marstr if you need help migrating the PR

AutorestCI · 2018-02-02T00:33:48Z

Did a commit to Azure/azure-sdk-for-python:
Azure/azure-sdk-for-python@0fafe0b

AutorestCI · 2018-02-02T00:36:34Z

Did a commit to Azure/azure-sdk-for-go:
Azure/azure-sdk-for-go@3299b1c

azuresdkciprbot · 2018-02-02T00:40:39Z

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): 0
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

mcardosos · 2018-02-02T01:17:18Z

Sorry @ravbhatnagar , I thought there were new operations, but that is not the case :)

AutorestCI · 2018-02-02T01:19:22Z

This commit was treated and no generation was made for Azure/azure-sdk-for-python

AutorestCI · 2018-02-02T01:22:37Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

darshanhs90 added 3 commits January 25, 2018 11:21

add 2018 preview version

25bc2cc

update calls with new versions

4340b78

move provider operations to new api

3fe7452

azuresdkci assigned mcardosos Jan 29, 2018

darshanhs90 changed the title ~~update Role based calls to new api version to support dataActions~~ update Role based calls to new api version to support dataActions[DONOT merge] Jan 29, 2018

AutorestCI mentioned this pull request Jan 29, 2018

Automatic PR of restapi_auto_darshanhs90_current into restapi_auto_current Azure/azure-sdk-for-python#1847

Closed

AutorestCI mentioned this pull request Jan 29, 2018

Automatic PR of restapi_auto_darshanhs90_current into restapi_auto_current Azure/azure-sdk-for-go#1010

Closed

darshanhs90 changed the title ~~update Role based calls to new api version to support dataActions[DONOT merge]~~ update Role based calls to new api version to support dataActions Jan 30, 2018

mcardosos reviewed Jan 31, 2018

View reviewed changes

address some of the comments

aa1e9fb

add disambiguation path

d00dab0

fix testing error

42f52c7

merge conflicts resolved

1206046

add missed out quotes

4e86c80

mcardosos reviewed Feb 2, 2018

View reviewed changes

mcardosos added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 2, 2018

fix linter issues

dc3eead

darshanhs90 changed the base branch from current to master February 2, 2018 00:35

mcardosos approved these changes Feb 2, 2018

View reviewed changes

mcardosos removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 2, 2018

mcardosos merged commit 88d2938 into Azure:master Feb 2, 2018

AutorestCI mentioned this pull request Feb 2, 2018

Automatic PR of restapi_auto_darshanhs90_current into master Azure/azure-sdk-for-python#1888

Closed

AutorestCI mentioned this pull request Feb 2, 2018

Automatic PR of restapi_auto_darshanhs90_current into dev Azure/azure-sdk-for-go#1037

Closed

darshanhs90 mentioned this pull request Feb 16, 2018

add dataaction attributes to permissions and Role Definition calls Azure/azure-sdk-for-net#4079

Merged

9 tasks

Conversation

darshanhs90 commented Jan 29, 2018 • 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!

AutorestCI commented Jan 29, 2018

Uh oh!

AutorestCI commented Jan 29, 2018

Uh oh!

darshanhs90 commented Jan 29, 2018

Uh oh!

mcardosos Jan 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mcardosos commented Jan 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcardosos commented Jan 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darshanhs90 commented Jan 31, 2018

Uh oh!

AutorestCI commented Jan 31, 2018

Uh oh!

AutorestCI commented Jan 31, 2018

Uh oh!

mcardosos commented Jan 31, 2018

Uh oh!

AutorestCI commented Jan 31, 2018

Uh oh!

AutorestCI commented Jan 31, 2018

Uh oh!

darshanhs90 commented Jan 31, 2018

Uh oh!

mcardosos commented Jan 31, 2018

Uh oh!

darshanhs90 commented Jan 31, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

azuresdkciprbot commented Feb 1, 2018

Uh oh!

AutorestCI commented Feb 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcardosos commented Feb 2, 2018

Uh oh!

mcardosos commented Feb 2, 2018

Uh oh!

AutorestCI commented Feb 2, 2018

darshanhs90 commented Jan 29, 2018 •

edited by azuresdkci

Loading

mcardosos Jan 31, 2018 •

edited

Loading

mcardosos commented Jan 31, 2018 •

edited

Loading