Skip to content

[Profile] Hotfix: Fix #15961: az login: UnboundLocalError: local variable 'token_entry' referenced before assignment - #15962

Merged
Jiashuo Li (jiasli) merged 2 commits into
Azure:releasefrom
evelyn-ys:refine_invalid_url
Nov 20, 2020
Merged

[Profile] Hotfix: Fix #15961: az login: UnboundLocalError: local variable 'token_entry' referenced before assignment#15962
Jiashuo Li (jiasli) merged 2 commits into
Azure:releasefrom
evelyn-ys:refine_invalid_url

Conversation

@evelyn-ys

@evelyn-ys Yishi Wang (evelyn-ys) commented Nov 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Fix #15961 : az login: UnboundLocalError: local variable 'token_entry' referenced before assignment
In previous pr #15776 , Invalid Url is captured and handled, but other exceptions are hidden inappropriately.
This pr revokes previous change, InvalidUrl handling need further discussion.

Testing Guide

evelyn-ys
Yishi Wang (evelyn-ys) commented Nov 18, 2020
View reviewed changes
Comment thread src/azure-cli-core/azure/cli/core/_profile.py Outdated

@evelyn-ys Yishi Wang (evelyn-ys) Nov 18, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There should be raise ex here

@yonzhan

Yong Zhang (yonzhan) commented Nov 18, 2020

Copy link
Copy Markdown
Collaborator

Profile

@yonzhan Yong Zhang (yonzhan) added this to the S179 milestone Nov 18, 2020
jiasli
Jiashuo Li (jiasli) reviewed Nov 19, 2020
View reviewed changes
Comment thread src/azure-cli/azure/cli/command_modules/profile/custom.py Outdated

@jiasli Jiashuo Li (jiasli) Nov 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A white space usually goes to the previous line.

@evelyn-ys Yishi Wang (evelyn-ys) Nov 19, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

jiasli
Jiashuo Li (jiasli) reviewed Nov 19, 2020
View reviewed changes
Comment thread src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_profile_custom.py Outdated
Comment on lines 133 to 134

@jiasli Jiashuo Li (jiasli) Nov 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A browse won't be available on CI machine, so auth code flow won't be used.

@evelyn-ys Yishi Wang (evelyn-ys) Nov 19, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Appreciate for your help! That makes sense why CI fails....

jiasli
Jiashuo Li (jiasli) reviewed Nov 19, 2020
View reviewed changes
Comment thread src/azure-cli-core/azure/cli/core/tests/test_profile.py Outdated

@jiasli Jiashuo Li (jiasli) Nov 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

adal.AuthenticationContext will not be called, since SubscriptionFinder is using lambda _, _1, _2: mock_auth_context.

mock_auth_context can simply be created as a MagicMock().

jiasli
Jiashuo Li (jiasli) reviewed Nov 19, 2020
View reviewed changes
Comment thread src/azure-cli/azure/cli/command_modules/profile/custom.py Outdated
jiasli
Jiashuo Li (jiasli) reviewed Nov 19, 2020
View reviewed changes
Comment thread src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_profile_custom.py Outdated

@jiasli Jiashuo Li (jiasli) Nov 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please make this a more general test case so that other errors can also be tested to avoid regression.

jiasli
Jiashuo Li (jiasli) reviewed Nov 19, 2020
View reviewed changes
Comment thread src/azure-cli/azure/cli/command_modules/profile/custom.py Outdated

@jiasli Jiashuo Li (jiasli) Nov 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please print the error detail.

@jiasli Jiashuo Li (jiasli) changed the title {Profile} refine InvalidUrl when login [Profile] Fix #15961: az login: UnboundLocalError: local variable 'token_entry' referenced before assignment Nov 20, 2020
@jiasli
Jiashuo Li (jiasli) changed the base branch from dev to release November 20, 2020 02:01
@jiasli Jiashuo Li (jiasli) changed the title [Profile] Fix #15961: az login: UnboundLocalError: local variable 'token_entry' referenced before assignment [Profile] Hotfix: Fix #15961: az login: UnboundLocalError: local variable 'token_entry' referenced before assignment Nov 20, 2020
@qwordy

Feiyue Yu (qwordy) commented Nov 20, 2020

Copy link
Copy Markdown
Member

Hard to review. The changes are mixed up with unrelated changes.

@jiasli

Jiashuo Li (jiasli) commented Nov 20, 2020

Copy link
Copy Markdown
Contributor

Hard to review. The changes are mixed up with unrelated changes.

I have fixed the history.

houk-ms
Houk (houk-ms) approved these changes Nov 20, 2020
View reviewed changes

@houk-ms Houk (houk-ms) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we explain more details about what's happening there in the pr description?

@jiasli

Jiashuo Li (jiasli) commented Nov 20, 2020

Copy link
Copy Markdown
Contributor

Could we explain more details about what's happening there in the pr description?

Will do a full analysis next week.

@jiasli
Jiashuo Li (jiasli) merged commit db2711a into Azure:release Nov 20, 2020
@jiasli Jiashuo Li (jiasli) mentioned this pull request Dec 23, 2020
Open
@jiasli Jiashuo Li (jiasli) mentioned this pull request Sep 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsntcy Yu Chen (jsntcy) jsntcy approved these changes
@fengzhou-msft Feng Zhou (fengzhou-msft) Awaiting requested review from fengzhou-msft
@haroldrandom Harold Ran (haroldrandom) Awaiting requested review from haroldrandom
@Juliehzl Zunli Hu (Juliehzl) Awaiting requested review from Juliehzl
@qwordy Feiyue Yu (qwordy) Awaiting requested review from qwordy
@bim-msft Bin Ma (bim-msft) Awaiting requested review from bim-msft
@yungezz Yunge Zhu (yungezz) Awaiting requested review from yungezz
@zhoxing-ms Xing Zhou (zhoxing-ms) Awaiting requested review from zhoxing-ms
+3 more reviewers
@jiasli Jiashuo Li (jiasli) jiasli left review comments
@arrownj Xiaojian Xu (arrownj) arrownj approved these changes
@houk-ms Houk (houk-ms) houk-ms approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@evelyn-ys Yishi Wang (evelyn-ys)

Labels

None yet

Projects

None yet

Milestone

S179

Development

Successfully merging this pull request may close these issues.

az login fails with UnboundLocalError: local variable 'token_entry' referenced before assignment

7 participants

@evelyn-ys @yonzhan @qwordy @jiasli @arrownj @jsntcy @houk-ms