From 532b31612f5688ed372aec7b51b8f39a90179367 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 21 Feb 2023 22:02:24 +1100 Subject: [PATCH] Allow users to cancel and esc from env creation (#12883) * Allow users to cancel and esc from env creation * Telemtry * fixes --- src/gdpr.ts | 1 + .../kernelSource/kernelSelector.ts | 48 ++++++++++++----- .../pythonEnvKernelConnectionCreator.ts | 54 +++++++++++++------ ...honEnvKernelConnectionCreator.unit.test.ts | 33 +++++++----- src/telemetry.ts | 8 +++ 5 files changed, 101 insertions(+), 43 deletions(-) diff --git a/src/gdpr.ts b/src/gdpr.ts index 5d06e5bdbf7..df865a2a050 100644 --- a/src/gdpr.ts +++ b/src/gdpr.ts @@ -44,6 +44,7 @@ "DATASCIENCE.CREATE_PYTHON_ENVIRONMENT" : { "reason": {"classification":"SystemMetaData","purpose":"PerformanceAndHealth","comment":"Reason for failure. cancelled - User cancelled the operation (cancellation token cancelled). kernelConnectionNotCreated - Kernel connection not created via the kernel finder.","owner":"donjayamanne"}, "dependenciesInstalled": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"Whether the kernel dependencies were installed or not.","owner":"donjayamanne"}, + "envType": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"Type of the Python environment.","owner":"donjayamanne"}, "${include}": [ "${F1}" diff --git a/src/notebooks/controllers/kernelSource/kernelSelector.ts b/src/notebooks/controllers/kernelSource/kernelSelector.ts index 3e9eadfa7dd..867ce1c3c0e 100644 --- a/src/notebooks/controllers/kernelSource/kernelSelector.ts +++ b/src/notebooks/controllers/kernelSource/kernelSelector.ts @@ -181,12 +181,6 @@ export class KernelSelector implements IDisposable { this.categories.set(item, new Set(items)); }); - const refreshButton: QuickInputButton = { iconPath: new ThemeIcon('refresh'), tooltip: Common.refresh }; - const refreshingButton: QuickInputButton = { - iconPath: new ThemeIcon('loading~spin'), - tooltip: Common.refreshing - }; - if ( !this.extensionChecker.isPythonExtensionInstalled && this.provider.kind === ContributedKernelFinderKind.LocalPythonEnvironment @@ -194,7 +188,9 @@ export class KernelSelector implements IDisposable { this.installPythonExtItems.push(this.installPythonExtension); } - let quickPickToBeUpdated: QuickPick | undefined; + const quickPickToBeUpdated: { quickPick: QuickPick | undefined } = { + quickPick: undefined + }; if ( this.extensionChecker.isPythonExtensionInstalled && this.provider.kind === ContributedKernelFinderKind.LocalPythonEnvironment @@ -213,14 +209,14 @@ export class KernelSelector implements IDisposable { ) { if (this.workspace.isTrusted) { this.installPythonItems.push(this.installPythonItem); - if (quickPickToBeUpdated) { - this.updateQuickPickItems(quickPickToBeUpdated); + if (quickPickToBeUpdated.quickPick) { + this.updateQuickPickItems(quickPickToBeUpdated.quickPick); } } } else if (this.provider.kernels.length) { this.installPythonItems.length = 0; - if (quickPickToBeUpdated) { - this.updateQuickPickItems(quickPickToBeUpdated); + if (quickPickToBeUpdated.quickPick) { + this.updateQuickPickItems(quickPickToBeUpdated.quickPick); } } }; @@ -247,6 +243,21 @@ export class KernelSelector implements IDisposable { ); } } + return this.selectKernelImpl(quickPickFactory, quickPickToBeUpdated); + } + public async selectKernelImpl( + quickPickFactory: CreateAndSelectItemFromQuickPick, + quickPickToBeUpdated: { quickPick: QuickPick | undefined } + ): Promise< + | { selection: 'controller'; finder: IContributedKernelFinder; connection: KernelConnectionMetadata } + | { selection: 'userPerformedSomeOtherAction' } + | undefined + > { + const refreshButton: QuickInputButton = { iconPath: new ThemeIcon('refresh'), tooltip: Common.refresh }; + const refreshingButton: QuickInputButton = { + iconPath: new ThemeIcon('loading~spin'), + tooltip: Common.refreshing + }; const { quickPick, selection } = quickPickFactory({ title: this.provider.title, items: this.installPythonItems @@ -263,7 +274,7 @@ export class KernelSelector implements IDisposable { } } }); - quickPickToBeUpdated = quickPick; + quickPickToBeUpdated.quickPick = quickPick; if (this.provider.status === 'discovering') { quickPick.busy = true; } @@ -309,6 +320,8 @@ export class KernelSelector implements IDisposable { } catch (ex) { if (ex instanceof SomeOtherActionError) { return { selection: 'userPerformedSomeOtherAction' }; + } else if (ex === InputFlowAction.back) { + return this.selectKernelImpl(quickPickFactory, quickPickToBeUpdated); } throw ex; } @@ -319,14 +332,21 @@ export class KernelSelector implements IDisposable { throw InputFlowAction.back; } } - private onCreatePythonEnvironment() { + private async onCreatePythonEnvironment() { const cancellationToken = new CancellationTokenSource(); this.disposables.push(new Disposable(() => cancellationToken.cancel())); this.disposables.push(cancellationToken); const creator = new PythonEnvKernelConnectionCreator(this.notebook, cancellationToken.token); this.disposables.push(creator); - return creator.createPythonEnvFromKernelPicker(); + const result = await creator.createPythonEnvFromKernelPicker(); + if ('action' in result) { + if (result.action === 'Cancel') { + throw InputFlowAction.cancel; + } + throw InputFlowAction.back; + } + return result.kernelConnection; } private updateQuickPickItems(quickPick: QuickPick) { quickPick.title = this.provider.title; diff --git a/src/notebooks/controllers/pythonEnvKernelConnectionCreator.ts b/src/notebooks/controllers/pythonEnvKernelConnectionCreator.ts index 0ab3aa24163..f5e7d3d83a5 100644 --- a/src/notebooks/controllers/pythonEnvKernelConnectionCreator.ts +++ b/src/notebooks/controllers/pythonEnvKernelConnectionCreator.ts @@ -23,6 +23,7 @@ import { IControllerRegistration } from './types'; type CreateEnvironmentResult = { path: string | undefined; + action?: 'Back' | 'Cancel'; }; /** @@ -61,20 +62,32 @@ export class PythonEnvKernelConnectionCreator { /** * Creates a Python Environment & then returns the corresponding kernel connection for the newly created python env. */ - public async createPythonEnvFromKernelPicker() { + public async createPythonEnvFromKernelPicker(): Promise< + | { + kernelConnection: PythonKernelConnectionMetadata; + } + | { + action: 'Back' | 'Cancel'; + } + > { let env: PythonEnvironment | undefined; - env = await this.createPythonEnvironment(); - if (this.cancelTokeSource.token.isCancellationRequested || !env) { + const envResult = await this.createPythonEnvironment(); + if (this.cancelTokeSource.token.isCancellationRequested || envResult.action) { sendTelemetryEvent(Telemetry.CreatePythonEnvironment, undefined, { failed: true, reason: 'cancelled' }); - return; + return { action: envResult.action || 'Cancel' }; + } + if (!envResult.interpreter) { + sendTelemetryEvent(Telemetry.CreatePythonEnvironment, undefined, { failed: true, reason: 'cancelled' }); + return { action: 'Cancel' }; } + env = envResult.interpreter; traceVerbose(`Python Environment created ${env.id}`); - const kernelConnection = await this.waitForPythonKernel(env); + const kernelConnection = await this.waitForPythonKernel(envResult.interpreter); if (this.cancelTokeSource.token.isCancellationRequested) { sendTelemetryEvent(Telemetry.CreatePythonEnvironment, undefined, { failed: true, reason: 'cancelled' }); - return; + return { action: 'Cancel' }; } if (!kernelConnection) { sendTelemetryEvent(Telemetry.CreatePythonEnvironment, undefined, { @@ -82,7 +95,7 @@ export class PythonEnvKernelConnectionCreator { reason: 'kernelConnectionNotCreated' }); traceVerbose(`Python Environment ${env.id} not found as a kernel`); - return; + return { action: 'Cancel' }; } traceVerbose(`Python Environment ${env.id} found as a kernel ${kernelConnection.kind}:${kernelConnection.id}`); const dependencyService = ServiceContainer.instance.get(IKernelDependencyService); @@ -105,8 +118,11 @@ export class PythonEnvKernelConnectionCreator { ); } - sendTelemetryEvent(Telemetry.CreatePythonEnvironment, undefined, { dependenciesInstalled }); - return kernelConnection; + sendTelemetryEvent(Telemetry.CreatePythonEnvironment, undefined, { + dependenciesInstalled, + envType: envResult.interpreter.envType + }); + return { kernelConnection }; } private async waitForPythonKernel(env: PythonEnvironment) { const kernelFinder = ServiceContainer.instance.get(IKernelFinder); @@ -162,7 +178,7 @@ export class PythonEnvKernelConnectionCreator { return kernel; }); } - private async createPythonEnvironment() { + private async createPythonEnvironment(): Promise<{ interpreter?: PythonEnvironment; action?: 'Back' | 'Cancel' }> { const interpreterService = ServiceContainer.instance.get(IInterpreterService); const cancellation = new CancellationTokenSource(); // While we're busy creating this env ignore other events from python extension @@ -171,28 +187,36 @@ export class PythonEnvKernelConnectionCreator { // we'll end up creating a controller for this venv and that will get selected by vscode interpreterService.pauseInterpreterDetection(cancellation.token); try { - const result: CreateEnvironmentResult = await commands.executeCommand('python.createEnvironment'); + const result: CreateEnvironmentResult = await commands.executeCommand('python.createEnvironment', { + showBackButton: true + }); + if (result?.action === 'Cancel') { + return { action: 'Cancel' }; + } + if (result?.action === 'Back') { + return { action: 'Back' }; + } const path = result?.path; if (this.cancelTokeSource.token.isCancellationRequested) { - return; + return { action: 'Cancel' }; } if (!path) { traceWarning( `Python Environment not created, either user cancelled the creation or there was an error in the Python Extension` ); - return; + return { action: 'Cancel' }; } this.createdEnvId = path; traceVerbose(`Python Environment created ${path}`); const env = await interpreterService.getInterpreterDetails({ path }); if (this.cancelTokeSource.token.isCancellationRequested) { - return; + return { action: 'Cancel' }; } if (!env) { traceWarning(`No interpreter details for New Python Environment ${getDisplayPath(Uri.file(path))}`); } this.createdEnvId = env?.id; - return env; + return { interpreter: env }; } finally { cancellation.cancel(); cancellation.dispose(); diff --git a/src/notebooks/controllers/pythonEnvKernelConnectionCreator.unit.test.ts b/src/notebooks/controllers/pythonEnvKernelConnectionCreator.unit.test.ts index 4474c990e70..62833044a42 100644 --- a/src/notebooks/controllers/pythonEnvKernelConnectionCreator.unit.test.ts +++ b/src/notebooks/controllers/pythonEnvKernelConnectionCreator.unit.test.ts @@ -115,15 +115,17 @@ suite('Python Environment Kernel Connection Creator', () => { }); teardown(() => disposeAllDisposables(disposables)); test('Not does create a Python Env when Python extension fails to create it', async () => { - when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment')).thenResolve(undefined); + when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment', anything())).thenResolve( + undefined + ); const kernel = await pythonEnvKernelConnectionCreator.createPythonEnvFromKernelPicker(); - - assert.isUndefined(kernel); + const connection = 'kernelConnection' in kernel ? kernel.kernelConnection : undefined; + assert.isUndefined(connection); }); test('Can cancel after creation of the Environment', async () => { const newCondaEnvPath = '/.conda'; - when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment')).thenResolve({ + when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment', anything())).thenResolve({ path: newCondaEnvPath } as any); when(localPythonEnvFinder.kernels).thenReturn([venvPythonKernel]); @@ -133,14 +135,14 @@ suite('Python Environment Kernel Connection Creator', () => { return Promise.resolve(newCondaPythonKernel.interpreter); }); - const kernelPromise = pythonEnvKernelConnectionCreator.createPythonEnvFromKernelPicker(); - - assert.isUndefined(await kernelPromise); + const kernel = await pythonEnvKernelConnectionCreator.createPythonEnvFromKernelPicker(); + const connection = 'kernelConnection' in kernel ? kernel.kernelConnection : undefined; + assert.isUndefined(connection); verify(interpreterService.getInterpreterDetails(deepEqual({ path: newCondaEnvPath }))).once(); }); test('Installs missing dependencies and returns the kernel connection', async () => { const newCondaEnvPath = '/.conda'; - when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment')).thenResolve({ + when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment', anything())).thenResolve({ path: newCondaEnvPath } as any); when(localPythonEnvFinder.kernels).thenReturn([venvPythonKernel, newCondaPythonKernel]); @@ -153,8 +155,11 @@ suite('Python Environment Kernel Connection Creator', () => { ); const kernel = await pythonEnvKernelConnectionCreator.createPythonEnvFromKernelPicker(); - - assert.strictEqual(kernel, newCondaPythonKernel); + const kernelConnection = 'kernelConnection' in kernel ? kernel.kernelConnection : undefined; + if (!kernelConnection) { + assert.fail('Kernel Connection not created'); + } + assert.strictEqual(kernelConnection, newCondaPythonKernel); verify(interpreterService.getInterpreterDetails(deepEqual({ path: newCondaEnvPath }))).once(); verify(kernelDependencyService.installMissingDependencies(anything())).once(); const args = capture(kernelDependencyService.installMissingDependencies).first()[0]; @@ -166,7 +171,7 @@ suite('Python Environment Kernel Connection Creator', () => { }); test('Abort creation if another controller is selected for the notebook', async () => { const newCondaEnvPath = '/.conda'; - when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment')).thenResolve({ + when(mockedVSCodeNamespaces.commands.executeCommand('python.createEnvironment', anything())).thenResolve({ path: newCondaEnvPath } as any); when(localPythonEnvFinder.kernels).thenReturn([venvPythonKernel]); @@ -183,9 +188,9 @@ suite('Python Environment Kernel Connection Creator', () => { return Promise.resolve(newCondaPythonKernel.interpreter); }); - const kernelPromise = pythonEnvKernelConnectionCreator.createPythonEnvFromKernelPicker(); - - assert.isUndefined(await kernelPromise); + const kernel = await pythonEnvKernelConnectionCreator.createPythonEnvFromKernelPicker(); + const connection = 'kernelConnection' in kernel ? kernel.kernelConnection : undefined; + assert.isUndefined(connection); verify(interpreterService.getInterpreterDetails(deepEqual({ path: newCondaEnvPath }))).once(); }); }); diff --git a/src/telemetry.ts b/src/telemetry.ts index 2f39960c2e5..0e9fa00edcf 100644 --- a/src/telemetry.ts +++ b/src/telemetry.ts @@ -3139,6 +3139,10 @@ export class IEventNamePropertyMapping { * kernelConnectionNotCreated - Kernel connection not created via the kernel finder. */ reason: 'cancelled' | 'kernelConnectionNotCreated'; + /** + * Type of the Python environment. + */ + envType?: EnvironmentType; } | /* Creation succeeded */ { /** @@ -3159,6 +3163,10 @@ export class IEventNamePropertyMapping { dependenciesInstalled: { classification: 'SystemMetaData', purpose: 'FeatureInsight' + }, + envType: { + classification: 'SystemMetaData', + purpose: 'FeatureInsight' } } };