Skip to content
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

login: set default subscription to one with the state of "Enabled" #2575

Merged
merged 3 commits into from
Mar 21, 2017

Conversation

yugangw-msft
Copy link
Contributor

@yugangw-msft yugangw-msft commented Mar 21, 2017

fix #2568

@codecov-io
Copy link

Codecov Report

Merging #2575 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2575      +/-   ##
==========================================
+ Coverage   72.32%   72.32%   +<.01%     
==========================================
  Files         363      363              
  Lines       19835    19838       +3     
  Branches     2923     2924       +1     
==========================================
+ Hits        14345    14348       +3     
  Misses       4582     4582              
  Partials      908      908
Impacted Files Coverage Δ
src/azure-cli-core/azure/cli/core/_profile.py 84.19% <100%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b6324d...3eb700a. Read the comment docs.

Copy link
Member

@derekbekoe derekbekoe left a comment

Choose a reason for hiding this comment

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

LGTM with one question.

def _pick_working_subscription(subscriptions):
from azure.mgmt.resource.subscriptions.models import SubscriptionState
s = next((x for x in subscriptions if x['state'] == SubscriptionState.enabled.value), None)
return s or subscriptions[0]
Copy link
Member

Choose a reason for hiding this comment

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

What if the subscriptions list is empty? Is that a scenario?

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 caller of _set_subscriptions will ensure the subscriptions will not be empty, so we are fine. Thanks for asking.

@yugangw-msft yugangw-msft merged commit 307c009 into Azure:master Mar 21, 2017
@yugangw-msft yugangw-msft deleted the nodisabled branch March 21, 2017 20:00
IsaacCalligeros95 added a commit to OctopusDeploy/WorkerTools that referenced this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants