Skip to content

Populate context list when no previous context found #6137

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

Merged
merged 6 commits into from
Jun 9, 2018

Conversation

cormacpayne
Copy link
Member

@cormacpayne cormacpayne commented May 7, 2018

Description

Fix for #6115

Checklist

Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

A couple of small changes. Also, can we write a unit test for this

string tempName = null;
if (!_profile.TryGetContextName(tempContext, out tempName))
{
WriteWarningMessage(string.Format("Unable to get context name for subscription with id '{0}'.", subscription.Id));
Copy link
Member

Choose a reason for hiding this comment

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

Let's put these strings into the assembly resources


if (!_profile.TrySetContext(tempName, tempContext))
{
WriteWarningMessage(string.Format("Cannot create a context for subscription with id '{0}'.", subscription.Id));
Copy link
Member

Choose a reason for hiding this comment

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

same

@@ -260,6 +261,35 @@ public bool TryRemoveContext(IAzureContext context)
}

_profile.DefaultContext.TokenCache = _cache;
if (shouldPopulateContextList)
Copy link
Member

Choose a reason for hiding this comment

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

Were we going to add an option in to the login cmdlet that would disable populating the contexts? Also, I think we should set a default limit to the number of populated contexts (say, 25).

@MiYanni
Copy link
Contributor

MiYanni commented Jun 6, 2018

@cormacpayne You have merge conflicts that need to be resolved.

@cormacpayne cormacpayne self-assigned this Jun 6, 2018
@markcowl
Copy link
Member

markcowl commented Jun 9, 2018

@markcowl markcowl removed their assignment Jun 9, 2018
@maddieclayton maddieclayton merged commit 663aa42 into Azure:preview Jun 9, 2018
@cormacpayne cormacpayne deleted the populate-contexts branch September 12, 2018 16:58
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