Skip to content

ManagementGroups API update (2018-01-01-preview)#2345

Merged
mcardosos merged 22 commits intoAzure:masterfrom
sendhil:update-microsoft-management-swagger
Feb 2, 2018
Merged

ManagementGroups API update (2018-01-01-preview)#2345
mcardosos merged 22 commits intoAzure:masterfrom
sendhil:update-microsoft-management-swagger

Conversation

@sendhil
Copy link
Contributor

@sendhil sendhil commented Jan 26, 2018

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

@AutorestCI
Copy link

This commit was treated and no generation was made for Python

1 similar comment
@AutorestCI
Copy link

This commit was treated and no generation was made for Python

* PUT /providers/Microsoft.Management/managementGroups/{groupName}/subscriptions/{subscriptionId} returns 204 instead of 200.
* DELETE /providers/Microsoft.Management/managementGroups/{groupName}/subscriptions/{subscriptionId} returns 204 instead of 200.
* GET /providers/Microsoft.Management/entities has been changed to POST /providers/Microsoft.Management/getEntities
* GET /providers/Microsoft.Management/entities/hierarchy has been changed to POST /providers/Microsoft.Management/getEntities?view=hierarchy.
* PUT /providers/Microsoft.Management/managementGroups/{groupName} had it's input structure change to match the response from the GET. Most of the properties are read only as we only need the displayName + parentId.
* Removed /providers/Microsoft.Management/operationResults endpoint.
@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@sendhil
Copy link
Contributor Author

sendhil commented Jan 28, 2018

Pushed some updates as per the changes suggested from our REST API board meeting.

  • PUT /providers/Microsoft.Management/managementGroups/{groupName}/subscriptions/{subscriptionId} returns 204 instead of 200.
  • DELETE /providers/Microsoft.Management/managementGroups/{groupName}/subscriptions/{subscriptionId} returns 204 instead of 200.
  • GET /providers/Microsoft.Management/entities has been changed to POST /providers/Microsoft.Management/getEntities
  • GET /providers/Microsoft.Management/entities/hierarchy has been changed to POST /providers/Microsoft.Management/getEntities?view=hierarchy.
  • PUT /providers/Microsoft.Management/managementGroups/{groupName} had it's input structure change to match the response from the GET. Most of the properties are read only as we only need the displayName + parentId.
  • Removed /providers/Microsoft.Management/operationResults endpoint.

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

"type": "string",
"description": "Page continuation token is only used if a previous operation returned a partial result. \nIf a previous response contains a nextLink element, the value of the nextLink element will include a token parameter that specifies a starting point to use for subsequent calls.\n"
},
"CacheControlHeader": {
Copy link
Contributor

@mcardosos mcardosos Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CacheControlHeader [](start = 5, length = 18)

Is this required? #Resolved

Copy link
Contributor Author

@sendhil sendhil Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There's a bug with arm pas caching that this bypasses. #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not mark it as required then?


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

Copy link
Contributor Author

@sendhil sendhil Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. #Resolved

"tags": [
"ManagementGroups"
],
"operationId": "ManagementGroups_List",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ManagementGroups_List [](start = 24, length = 21)

Odata extension could be useful here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only do a subset of Odata operations and the generators complain due to a lack of some parameters. e.g. "Method with x-ms-odata needs to have "$filter" parameter.

"id": {
"type": "string",
"description": "The fully qualified ID for the management group. For example, /providers/Microsoft.Management/managementGroups/0000000-0000-0000-0000-000000000000",
"readOnly": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readOnly [](start = 11, length = 8)

Why would there be readonly parameters in a model that is used to create a resource?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we only actually take the display name field and the parent id. This was done as our review wanted the put structure to more closely match the get structure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API review board you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - sorry, was typing from my phone.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okok :)

"description": "The friendly name of the management group. If no value is passed then this field will be set to the groupId.",
"x-nullable": true
},
"roles": {
Copy link
Contributor

@mcardosos mcardosos Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roles [](start = 9, length = 5)

What kind of roles? It sounds as if this could be expressed as an enum #Resolved

Copy link
Contributor Author

@sendhil sendhil Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Role definition ids #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be clarified in the description?


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

Copy link
Contributor Author

@sendhil sendhil Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. #Resolved

"description": "The name of the management group. For example, 00000000-0000-0000-0000-000000000000",
"readOnly": true
},
"provisioningState": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provisioningState [](start = 9, length = 17)

Could this be an enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now it's just one value... But we can change it

"type": "string",
"readOnly": true
},
"display": {
Copy link
Contributor

@mcardosos mcardosos Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

display [](start = 9, length = 7)

For code generation purposes, it is better no to define models inline #Resolved

"tags": [
"Entities"
],
"operationId": "Entities_List",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entities_List [](start = 24, length = 13)

Odata extension can be useful here too

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

@AutorestCI
Copy link

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

"host": "management.azure.com",
"info": {
"version": "2018-01-01-preview",
"title": "Management Groups API",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you adding this to the markdown file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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/resources/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 71
After the PR: Warning(s): 0 Error(s): 71

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

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/resources/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 71
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@AutorestCI
Copy link

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

Command: ['/usr/local/bin/autorest', '/tmp/tmpsh7fdubi/rest/specification/resources/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmpsh7fdubi/sdk', '--multiapi', '--package-version=v12.2.1-beta', '--use=@microsoft.azure/autorest.go@preview', "--user-agent='Azure-SDK-For-Go/v12.2.1-beta services'", '--verbose']
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)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (preview->3.0.40)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.1.22->2.1.22)
Processing batch task - {"tag":"package-2017-04-preview"} .
Failure during batch task - {"tag":"package-2017-04-preview"} -- Error: [Exception] No input files provided.

Use --help to get help information..
[Exception] No input files provided.

Use --help to get help information.

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Feb 2, 2018
@ravbhatnagar
Copy link
Contributor

This was reviewed over skype by Azure API review board and ARM. Signing off!

@AutorestCI
Copy link

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

Command: ['/usr/local/bin/autorest', '/tmp/tmpuvcktwmy/rest/specification/resources/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmpuvcktwmy/sdk', '--multiapi', '--package-version=v12.2.1-beta', '--use=@microsoft.azure/autorest.go@preview', "--user-agent='Azure-SDK-For-Go/v12.2.1-beta services'", '--verbose']
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)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (preview->3.0.40)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.1.22->2.1.22)
Processing batch task - {"tag":"package-2017-04-preview"} .
Failure during batch task - {"tag":"package-2017-04-preview"} -- Error: [Exception] No input files provided.

Use --help to get help information..
[Exception] No input files provided.

Use --help to get help information.

@AutorestCI
Copy link

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

1 similar comment
@AutorestCI
Copy link

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

Copy link
Contributor

@mcardosos mcardosos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the good teamwork :D

@mcardosos mcardosos merged commit f9ec240 into Azure:master Feb 2, 2018
@AutorestCI
Copy link

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

Command: ['/usr/local/bin/autorest', '/tmp/tmpqcw4b1rf/rest/specification/resources/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmpqcw4b1rf/sdk', '--multiapi', '--package-version=v12.2.1-beta', '--use=@microsoft.azure/autorest.go@preview', "--user-agent='Azure-SDK-For-Go/v12.2.1-beta services'", '--verbose']
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)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (preview->3.0.40)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.1.22->2.1.22)
Processing batch task - {"tag":"package-2017-04-preview"} .
Failure during batch task - {"tag":"package-2017-04-preview"} -- Error: [Exception] No input files provided.

Use --help to get help information..
[Exception] No input files provided.

Use --help to get help information.

@AutorestCI
Copy link

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

@AutorestCI
Copy link

Was unable to create SDK Azure/azure-sdk-for-python PR for this closed PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants