-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add constructor taking tenant id as parameter for AzureCredentials constructor when using MSI #662
Add constructor taking tenant id as parameter for AzureCredentials constructor when using MSI #662
Conversation
Can one of the admins verify this patch? |
@@ -81,6 +81,12 @@ public AzureCredentials(MSILoginInformation msiLoginInformation, AzureEnvironmen | |||
this.msiTokenProviderFactory = new MSITokenProviderFactory(msiLoginInformation); | |||
} | |||
|
|||
public AzureCredentials(MSILoginInformation msiLoginInformation, string tenantId, AzureEnvironment environment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the tenantId is required, please change the above constructor directly, but not provide another one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the teantId as optional parameter as the last one and default value to null would be better, but not provide another API. Thanks.
@@ -81,6 +81,12 @@ public AzureCredentials(MSILoginInformation msiLoginInformation, AzureEnvironmen | |||
this.msiTokenProviderFactory = new MSITokenProviderFactory(msiLoginInformation); | |||
} | |||
|
|||
public AzureCredentials(MSILoginInformation msiLoginInformation, string tenantId, AzureEnvironment environment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the teantId as optional parameter as the last one and default value to null would be better, but not provide another API. Thanks.
@michaelelleby could you help update this PR? Thanks. |
… to allow talking to Azure AD through MSI
b3e1ab5
to
fcd2030
Compare
@yaohaizh I have updated the PR, to avoid introducing any breaking changes to the SDK. |
Thanks for your contributions! |
Fixes #661