Skip to content

Comments

azure: Change the typings of the created clients#2193

Draft
bwateratmsft wants to merge 6 commits intomainfrom
bmw/azure/types
Draft

azure: Change the typings of the created clients#2193
bwateratmsft wants to merge 6 commits intomainfrom
bmw/azure/types

Conversation

@bwateratmsft
Copy link
Contributor

This would have build-time effects so I'm revving the major...shouldn't have any runtime impact, other than throwing a better error when using Azure Stack + createRoleDefinitionsItems().

const authClient = await createAuthorizationManagementClient([context, subContext]);

if (isProfileAuthorizationManagementClient(authClient)) {
throw new Error('TODO: no can do boss');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Do. The Azure Stack SDK does not have the client.roleAssignments.listForSubscription() method.

export async function createStorageClient(context: InternalAzExtClientContext): Promise<CommonStorageManagementClient> {
if (parseClientContext(context).isCustomCloud) {
return <StorageManagementClient><unknown>createAzureClient(context, (await import('@azure/arm-storage-profile-2020-09-01-hybrid')).StorageManagementClient);
return createAzureClient(context, (await import('@azure/arm-storage-profile-2020-09-01-hybrid')).StorageManagementClient);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The casts become unnecessary, by declaring that the return type is a union of the two--that way the callers don't use something that exists on the normal SDK but not on the Azure Stack one.

Comment on lines +17 to +19
export type CommonAuthorizationManagementClient = AuthorizationManagementClient | PAMC;
export type CommonResourcesClient = ResourceManagementClient | PRMC;
export type CommonStorageManagementClient = StorageManagementClient | PSMC;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These aren't in index.d.ts, should add them...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or just murder index.d.ts: #2194

{
roleDefinitionId, // Regular SDK wants this
principalId, // Regular SDK wants this
properties: { // Azure Stack SDK wants this instead
Copy link
Contributor

@MicroFish91 MicroFish91 Feb 11, 2026

Choose a reason for hiding this comment

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

So does this mean the regular SDK doesn't care about having the Stack SDK properties present (in that shape) and vice versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope so! I expect that either they won't get serialized into the request or the service will ignore them.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants