diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index 7e85afd69945..5dd9bbc28716 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -113,8 +113,8 @@ export interface IPythonExecutionFactory { create(options: ExecutionFactoryCreationOptions): Promise; /** * Creates a daemon Python Process. - * On windows its cheapter to create a daemon and use that than spin up Python Processes everytime. - * If something cannot be executed within the daemin, it will resort to using the stanard IPythonExecutionService. + * On windows it's cheaper to create a daemon and use that than spin up Python Processes everytime. + * If something cannot be executed within the daemon, it will resort to using the standard IPythonExecutionService. * Note: The returned execution service is always using an activated environment. * * @param {ExecutionFactoryCreationOptions} options diff --git a/src/client/datascience/activation.ts b/src/client/datascience/activation.ts index 4ff5b35c8d7c..7e73b10dd1a3 100644 --- a/src/client/datascience/activation.ts +++ b/src/client/datascience/activation.ts @@ -28,6 +28,8 @@ export class Activation implements IExtensionSingleActivationService { public async activate(): Promise { this.disposables.push(this.notebookProvider.onDidOpenNotebookEditor(this.onDidOpenNotebookEditor, this)); this.disposables.push(this.jupyterInterpreterService.onDidChangeInterpreter(this.onDidChangeInterpreter, this)); + // Warm up our selected interpreter for the extension + this.jupyterInterpreterService.setInitialInterpreter().ignoreErrors(); await this.contextService.activate(); } diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts index 042e931ca3d3..d12d56d02749 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts @@ -8,6 +8,7 @@ import { Event, EventEmitter } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import { createPromiseFromCancellation } from '../../../common/cancellation'; import '../../../common/extensions'; +import { noop } from '../../../common/utils/misc'; import { IInterpreterService, PythonInterpreter } from '../../../interpreter/contracts'; import { sendTelemetryEvent } from '../../../telemetry'; import { Telemetry } from '../../constants'; @@ -22,8 +23,8 @@ import { JupyterInterpreterStateStore } from './jupyterInterpreterStateStore'; @injectable() export class JupyterInterpreterService { private _selectedInterpreter?: PythonInterpreter; - private _selectedInterpreterPath?: string; private _onDidChangeInterpreter = new EventEmitter(); + private getInitialInterpreterPromise: Promise | undefined; public get onDidChangeInterpreter(): Event { return this._onDidChangeInterpreter.event; } @@ -45,50 +46,30 @@ export class JupyterInterpreterService { * @memberof JupyterInterpreterService */ public async getSelectedInterpreter(token?: CancellationToken): Promise { - if (this._selectedInterpreter) { - return this._selectedInterpreter; - } + // Before we return _selected interpreter make sure that we have run our initial set interpreter once + // because _selectedInterpreter can be changed by other function and at other times, this promise + // is cached to only run once + await this.setInitialInterpreter(token); - const resolveToUndefinedWhenCancelled = createPromiseFromCancellation({ - cancelAction: 'resolve', - defaultValue: undefined, - token - }); - // For backwards compatiblity check if we have a cached interpreter (older version of extension). - // If that interpreter has everything we need then use that. - let interpreter = await Promise.race([ - this.getInterpreterFromChangeOfOlderVersionOfExtension(), - resolveToUndefinedWhenCancelled - ]); - if (interpreter) { - return interpreter; - } + return this._selectedInterpreter; + } - const pythonPath = this._selectedInterpreterPath || this.interpreterSelectionState.selectedPythonPath; - if (!pythonPath) { - // Check if current interpreter has all of the required dependencies. - // If yes, then use that. - interpreter = await this.interpreterService.getActiveInterpreter(undefined); - if (!interpreter) { - return; - } - // Use this interpreter going forward. - if (await this.interpreterConfiguration.areDependenciesInstalled(interpreter)) { - this.setAsSelectedInterpreter(interpreter); - return interpreter; - } - return; + // To be run one initial time. Check our saved locations and then current interpreter to try to start off + // with a valid jupyter interpreter + public async setInitialInterpreter(token?: CancellationToken): Promise { + if (!this.getInitialInterpreterPromise) { + this.getInitialInterpreterPromise = this.getInitialInterpreterImpl(token).then(result => { + // Set ourselves as a valid interpreter if we found something + if (result) { + this.changeSelectedInterpreterProperty(result); + } + return result; + }); } - const interpreterDetails = await Promise.race([ - this.interpreterService.getInterpreterDetails(pythonPath, undefined), - resolveToUndefinedWhenCancelled - ]); - if (interpreterDetails) { - this._selectedInterpreter = interpreterDetails; - } - return interpreterDetails; + return this.getInitialInterpreterPromise; } + /** * Selects and interpreter to run jupyter server. * Validates and configures the interpreter. @@ -116,7 +97,7 @@ export class JupyterInterpreterService { const result = await this.interpreterConfiguration.installMissingDependencies(interpreter, undefined, token); switch (result) { case JupyterInterpreterDependencyResponse.ok: { - this.setAsSelectedInterpreter(interpreter); + await this.setAsSelectedInterpreter(interpreter); return interpreter; } case JupyterInterpreterDependencyResponse.cancel: @@ -126,30 +107,97 @@ export class JupyterInterpreterService { return this.selectInterpreter(token); } } - private async getInterpreterFromChangeOfOlderVersionOfExtension(): Promise { + + // Check the location that we stored jupyter launch path in the old version + // if it's there, return it and clear the location + private getInterpreterFromChangeOfOlderVersionOfExtension(): string | undefined { const pythonPath = this.oldVersionCacheStateStore.getCachedInterpreterPath(); if (!pythonPath) { return; } + + // Clear the cache to not check again + this.oldVersionCacheStateStore.clearCache().ignoreErrors(); + return pythonPath; + } + + // Set the specified interpreter as our current selected interpreter + private async setAsSelectedInterpreter(interpreter: PythonInterpreter): Promise { + // Make sure that our initial set has happened before we allow a set so that + // calculation of the initial interpreter doesn't clobber the existing one + await this.setInitialInterpreter(); + this.changeSelectedInterpreterProperty(interpreter); + } + + private changeSelectedInterpreterProperty(interpreter: PythonInterpreter) { + this._selectedInterpreter = interpreter; + this._onDidChangeInterpreter.fire(interpreter); + this.interpreterSelectionState.updateSelectedPythonPath(interpreter.path); + sendTelemetryEvent(Telemetry.SelectJupyterInterpreter, undefined, { result: 'selected' }); + } + + // For a given python path check if it can run jupyter for us + // if so, return the interpreter + private async validateInterpreterPath( + pythonPath: string, + token?: CancellationToken + ): Promise { try { - const interpreter = await this.interpreterService.getInterpreterDetails(pythonPath, undefined); + const resolveToUndefinedWhenCancelled = createPromiseFromCancellation({ + cancelAction: 'resolve', + defaultValue: undefined, + token + }); + + // First see if we can get interpreter details + const interpreter = await Promise.race([ + this.interpreterService.getInterpreterDetails(pythonPath, undefined), + resolveToUndefinedWhenCancelled + ]); + if (interpreter) { + // Then check that dependencies are installed + if (await this.interpreterConfiguration.areDependenciesInstalled(interpreter, token)) { + return interpreter; + } + } + } catch (_err) { + // For any errors we are ok with just returning undefined for an invalid interpreter + noop(); + } + return undefined; + } + + private async getInitialInterpreterImpl(token?: CancellationToken): Promise { + let interpreter: PythonInterpreter | undefined; + + // Check the old version location first, we will clear it if we find it here + const oldVersionPythonPath = this.getInterpreterFromChangeOfOlderVersionOfExtension(); + if (oldVersionPythonPath) { + interpreter = await this.validateInterpreterPath(oldVersionPythonPath, token); + } + + // Next check the saved global path + if (!interpreter && this.interpreterSelectionState.selectedPythonPath) { + interpreter = await this.validateInterpreterPath(this.interpreterSelectionState.selectedPythonPath, token); + + // If we had a global path, but it's not valid, trash it if (!interpreter) { - return; + this.interpreterSelectionState.updateSelectedPythonPath(undefined); } - if (await this.interpreterConfiguration.areDependenciesInstalled(interpreter)) { - this.setAsSelectedInterpreter(interpreter); - return interpreter; + } + + // Nothing saved found, so check our current interpreter + if (!interpreter) { + const currentInterpreter = await this.interpreterService.getActiveInterpreter(undefined); + + if (currentInterpreter) { + // Ask and give a chance to install dependencies in current interpreter + if (await this.interpreterConfiguration.areDependenciesInstalled(currentInterpreter, token)) { + interpreter = currentInterpreter; + } } - // If dependencies are not installed, then ignore it. lets continue with the current logic. - } finally { - // Don't perform this check again, just clear the cache. - this.oldVersionCacheStateStore.clearCache().ignoreErrors(); } - } - private setAsSelectedInterpreter(interpreter: PythonInterpreter): void { - this._selectedInterpreter = interpreter; - this._onDidChangeInterpreter.fire(interpreter); - this.interpreterSelectionState.updateSelectedPythonPath((this._selectedInterpreterPath = interpreter.path)); - sendTelemetryEvent(Telemetry.SelectJupyterInterpreter, undefined, { result: 'selected' }); + + return interpreter; } } diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts index d4e3585b2c5c..3fecb01848db 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts @@ -11,11 +11,11 @@ import { noop } from '../../../common/utils/misc'; const key = 'INTERPRETER_PATH_SELECTED_FOR_JUPYTER_SERVER'; const keySelected = 'INTERPRETER_PATH_WAS_SELECTED_FOR_JUPYTER_SERVER'; /** - * Keeps track of whether the user ever selected an interpreter to be used as the gloabl jupyter interpreter. + * Keeps track of whether the user ever selected an interpreter to be used as the global jupyter interpreter. * Keeps track of the interpreter path of the interpreter used as the global jupyter interpreter. * * @export - * @class JupyterInterpreterFinderEverSet + * @class JupyterInterpreterStateStore */ @injectable() export class JupyterInterpreterStateStore { @@ -27,7 +27,6 @@ export class JupyterInterpreterStateStore { * * @readonly * @type {Promise} - * @memberof JupyterInterpreterFinderEverSet */ public get interpreterSetAtleastOnce(): boolean { return !!this.selectedPythonPath || this.memento.get(keySelected, false); @@ -35,7 +34,7 @@ export class JupyterInterpreterStateStore { public get selectedPythonPath(): string | undefined { return this._interpreterPath || this.memento.get(key, undefined); } - public updateSelectedPythonPath(value: string) { + public updateSelectedPythonPath(value: string | undefined) { this._interpreterPath = value; this.memento.update(key, value).then(noop, noop); this.memento.update(keySelected, true).then(noop, noop); diff --git a/src/test/datascience/activation.unit.test.ts b/src/test/datascience/activation.unit.test.ts index 03cc7f3d166e..d7d996ef258a 100644 --- a/src/test/datascience/activation.unit.test.ts +++ b/src/test/datascience/activation.unit.test.ts @@ -53,6 +53,7 @@ suite('Data Science - Activation', () => { ); when(jupyterInterpreterService.getSelectedInterpreter()).thenResolve(interpreter); when(jupyterInterpreterService.getSelectedInterpreter(anything())).thenResolve(interpreter); + when(jupyterInterpreterService.setInitialInterpreter()).thenResolve(interpreter); await activator.activate(); }); teardown(() => fakeTimer.uninstall()); diff --git a/src/test/datascience/jupyter/interpreter/jupyterInterpreterService.unit.test.ts b/src/test/datascience/jupyter/interpreter/jupyterInterpreterService.unit.test.ts index 84d9d4e98398..4573149a357f 100644 --- a/src/test/datascience/jupyter/interpreter/jupyterInterpreterService.unit.test.ts +++ b/src/test/datascience/jupyter/interpreter/jupyterInterpreterService.unit.test.ts @@ -4,7 +4,7 @@ 'use strict'; import { assert } from 'chai'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anyString, anything, instance, mock, verify, when } from 'ts-mockito'; import { Memento } from 'vscode'; import { Architecture } from '../../../../client/common/utils/platform'; import { @@ -122,4 +122,32 @@ suite('Data Science - Jupyter Interpreter Service', () => { assert.equal(selectedInterpreter, secondPythonInterpreter); }); + test('setInitialInterpreter if older version is set should use and clear', async () => { + when(oldVersionCacheStateStore.getCachedInterpreterPath()).thenReturn(pythonInterpreter.path); + when(oldVersionCacheStateStore.clearCache()).thenResolve(); + when(interpreterConfiguration.areDependenciesInstalled(pythonInterpreter, anything())).thenResolve(true); + const initialInterpreter = await jupyterInterpreterService.setInitialInterpreter(undefined); + verify(oldVersionCacheStateStore.clearCache()).once(); + assert.equal(initialInterpreter, pythonInterpreter); + }); + test('setInitialInterpreter use saved interpreter if valid', async () => { + when(oldVersionCacheStateStore.getCachedInterpreterPath()).thenReturn(undefined); + when(interpreterSelectionState.selectedPythonPath).thenReturn(pythonInterpreter.path); + when(interpreterConfiguration.areDependenciesInstalled(pythonInterpreter, anything())).thenResolve(true); + const initialInterpreter = await jupyterInterpreterService.setInitialInterpreter(undefined); + assert.equal(initialInterpreter, pythonInterpreter); + }); + test('setInitialInterpreter saved interpreter invalid, clear it and use active interpreter', async () => { + when(oldVersionCacheStateStore.getCachedInterpreterPath()).thenReturn(undefined); + when(interpreterSelectionState.selectedPythonPath).thenReturn(secondPythonInterpreter.path); + when(interpreterConfiguration.areDependenciesInstalled(secondPythonInterpreter, anything())).thenResolve(false); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(pythonInterpreter); + when(interpreterConfiguration.areDependenciesInstalled(pythonInterpreter, anything())).thenResolve(true); + const initialInterpreter = await jupyterInterpreterService.setInitialInterpreter(undefined); + assert.equal(initialInterpreter, pythonInterpreter); + // Make sure we set our saved interpreter to the new active interpreter + // it should have been cleared to undefined, then set to a new value + verify(interpreterSelectionState.updateSelectedPythonPath(undefined)).once(); + verify(interpreterSelectionState.updateSelectedPythonPath(anyString())).once(); + }); });