[Identity] Make credentials and DAC list public #9274
Conversation
| // @public | ||
| export class AzureCliCredential implements TokenCredential { | ||
| constructor(); | ||
| protected getAzureCliAccessToken(resource: string): Promise<unknown>; |
There was a problem hiding this comment.
Should this be refactored so it's not exposed?
There was a problem hiding this comment.
The mock will overwrite this. For that to work, it has to at least be protected. We can document that this isn't intended to be used publicly.
There was a problem hiding this comment.
Is there not another way to mock / test this? I'm not a big fan of adding public surface area for testing purposes. If we can't fix it for this preview could we file an issue to fix before GA?
There was a problem hiding this comment.
This is coming from the mocking that is used for testing, since we can't override that method if it's private.
Can look into a better approach for the next preview.
| @@ -35,6 +36,15 @@ export class DefaultAzureCredential extends ChainedTokenCredential { | |||
| credentials.push(new AzureCliCredential()); | |||
There was a problem hiding this comment.
Shouldn't tokenCredentialOptions also be passed to the AzureCliCredential?
There was a problem hiding this comment.
It doesn't currently accept or use the settings. We can take on some work for the next preview to bring AzureCli to the same style that the other credentials are using.
This makes a few changes to adjust to the current DAC recommendation:
In addition to these changes, we should also include running the manual tests against a ChainedTokenCredential built from this new DAC helper