-
Notifications
You must be signed in to change notification settings - Fork 47
Use v6 of auth package #1292
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
base: main
Are you sure you want to change the base?
Use v6 of auth package #1292
Conversation
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import type { AzureSubscriptionProvider } from "@microsoft/vscode-azext-azureauth"; | ||
| import { type AzureDevOpsSubscriptionProviderInitializer, createAzureDevOpsSubscriptionProviderFactory } from "@microsoft/vscode-azext-azureauth/azdo"; |
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.
We should move this out so it's actually in the test code, and thus all the stuff doesn't get bundled.
| await provider.signIn(); | ||
| } finally { | ||
| _isLoggingIn = false; | ||
| // TODO: do we need this or does the session change event take care of it? |
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.
TODO: Do
| let selectedTenant: TenantIdDescription | undefined = undefined; | ||
|
|
||
| const subscriptions = await subscriptionProvider.getSubscriptions(false); | ||
| const subscriptions = await subscriptionProvider.getAvailableSubscriptions({ filter: false }); |
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.
This can be reduced to just the getAvailableSubscriptions call
| } else { | ||
| // All tenants are authenticated but no subscriptions exist | ||
| // The prior behavior was to still show the Select Subscriptions item in this case | ||
| // TODO: this isn't exactly right? Should we throw a `NotSignedInError` instead? |
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.
TODO: Do
| for await (const tenant of tenants) { | ||
| const isSignedIn = await subscriptionProvider.isSignedIn(nonNullProp(tenant, 'tenantId'), account); | ||
| for await (const tenant of allTenants) { | ||
| // TODO: This is n^2 which is not great, but the number of tenants is usually quite small |
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.
TODO: Do? Tenants can be as high as 650+...
| return getSignInTreeItems(false); | ||
| } | ||
|
|
||
| // TODO: Else do we throw? What did we do before? |
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.
TODO: Do
No description provided.