Skip to content

Add princiaplType to arm-authorization.#4331

Closed
ahmagdy wants to merge 2 commits intoAzure:masterfrom
ahmagdy:authorization-principalType
Closed

Add princiaplType to arm-authorization.#4331
ahmagdy wants to merge 2 commits intoAzure:masterfrom
ahmagdy:authorization-principalType

Conversation

@ahmagdy
Copy link

@ahmagdy ahmagdy commented Jul 16, 2019

Include princiaplType in the parsed response and add it the model.
In my case, I tried to use AuthorizationManagmentClient to get the role assignments for a resource group, authorizationManagmentClient.listForResourceGroup('NAME').
The response will include principal id and principal type, the SDK will only show the principal id.
It's helpful to parse the type and include it because it will help to get more information about the principal id.
So this PR is extending this part.

ahmagdy added 2 commits July 16, 2019 20:56
Map PrincipalType and make it included in the parsed response.
@ahmagdy
Copy link
Author

ahmagdy commented Jul 17, 2019

@mikeharder @daviwil @chradek @ramya-rao-a @sadasant
Can you check my PR?

@maggiepint
Copy link
Contributor

@rthorn17 - is this something you can help with?

@ramya-rao-a
Copy link
Contributor

The code here is mainly auto-generated code, so I don't believe we can accept manual changes in PRs.
@amarzavery ?

@ramya-rao-a ramya-rao-a added Authorization Mgmt This issue is related to a management-plane library. labels Jul 19, 2019
@ramya-rao-a
Copy link
Contributor

@daviwil From your recent code-gen experience, would you say that this change needs to happen in the swagger spec first and then the code re-generated?

@amarzavery
Copy link
Contributor

May be we just need to regenerate with latest swagger spec for authorization

@sarangan12
Copy link
Contributor

@ahmad-magdy A new version of @azure/arm-authorization (8.3.2) has been released which has the changes to include principalType. So, I am closing this PR.

@sarangan12 sarangan12 closed this Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Authorization Mgmt This issue is related to a management-plane library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants