Synapse LLC for AccessControl#1
Synapse LLC for AccessControl#1annelo-msft merged 2 commits intoannelo-msft:users/annelo/synapse-rbac-llcfrom
Conversation
| public IList<string> Actions { get; } | ||
|
|
||
| public IList<string> NotActions { get; } | ||
| // TODO: change the below type from 'string' to 'SynapseDataAction', populate the list of actions |
There was a problem hiding this comment.
Thanks for including these comments! Would you mind adding to them that the reason is to be consistent with the ARM and KeyVault RBAC APIs, just so that doesn't get forgotten?
| } | ||
| } | ||
|
|
||
| // TODO: do not use roleScope ? |
There was a problem hiding this comment.
It will be interesting to understand why the ARM and KV RBAC APIs have this and Synapse API doesn't. I agree that we shouldn't take roleScope as a parameter if we're not using it in the method.
|
|
||
| // TODO: do not use roleScope ? | ||
| // GetRoleAssignmentById(string roleAssignmentId, RequestOptions options = null) | ||
| public virtual Response<SynapseRoleAssignment> GetRoleAssignment(SynapseRoleScope roleScope, string roleAssignmentName, CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
We should understand the difference between roleAssignmentName and roleAssignmentId as well. Are these different names for the same thing in both APIs, or would they actually take different values?
| } | ||
| } | ||
|
|
||
| // TODO: change return type from list to pageable |
There was a problem hiding this comment.
// TODO: change return type from list to pageable [](http://example.com/codeflow?start=2&length=55)
Have you got a chance to test the method after changing the returning type to pagable?
There was a problem hiding this comment.
No, according to our offline discuss, may be we should first add "x-ms-pageable" in swagger file, and then test the rewrite method based on the new generated LLC APIs?
…dentifier conversion (Azure#28574) * Added support for rawId<->CommunicationIdentifier conversion * fixed tests * fixed compiler suggestions * updated generated api surface * fixed tests + added serialization of rawid for communication user * fixing recordings #1 - for chat * update recordings with serialized rawId * add a changelog record for rawId (de)serialization * adjusted comments * gethashcode adjustments * lazy property init * updated api * testing that CommunicationIdentifier subclasses implement RawId
4 participants
All SDK Contribution checklist:
This checklist is used to make sure that common guidelines for a pull request are followed.
Draftmode if it is:General Guidelines and Best Practices
Testing Guidelines
SDK Generation Guidelines
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK. Please double check nuget.org current release version.Additional management plane SDK specific contribution checklist:
Note: Only applies to
Microsoft.Azure.Management.[RP]orAzure.ResourceManager.[RP]Management plane SDK Troubleshooting
If this is very first SDK for a services and you are adding new service folders directly under /SDK, please add
new servicelabel and/or contact assigned reviewer.If the check fails at the
Verify Code Generationstep, please ensure:generate.ps1/cmdto generate this PR instead of callingautorestdirectly.Please pay attention to the @microsoft.csharp version output after running
generate.ps1. If it is lower than current released version (2.3.82), please run it again as it should pull down the latest version.Note: We have recently updated the PSH module called by
generate.ps1to emit additional data. This would help reduce/eliminate the Code Verification check error. Please run following command:Old outstanding PR cleanup
Please note:
If PRs (including draft) has been out for more than 60 days and there are no responses from our query or followups, they will be closed to maintain a concise list for our reviewers.