[Core] Fix get_token() issue in msi login and expiresIn key error in cloud shell login credentials for track 2 SDK related commands
#14187
Conversation
Juliehzl
requested review from
arrownj,
fengzhou-msft,
haroldrandom,
jiasli,
mmyyrroonn and
qianwens
as code owners
|
|
||
| @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) | ||
| @mock.patch('msrestazure.azure_active_directory.MSIAuthentication', autospec=True) | ||
| @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) |
There was a problem hiding this comment.
Is this change necessary? Won't it be nice to have MSIAuthenticationWrapper also tested?
There was a problem hiding this comment.
mock.patch by default will create a MagicMock instance which can't be inherited, maybe using new=MSIAuthentication may work? Haven't tested.
| from azure.core.credentials import AccessToken | ||
| from msrestazure.azure_active_directory import MSIAuthentication |
There was a problem hiding this comment.
Please delay these imports as they will cause significant latency on client-side commands.
There was a problem hiding this comment.
How about extracting MSIAuthenticationWrapper to src/azure-cli-core/azure/cli/core/adal_authentication.py and import it conditionally?
| try: | ||
| return AccessToken(token, int(full_token['expiresIn'] + time.time())) | ||
| except KeyError: | ||
| return AccessToken(token, int(full_token['expires_on'])) |
There was a problem hiding this comment.
This logic is for cloud shell? If yes, can we add comments so that others can understand the purpose?
|
add to S172 |
|
Please specify what "login credentials issue" is. |
| is_windows, is_wsl | ||
| from azure.cli.core.cloud import get_active_cloud, set_cloud_subscription | ||
|
|
||
| from .adal_authentication import MSIAuthenticationWrapper |
There was a problem hiding this comment.
Another problem with mock.patch is for this form of import, it won't work. After importing MSIAuthenticationWrapper, it becomes a local reference azure.cli.core._profile.MSIAuthenticationWrapper. See Where to patch.
Juliehzl
changed the title
Juliehzl
changed the title
expiresIn key error in cloud shell login credentials for track 2 SDK related commands
5 participants
Description
Testing Guide
az login --identityand try track 2 extensionHistory Notes
[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.