From 357940fd5dd138b35a347c3d0ac23ec71ea7e6f1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 08:26:35 -0700 Subject: [PATCH 01/14] Prevent unnecessary activation of the Python extension --- news/2 Fixes/5193.md | 1 + src/client/api/pythonApi.ts | 48 +++++++++++++++++++++++-------------- src/client/api/types.ts | 2 ++ 3 files changed, 33 insertions(+), 18 deletions(-) create mode 100644 news/2 Fixes/5193.md diff --git a/news/2 Fixes/5193.md b/news/2 Fixes/5193.md new file mode 100644 index 00000000000..2b0e1c89587 --- /dev/null +++ b/news/2 Fixes/5193.md @@ -0,0 +1 @@ +Prevent unnecessary activation of the Python extension. diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index 4493483c4bf..3fea581dc2e 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -49,6 +49,10 @@ import { @injectable() export class PythonApiProvider implements IPythonApiProvider { private readonly api = createDeferred(); + private readonly didActivePython = new EventEmitter(); + public get onDidActivePythonExtension() { + return this.didActivePython.event; + } private initialized?: boolean; @@ -80,6 +84,7 @@ export class PythonApiProvider implements IPythonApiProvider { } else { if (!pythonExtension.isActive) { await pythonExtension.activate(); + this.didActivePython.fire(); } pythonExtension.exports.jupyter.registerHooks(); } @@ -106,6 +111,9 @@ export class PythonExtensionChecker implements IPythonExtensionChecker { public get isPythonExtensionInstalled() { return this.extensions.getExtension(this.pythonExtensionId) !== undefined; } + public get isPythonExtensionActive() { + return this.extensions.getExtension(this.pythonExtensionId)?.isActive === true; + } public async showPythonExtensionInstallRequiredPrompt(): Promise { if (this.waitingOnInstallPrompt) { @@ -292,24 +300,17 @@ export class InterpreterService implements IInterpreterService { ) {} public get onDidChangeInterpreter(): Event { - if (this.extensionChecker.isPythonExtensionInstalled && !this.eventHandlerAdded) { - this.apiProvider - .getApi() - .then((api) => { - if (!this.eventHandlerAdded) { - this.eventHandlerAdded = true; - api.onDidChangeInterpreter( - () => { - // Clear our cache of active interpreters. - this.workspaceCachedActiveInterpreter.clear(); - this.didChangeInterpreter.fire(); - }, - this, - this.disposables - ); - } - }) - .catch(noop); + if (this.extensionChecker.isPythonExtensionInstalled) { + if (this.extensionChecker.isPythonExtensionActive && this.eventHandlerAdded) { + this.hookupOnDidChangeInterpreterEvent(); + } + if (!this.extensionChecker.isPythonExtensionActive) { + this.apiProvider.onDidActivePythonExtension( + this.hookupOnDidChangeInterpreterEvent, + this, + this.disposables + ); + } } return this.didChangeInterpreter.event; } @@ -345,4 +346,15 @@ export class InterpreterService implements IInterpreterService { return undefined; } } + private hookupOnDidChangeInterpreterEvent() { + this.apiProvider + .getApi() + .then((api) => { + if (!this.eventHandlerAdded) { + this.eventHandlerAdded = true; + api.onDidChangeInterpreter(() => this.didChangeInterpreter.fire(), this, this.disposables); + } + }) + .catch(noop); + } } diff --git a/src/client/api/types.ts b/src/client/api/types.ts index b9f5c098bf1..cf9146a1099 100644 --- a/src/client/api/types.ts +++ b/src/client/api/types.ts @@ -19,12 +19,14 @@ export interface ILanguageServer extends Disposable { export const IPythonApiProvider = Symbol('IPythonApi'); export interface IPythonApiProvider { + onDidActivePythonExtension: Event; getApi(): Promise; setApi(api: PythonApi): void; } export const IPythonExtensionChecker = Symbol('IPythonExtensionChecker'); export interface IPythonExtensionChecker { readonly isPythonExtensionInstalled: boolean; + readonly isPythonExtensionActive: boolean; showPythonExtensionInstallRequiredPrompt(): Promise; showPythonExtensionInstallRecommendedPrompt(): Promise; } From 5ffce0bead95027f39a97810bcd591ab13568f35 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 10:17:09 -0700 Subject: [PATCH 02/14] More changes --- src/client/activation/activationManager.ts | 2 +- src/client/api/pythonApi.ts | 27 ++++++++++--- src/client/datascience/activation.ts | 7 ++-- .../jupyterInterpreterStateStore.ts | 24 +++++++---- .../kernel-launcher/kernelDaemonPreWarmer.ts | 29 ++++++++++---- .../kernel-launcher/localKernelFinder.ts | 4 +- .../notebookStorage/nativeEditorStorage.ts | 2 +- src/client/datascience/preWarmVariables.ts | 6 ++- .../telemetry/interpreterCountTracker.ts | 16 ++++---- .../telemetry/interpreterPackageTracker.ts | 10 +++-- .../telemetry/interpreterPackages.ts | 16 ++++++-- .../telemetry/workspaceInterpreterTracker.ts | 2 +- .../interpreter/display/visibilityFilter.ts | 40 +++++++++---------- src/test/datascience/activation.unit.test.ts | 5 ++- .../datascience/dataScienceIocContainer.ts | 2 +- .../datascience/preWarmVariables.unit.test.ts | 4 ++ 16 files changed, 130 insertions(+), 66 deletions(-) diff --git a/src/client/activation/activationManager.ts b/src/client/activation/activationManager.ts index dfc09a4bedd..e0ac7b741c0 100644 --- a/src/client/activation/activationManager.ts +++ b/src/client/activation/activationManager.ts @@ -73,7 +73,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { this.activatedWorkspaces.add(key); // Get latest interpreter list in the background. - if (this.extensionChecker.isPythonExtensionInstalled) { + if (this.extensionChecker.isPythonExtensionActive) { this.interpreterService.getInterpreters(resource).ignoreErrors(); } diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index 3fea581dc2e..cf9bd238eed 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -55,11 +55,15 @@ export class PythonApiProvider implements IPythonApiProvider { } private initialized?: boolean; + private hooksRegistered?: boolean; constructor( @inject(IExtensions) private readonly extensions: IExtensions, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker - ) {} + ) { + this.extensions.onDidChange(this.registerHooks, this, this.disposables); + } public getApi(): Promise { this.init().catch(noop); @@ -82,12 +86,23 @@ export class PythonApiProvider implements IPythonApiProvider { if (!pythonExtension) { await this.extensionChecker.showPythonExtensionInstallRequiredPrompt(); } else { - if (!pythonExtension.isActive) { - await pythonExtension.activate(); - this.didActivePython.fire(); - } - pythonExtension.exports.jupyter.registerHooks(); + await this.registerHooks(); + } + } + private async registerHooks() { + if (this.hooksRegistered) { + return; + } + this.hooksRegistered = true; + const pythonExtension = this.extensions.getExtension<{ jupyter: { registerHooks(): void } }>(PythonExtension); + if (!pythonExtension) { + return; + } + if (!pythonExtension.isActive) { + await pythonExtension.activate(); + this.didActivePython.fire(); } + pythonExtension.exports.jupyter.registerHooks(); } } diff --git a/src/client/datascience/activation.ts b/src/client/datascience/activation.ts index a889b849d3d..c89c100d0e4 100644 --- a/src/client/datascience/activation.ts +++ b/src/client/datascience/activation.ts @@ -15,7 +15,7 @@ import { JupyterDaemonModule, Telemetry } from './constants'; import { ActiveEditorContextService } from './commands/activeEditorContext'; import { JupyterInterpreterService } from './jupyter/interpreter/jupyterInterpreterService'; import { KernelDaemonPreWarmer } from './kernel-launcher/kernelDaemonPreWarmer'; -import { INotebookCreationTracker, INotebookEditorProvider } from './types'; +import { INotebookCreationTracker, INotebookEditorProvider, IRawNotebookSupportedService } from './types'; @injectable() export class Activation implements IExtensionSingleActivationService { @@ -27,6 +27,7 @@ export class Activation implements IExtensionSingleActivationService { @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(ActiveEditorContextService) private readonly contextService: ActiveEditorContextService, @inject(KernelDaemonPreWarmer) private readonly daemonPoolPrewarmer: KernelDaemonPreWarmer, + @inject(IRawNotebookSupportedService) private readonly rawSupported: IRawNotebookSupportedService, @inject(INotebookCreationTracker) private readonly tracker: INotebookCreationTracker, @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker @@ -44,7 +45,7 @@ export class Activation implements IExtensionSingleActivationService { this.PreWarmDaemonPool().ignoreErrors(); sendTelemetryEvent(Telemetry.OpenNotebookAll); - if (this.extensionChecker.isPythonExtensionInstalled) { + if (!this.rawSupported.supported() && this.extensionChecker.isPythonExtensionInstalled) { // Warm up our selected interpreter for the extension this.jupyterInterpreterService.setInitialInterpreter().ignoreErrors(); } @@ -61,7 +62,7 @@ export class Activation implements IExtensionSingleActivationService { @debounceAsync(500) @swallowExceptions('Failed to pre-warm daemon pool') private async PreWarmDaemonPool() { - if (!this.extensionChecker.isPythonExtensionInstalled) { + if (!this.extensionChecker.isPythonExtensionActive) { // Skip prewarm if no python extension return; } diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts index fa5531404c4..282b1c26979 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts @@ -7,7 +7,7 @@ import { inject, injectable, named } from 'inversify'; import { Memento } from 'vscode'; import { IExtensionSingleActivationService } from '../../../activation/types'; import { IPythonApiProvider, IPythonExtensionChecker } from '../../../api/types'; -import { GLOBAL_MEMENTO, IMemento } from '../../../common/types'; +import { GLOBAL_MEMENTO, IDisposableRegistry, IMemento } from '../../../common/types'; import { noop } from '../../../common/utils/misc'; const key = 'INTERPRETER_PATH_SELECTED_FOR_JUPYTER_SERVER'; @@ -45,25 +45,35 @@ export class JupyterInterpreterStateStore { @injectable() export class MigrateJupyterInterpreterStateService implements IExtensionSingleActivationService { + private settingsMigrated?: boolean; constructor( @inject(IPythonApiProvider) private readonly api: IPythonApiProvider, @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly memento: Memento, - @inject(IPythonExtensionChecker) private readonly checker: IPythonExtensionChecker + @inject(IPythonExtensionChecker) private readonly checker: IPythonExtensionChecker, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry ) {} // Migrate the interpreter path selected for Jupyter server from the Python extension's globalState memento public async activate() { this.activateBackground().catch(noop); + this.api.onDidActivePythonExtension(this.activateBackground, this, this.disposables); } public async activateBackground() { // Migrate in the background. // Python extension will not activate unless Jupyter activates, and here we're waiting for Python. // Hence end in deadlock (caught in smoke test). - if (!this.memento.get(key) && this.checker.isPythonExtensionInstalled) { - const api = await this.api.getApi(); - const data = api.getInterpreterPathSelectedForJupyterServer(); - await this.memento.update(key, data); - await this.memento.update(keySelected, true); + if (!this.memento.get(key) && this.checker.isPythonExtensionActive) { + await this.migrateSettings(); } } + private async migrateSettings() { + if (this.settingsMigrated) { + return; + } + this.settingsMigrated = true; + const api = await this.api.getApi(); + const data = api.getInterpreterPathSelectedForJupyterServer(); + await this.memento.update(key, data); + await this.memento.update(keySelected, true); + } } diff --git a/src/client/datascience/kernel-launcher/kernelDaemonPreWarmer.ts b/src/client/datascience/kernel-launcher/kernelDaemonPreWarmer.ts index 79fb0562fc4..13b559ec380 100644 --- a/src/client/datascience/kernel-launcher/kernelDaemonPreWarmer.ts +++ b/src/client/datascience/kernel-launcher/kernelDaemonPreWarmer.ts @@ -11,7 +11,9 @@ import { PYTHON_LANGUAGE } from '../../common/constants'; import '../../common/extensions'; import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; import { swallowExceptions } from '../../common/utils/decorators'; -import { isJupyterKernel } from '../notebook/helpers/helpers'; +import { isUntitledFile } from '../../common/utils/misc'; +import { isPythonKernelConnection } from '../jupyter/kernels/helpers'; +import { getNotebookMetadata, isJupyterKernel, isPythonNotebook } from '../notebook/helpers/helpers'; import { IInteractiveWindowProvider, INotebookCreationTracker, @@ -40,7 +42,7 @@ export class KernelDaemonPreWarmer { // If not, don't bother with prewarming // Also respect the disable autostart setting to not do any prewarming for the user if ( - !(await this.rawNotebookSupported.supported()) || + !this.rawNotebookSupported.supported() || this.configService.getSettings().disableJupyterAutoStart || !this.extensionChecker.isPythonExtensionInstalled ) { @@ -54,14 +56,20 @@ export class KernelDaemonPreWarmer { this.disposables.push(this.vscodeNotebook.onDidOpenNotebookDocument(this.onDidOpenNotebookDocument, this)); - if (this.notebookEditorProvider.editors.length > 0 || this.interactiveProvider.windows.length > 0) { + if ( + this.extensionChecker.isPythonExtensionActive && + (this.notebookEditorProvider.editors.length > 0 || this.interactiveProvider.windows.length > 0) + ) { await this.preWarmKernelDaemonPool(); } - await this.preWarmDaemonPoolIfNecesary(); + await this.preWarmDaemonPoolIfNecessary(); } - private async preWarmDaemonPoolIfNecesary() { + private async preWarmDaemonPoolIfNecessary() { // This is only for python, so prewarm just if we've seen python recently in this workspace - if (this.shouldPreWarmDaemonPool(this.usageTracker.lastPythonNotebookCreated)) { + if ( + this.shouldPreWarmDaemonPool(this.usageTracker.lastPythonNotebookCreated) && + this.extensionChecker.isPythonExtensionActive + ) { await this.preWarmKernelDaemonPool(); } } @@ -79,9 +87,16 @@ export class KernelDaemonPreWarmer { // Handle opening of native documents private async onDidOpenNotebookDocument(doc: NotebookDocument): Promise { + // It could be anything, lets not make any assumptions. + if (isUntitledFile(doc.uri)) { + return; + } const kernel = this.vscodeNotebook.notebookEditors.find((item) => item.document === doc)?.kernel; + const isPythonKernel = isJupyterKernel(kernel) ? isPythonKernelConnection(kernel.selection) : false; + const notebookMetadata = isPythonNotebook(getNotebookMetadata(doc)); if ( - isJupyterKernel(kernel) || + isPythonKernel || + notebookMetadata || doc.cells.some((cell: NotebookCell) => { return cell.document.languageId === PYTHON_LANGUAGE; }) diff --git a/src/client/datascience/kernel-launcher/localKernelFinder.ts b/src/client/datascience/kernel-launcher/localKernelFinder.ts index 3efbe4dcde8..b132b5256cc 100644 --- a/src/client/datascience/kernel-launcher/localKernelFinder.ts +++ b/src/client/datascience/kernel-launcher/localKernelFinder.ts @@ -33,6 +33,7 @@ import { import { IJupyterKernelSpec } from '../types'; import { ILocalKernelFinder } from './types'; import { tryGetRealPath } from '../common'; +import { isPythonNotebook } from '../notebook/helpers/helpers'; const winJupyterPath = path.join('AppData', 'Roaming', 'jupyter', 'kernels'); const linuxJupyterPath = path.join('.local', 'share', 'jupyter', 'kernels'); @@ -90,12 +91,13 @@ export class LocalKernelFinder implements ILocalKernelFinder { try { // Get list of all of the specs const kernels = await this.listKernels(resource, cancelToken); + const isPythonNb = isPythonNotebook(option); // Always include the interpreter in the search if we can const interpreter = option && isInterpreter(option) ? option - : resource && this.extensionChecker.isPythonExtensionInstalled + : resource && isPythonNb && this.extensionChecker.isPythonExtensionInstalled ? await this.interpreterService.getActiveInterpreter(resource) : undefined; diff --git a/src/client/datascience/notebookStorage/nativeEditorStorage.ts b/src/client/datascience/notebookStorage/nativeEditorStorage.ts index 9ddb8973341..8e90c143874 100644 --- a/src/client/datascience/notebookStorage/nativeEditorStorage.ts +++ b/src/client/datascience/notebookStorage/nativeEditorStorage.ts @@ -221,7 +221,7 @@ export class NativeEditorStorage implements INotebookStorage { return (notebookData.metadata.language_info.codemirror_mode as any).version; } // Use the active interpreter if allowed - if (this.extensionChecker.isPythonExtensionInstalled) { + if (this.extensionChecker.isPythonExtensionActive) { const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython(); return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3; } diff --git a/src/client/datascience/preWarmVariables.ts b/src/client/datascience/preWarmVariables.ts index 36731ba7138..b7d5da48ea7 100644 --- a/src/client/datascience/preWarmVariables.ts +++ b/src/client/datascience/preWarmVariables.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { IExtensionSingleActivationService } from '../activation/types'; -import { IPythonExtensionChecker } from '../api/types'; +import { IPythonApiProvider, IPythonExtensionChecker } from '../api/types'; import '../common/extensions'; import { IDisposableRegistry } from '../common/types'; import { noop } from '../common/utils/misc'; @@ -20,6 +20,7 @@ export class PreWarmActivatedJupyterEnvironmentVariables implements IExtensionSi @inject(JupyterInterpreterService) private readonly jupyterInterpreterService: JupyterInterpreterService, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker, + @inject(IPythonApiProvider) private readonly apiProvider: IPythonApiProvider, @inject(IRawNotebookSupportedService) private readonly rawNotebookSupported: IRawNotebookSupportedService ) {} public async activate(): Promise { @@ -32,10 +33,11 @@ export class PreWarmActivatedJupyterEnvironmentVariables implements IExtensionSi ); this.preWarmInterpreterVariables().ignoreErrors(); } + this.apiProvider.onDidActivePythonExtension(this.preWarmInterpreterVariables, this, this.disposables); } private async preWarmInterpreterVariables() { - if (!this.extensionChecker.isPythonExtensionInstalled) { + if (!this.extensionChecker.isPythonExtensionActive) { return; } const interpreter = await this.jupyterInterpreterService.getSelectedInterpreter(); diff --git a/src/client/datascience/telemetry/interpreterCountTracker.ts b/src/client/datascience/telemetry/interpreterCountTracker.ts index fe79ea535e4..c80f45278fd 100644 --- a/src/client/datascience/telemetry/interpreterCountTracker.ts +++ b/src/client/datascience/telemetry/interpreterCountTracker.ts @@ -4,9 +4,9 @@ import { IExtensionSingleActivationService } from '../../activation/types'; import { inject, injectable } from 'inversify'; import { IInterpreterService } from '../../interpreter/contracts'; -import { IPythonExtensionChecker } from '../../api/types'; +import { IPythonApiProvider, IPythonExtensionChecker } from '../../api/types'; import { noop } from '../../common/utils/misc'; -import { IDisposableRegistry, IExtensions } from '../../common/types'; +import { IDisposableRegistry } from '../../common/types'; @injectable() export class InterpreterCountTracker implements IExtensionSingleActivationService { @@ -16,23 +16,23 @@ export class InterpreterCountTracker implements IExtensionSingleActivationServic return InterpreterCountTracker.interpreterCount; } constructor( - @inject(IExtensions) private readonly extensions: IExtensions, @inject(IPythonExtensionChecker) private readonly pythonExtensionChecker: IPythonExtensionChecker, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(IPythonApiProvider) private pythonApi: IPythonApiProvider, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService ) {} public async activate() { - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { - this.extensions.onDidChange(this.trackInterpreters, this, this.disposables); - return; + if (this.pythonExtensionChecker.isPythonExtensionActive) { + this.trackInterpreters(); + } else { + this.pythonApi.onDidActivePythonExtension(this.trackInterpreters, this, this.disposables); } - this.trackInterpreters(); } private trackInterpreters() { if (this.interpretersTracked) { return; } - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { + if (!this.pythonExtensionChecker.isPythonExtensionActive) { return; } this.interpretersTracked = true; diff --git a/src/client/datascience/telemetry/interpreterPackageTracker.ts b/src/client/datascience/telemetry/interpreterPackageTracker.ts index f446b4ee17d..eb811efdff7 100644 --- a/src/client/datascience/telemetry/interpreterPackageTracker.ts +++ b/src/client/datascience/telemetry/interpreterPackageTracker.ts @@ -4,7 +4,7 @@ import { inject, injectable } from 'inversify'; import { NotebookKernel as VSCNotebookKernel } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; -import { IPythonInstaller, IPythonExtensionChecker } from '../../api/types'; +import { IPythonInstaller, IPythonExtensionChecker, IPythonApiProvider } from '../../api/types'; import { IVSCodeNotebook } from '../../common/application/types'; import { InterpreterUri } from '../../common/installer/types'; import { IExtensions, IDisposableRegistry, Product, IConfigurationService } from '../../common/types'; @@ -25,6 +25,7 @@ export class InterpreterPackageTracker implements IExtensionSingleActivationServ @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook, + @inject(IPythonApiProvider) private readonly apiProvider: IPythonApiProvider, @inject(IConfigurationService) private readonly configurationService: IConfigurationService ) {} public async activate(): Promise { @@ -36,6 +37,7 @@ export class InterpreterPackageTracker implements IExtensionSingleActivationServ this.installer.onInstalled(this.onDidInstallPackage, this, this.disposables); this.extensions.onDidChange(this.trackUponActivation, this, this.disposables); this.trackUponActivation().catch(noop); + this.apiProvider.onDidActivePythonExtension(this.trackUponActivation, this, this.disposables); } private async onDidChangeActiveNotebookKernel({ kernel }: { kernel: VSCNotebookKernel | undefined }) { if (!kernel || !isJupyterKernel(kernel) || !kernel.selection.interpreter) { @@ -47,14 +49,14 @@ export class InterpreterPackageTracker implements IExtensionSingleActivationServ if (this.activeInterpreterTrackedUponActivation) { return; } - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { + if (!this.pythonExtensionChecker.isPythonExtensionActive) { return; } this.activeInterpreterTrackedUponActivation = true; await this.trackPackagesOfActiveInterpreter(); } private async trackPackagesOfActiveInterpreter() { - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { + if (!this.pythonExtensionChecker.isPythonExtensionActive) { return; } // Get details of active interpreter. @@ -65,7 +67,7 @@ export class InterpreterPackageTracker implements IExtensionSingleActivationServ await this.packages.trackPackages(activeInterpreter); } private async onDidInstallPackage(args: { product: Product; resource?: InterpreterUri }) { - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { + if (!this.pythonExtensionChecker.isPythonExtensionActive) { return; } if (isResource(args.resource)) { diff --git a/src/client/datascience/telemetry/interpreterPackages.ts b/src/client/datascience/telemetry/interpreterPackages.ts index 7d8a68a09b0..2800a22cf78 100644 --- a/src/client/datascience/telemetry/interpreterPackages.ts +++ b/src/client/datascience/telemetry/interpreterPackages.ts @@ -2,9 +2,10 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { IPythonExtensionChecker } from '../../api/types'; +import { IPythonApiProvider, IPythonExtensionChecker } from '../../api/types'; import { InterpreterUri } from '../../common/installer/types'; import { IPythonExecutionFactory } from '../../common/process/types'; +import { IDisposableRegistry } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import { isResource, noop } from '../../common/utils/misc'; import { IInterpreterService } from '../../interpreter/contracts'; @@ -32,13 +33,21 @@ const interestedPackages = new Set( export class InterpreterPackages { private static interpreterInformation = new Map>>(); private static pendingInterpreterInformation = new Map>(); + private pendingInterpreterBeforeActivation = new Set(); private static instance?: InterpreterPackages; constructor( @inject(IPythonExtensionChecker) private readonly pythonExtensionChecker: IPythonExtensionChecker, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, - @inject(IPythonExecutionFactory) private readonly executionFactory: IPythonExecutionFactory + @inject(IPythonExecutionFactory) private readonly executionFactory: IPythonExecutionFactory, + @inject(IPythonApiProvider) private readonly apiProvider: IPythonApiProvider, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry ) { InterpreterPackages.instance = this; + this.apiProvider.onDidActivePythonExtension( + () => this.pendingInterpreterBeforeActivation.forEach((item) => this.trackPackages(item)), + this, + this.disposables + ); } public static getPackageVersions(interpreter: PythonEnvironment): Promise> { let deferred = InterpreterPackages.interpreterInformation.get(interpreter.path); @@ -56,7 +65,8 @@ export class InterpreterPackages { this.trackPackagesInternal(interpreterUri, ignoreCache).catch(noop); } public async trackPackagesInternal(interpreterUri: InterpreterUri, ignoreCache?: boolean) { - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { + if (!this.pythonExtensionChecker.isPythonExtensionActive) { + this.pendingInterpreterBeforeActivation.add(interpreterUri); return; } let interpreter: PythonEnvironment; diff --git a/src/client/datascience/telemetry/workspaceInterpreterTracker.ts b/src/client/datascience/telemetry/workspaceInterpreterTracker.ts index 6b1117a294e..8cda4f24c02 100644 --- a/src/client/datascience/telemetry/workspaceInterpreterTracker.ts +++ b/src/client/datascience/telemetry/workspaceInterpreterTracker.ts @@ -42,7 +42,7 @@ export class WorkspaceInterpreterTracker implements IExtensionSyncActivationServ return activeInterpreterPath === interpreter.path; } private trackActiveInterpreters() { - if (this.trackingInterpreters || !this.pythonExtensionChecker.isPythonExtensionInstalled) { + if (this.trackingInterpreters || !this.pythonExtensionChecker.isPythonExtensionActive) { return; } this.trackingInterpreters = true; diff --git a/src/client/interpreter/display/visibilityFilter.ts b/src/client/interpreter/display/visibilityFilter.ts index 37af1db8232..cd708f270cc 100644 --- a/src/client/interpreter/display/visibilityFilter.ts +++ b/src/client/interpreter/display/visibilityFilter.ts @@ -6,7 +6,7 @@ import { Event, EventEmitter } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; import { IInterpreterStatusbarVisibilityFilter, IPythonApiProvider, IPythonExtensionChecker } from '../../api/types'; import { IVSCodeNotebook } from '../../common/application/types'; -import { IDisposableRegistry, IExtensions } from '../../common/types'; +import { IDisposableRegistry } from '../../common/types'; import { isJupyterNotebook } from '../../datascience/notebook/helpers/helpers'; @injectable() @@ -17,10 +17,9 @@ export class InterpreterStatusBarVisibility constructor( @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook, - @inject(IDisposableRegistry) disposables: IDisposableRegistry, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker, - @inject(IPythonApiProvider) private pythonApi: IPythonApiProvider, - @inject(IExtensions) readonly extensions: IExtensions + @inject(IPythonApiProvider) private pythonApi: IPythonApiProvider ) { vscNotebook.onDidChangeActiveNotebookEditor( () => { @@ -29,21 +28,13 @@ export class InterpreterStatusBarVisibility this, disposables ); - extensions.onDidChange(this.extensionsChanged, this, disposables); } public async activate(): Promise { // Tell the python extension about our filter - if (this.extensionChecker.isPythonExtensionInstalled) { - this._registered = true; - this.pythonApi - .getApi() - .then((a) => { - // Python API may not have the register function yet. - if (a.registerInterpreterStatusFilter) { - a.registerInterpreterStatusFilter(this); - } - }) - .ignoreErrors(); + if (this.extensionChecker.isPythonExtensionActive) { + this.registerStatusFilter(); + } else { + this.pythonApi.onDidActivePythonExtension(this.registerStatusFilter, this, this.disposables); } } public get changed(): Event { @@ -55,10 +46,19 @@ export class InterpreterStatusBarVisibility ? true : false; } - private extensionsChanged() { - // See if the python extension was suddenly registered - if (this.extensionChecker.isPythonExtensionInstalled && this._registered) { - this.activate().ignoreErrors(); + private registerStatusFilter() { + if (this._registered) { + return; } + this._registered = true; + this.pythonApi + .getApi() + .then((a) => { + // Python API may not have the register function yet. + if (a.registerInterpreterStatusFilter) { + a.registerInterpreterStatusFilter(this); + } + }) + .ignoreErrors(); } } diff --git a/src/test/datascience/activation.unit.test.ts b/src/test/datascience/activation.unit.test.ts index b5b4276871d..4c214f9d542 100644 --- a/src/test/datascience/activation.unit.test.ts +++ b/src/test/datascience/activation.unit.test.ts @@ -17,7 +17,7 @@ import { NativeEditor } from '../../client/datascience/interactive-ipynb/nativeE import { JupyterInterpreterService } from '../../client/datascience/jupyter/interpreter/jupyterInterpreterService'; import { KernelDaemonPreWarmer } from '../../client/datascience/kernel-launcher/kernelDaemonPreWarmer'; import { NativeEditorProvider } from '../../client/datascience/notebookStorage/nativeEditorProvider'; -import { INotebookCreationTracker, INotebookEditor, INotebookEditorProvider } from '../../client/datascience/types'; +import { INotebookCreationTracker, INotebookEditor, INotebookEditorProvider, IRawNotebookSupportedService } from '../../client/datascience/types'; import { PythonEnvironment } from '../../client/pythonEnvironments/info'; import { FakeClock } from '../common'; import { createPythonInterpreter } from '../utils/interpreters'; @@ -50,6 +50,8 @@ suite('DataScience - Activation', () => { when(contextService.activate()).thenResolve(); when(daemonPool.activate(anything())).thenResolve(); const extensionChecker = mock(PythonExtensionChecker); + const rawNotebook = mock(); + when(rawNotebook.supported()).thenReturn(false); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); activator = new Activation( instance(notebookEditorProvider), @@ -58,6 +60,7 @@ suite('DataScience - Activation', () => { [], instance(contextService), instance(daemonPool), + instance(rawNotebook), instance(tracker), instance(extensionChecker) ); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index a8240b39e87..79fda465fc3 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -536,6 +536,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingletonInstance(IExperimentService, instance(experimentService)); const extensionChecker = mock(PythonExtensionChecker); when(extensionChecker.isPythonExtensionInstalled).thenCall(this.isPythonExtensionInstalled.bind(this)); + when(extensionChecker.isPythonExtensionActive).thenCall(this.isPythonExtensionInstalled.bind(this)); when(extensionChecker.showPythonExtensionInstallRequiredPrompt()).thenCall( this.installPythonExtension.bind(this) ); @@ -1175,7 +1176,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private isPythonExtensionInstalled() { return this.pythonExtensionState; } - private installPythonExtension() { this.attemptedPythonExtension = true; } diff --git a/src/test/datascience/preWarmVariables.unit.test.ts b/src/test/datascience/preWarmVariables.unit.test.ts index 973daad57ca..cc8cc9e9b97 100644 --- a/src/test/datascience/preWarmVariables.unit.test.ts +++ b/src/test/datascience/preWarmVariables.unit.test.ts @@ -7,6 +7,7 @@ import { anything, instance, mock, verify, when } from 'ts-mockito'; import { EventEmitter } from 'vscode'; import { IExtensionSingleActivationService } from '../../client/activation/types'; import { PythonExtensionChecker } from '../../client/api/pythonApi'; +import { IPythonApiProvider } from '../../client/api/types'; import { createDeferred } from '../../client/common/utils/async'; import { JupyterInterpreterService } from '../../client/datascience/jupyter/interpreter/jupyterInterpreterService'; import { PreWarmActivatedJupyterEnvironmentVariables } from '../../client/datascience/preWarmVariables'; @@ -34,6 +35,8 @@ suite('DataScience - PreWarm Env Vars', () => { jupyterInterpreter = mock(JupyterInterpreterService); when(jupyterInterpreter.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event); extensionChecker = mock(PythonExtensionChecker); + const apiProvider = mock(); + when(apiProvider.onDidActivePythonExtension).thenReturn(new EventEmitter().event); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); zmqSupported = mock(); when(zmqSupported.supported()).thenReturn(false); @@ -42,6 +45,7 @@ suite('DataScience - PreWarm Env Vars', () => { instance(jupyterInterpreter), [], instance(extensionChecker), + instance(apiProvider), instance(zmqSupported) ); }); From 2842edce5d85bff3bce74bbcf179b6c9ff136ca2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 18:54:43 -0700 Subject: [PATCH 03/14] oops --- .eslintrc.js | 1 - 1 file changed, 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index d517ae989c6..732d475941e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -912,7 +912,6 @@ module.exports = { 'src/client/datascience/notebookAndInteractiveTracker.ts', 'src/client/datascience/statusProvider.ts', 'src/client/datascience/jupyter/interpreter/jupyterInterpreterSelectionCommand.ts', - 'src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts', 'src/client/datascience/jupyter/interpreter/jupyterInterpreterOldCacheStateStore.ts', 'src/client/datascience/jupyter/interpreter/jupyterInterpreterSelector.ts', 'src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts', From 03e727c54a8118da2a783cabc291c3a01ec740f7 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 18:58:17 -0700 Subject: [PATCH 04/14] Fix tests --- src/test/datascience/activation.unit.test.ts | 1 + .../kernel-launcher/kernelDaemonPoolPreWarmer.unit.test.ts | 1 + src/test/datascience/preWarmVariables.unit.test.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/src/test/datascience/activation.unit.test.ts b/src/test/datascience/activation.unit.test.ts index 4c214f9d542..ec38eee8127 100644 --- a/src/test/datascience/activation.unit.test.ts +++ b/src/test/datascience/activation.unit.test.ts @@ -53,6 +53,7 @@ suite('DataScience - Activation', () => { const rawNotebook = mock(); when(rawNotebook.supported()).thenReturn(false); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); + when(extensionChecker.isPythonExtensionActive).thenReturn(true); activator = new Activation( instance(notebookEditorProvider), instance(jupyterInterpreterService), diff --git a/src/test/datascience/kernel-launcher/kernelDaemonPoolPreWarmer.unit.test.ts b/src/test/datascience/kernel-launcher/kernelDaemonPoolPreWarmer.unit.test.ts index f6a2006bb40..39ecfc20def 100644 --- a/src/test/datascience/kernel-launcher/kernelDaemonPoolPreWarmer.unit.test.ts +++ b/src/test/datascience/kernel-launcher/kernelDaemonPoolPreWarmer.unit.test.ts @@ -39,6 +39,7 @@ suite('DataScience - Kernel Daemon Pool PreWarmer', () => { when(experimentService.inExperiment(anything())).thenResolve(true); extensionChecker = mock(PythonExtensionChecker); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); + when(extensionChecker.isPythonExtensionActive).thenReturn(true); // Set up our config settings settings = mock(JupyterSettings); diff --git a/src/test/datascience/preWarmVariables.unit.test.ts b/src/test/datascience/preWarmVariables.unit.test.ts index cc8cc9e9b97..c63b3ee17f4 100644 --- a/src/test/datascience/preWarmVariables.unit.test.ts +++ b/src/test/datascience/preWarmVariables.unit.test.ts @@ -38,6 +38,7 @@ suite('DataScience - PreWarm Env Vars', () => { const apiProvider = mock(); when(apiProvider.onDidActivePythonExtension).thenReturn(new EventEmitter().event); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); + when(extensionChecker.isPythonExtensionActive).thenReturn(true); zmqSupported = mock(); when(zmqSupported.supported()).thenReturn(false); activationService = new PreWarmActivatedJupyterEnvironmentVariables( From 366b7f1cdd18da08210a7cc3b49483b1f8b28a9e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 19:01:26 -0700 Subject: [PATCH 05/14] Misc --- src/client/api/pythonApi.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index cf9bd238eed..765459be644 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -62,7 +62,18 @@ export class PythonApiProvider implements IPythonApiProvider { @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker ) { - this.extensions.onDidChange(this.registerHooks, this, this.disposables); + const previouslyInstalled = this.extensionChecker.isPythonExtensionInstalled; + if (!previouslyInstalled) { + this.extensions.onDidChange( + async () => { + if (this.extensionChecker.isPythonExtensionInstalled) { + await this.registerHooks(); + } + }, + this, + this.disposables + ); + } } public getApi(): Promise { @@ -93,11 +104,11 @@ export class PythonApiProvider implements IPythonApiProvider { if (this.hooksRegistered) { return; } - this.hooksRegistered = true; const pythonExtension = this.extensions.getExtension<{ jupyter: { registerHooks(): void } }>(PythonExtension); if (!pythonExtension) { return; } + this.hooksRegistered = true; if (!pythonExtension.isActive) { await pythonExtension.activate(); this.didActivePython.fire(); From 4b0ba93339a977747ef6e7b291863545acba1173 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 19:03:04 -0700 Subject: [PATCH 06/14] Misc --- src/client/api/pythonApi.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index 765459be644..4a82465d912 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -373,6 +373,9 @@ export class InterpreterService implements IInterpreterService { } } private hookupOnDidChangeInterpreterEvent() { + if (this.eventHandlerAdded) { + return; + } this.apiProvider .getApi() .then((api) => { From fbda68bda18fc8b31fc7555c4c7cf084916cf301 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 19:09:58 -0700 Subject: [PATCH 07/14] oops --- src/client/api/pythonApi.ts | 10 +++++----- src/client/api/types.ts | 2 +- .../interpreter/jupyterInterpreterStateStore.ts | 2 +- src/client/datascience/preWarmVariables.ts | 2 +- .../datascience/telemetry/interpreterCountTracker.ts | 2 +- .../datascience/telemetry/interpreterPackageTracker.ts | 2 +- .../datascience/telemetry/interpreterPackages.ts | 2 +- src/client/interpreter/display/visibilityFilter.ts | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index 4a82465d912..44f4f2e95a0 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -49,9 +49,9 @@ import { @injectable() export class PythonApiProvider implements IPythonApiProvider { private readonly api = createDeferred(); - private readonly didActivePython = new EventEmitter(); - public get onDidActivePythonExtension() { - return this.didActivePython.event; + private readonly didActivatePython = new EventEmitter(); + public get onDidActivatePythonExtension() { + return this.didActivatePython.event; } private initialized?: boolean; @@ -111,7 +111,7 @@ export class PythonApiProvider implements IPythonApiProvider { this.hooksRegistered = true; if (!pythonExtension.isActive) { await pythonExtension.activate(); - this.didActivePython.fire(); + this.didActivatePython.fire(); } pythonExtension.exports.jupyter.registerHooks(); } @@ -331,7 +331,7 @@ export class InterpreterService implements IInterpreterService { this.hookupOnDidChangeInterpreterEvent(); } if (!this.extensionChecker.isPythonExtensionActive) { - this.apiProvider.onDidActivePythonExtension( + this.apiProvider.onDidActivatePythonExtension( this.hookupOnDidChangeInterpreterEvent, this, this.disposables diff --git a/src/client/api/types.ts b/src/client/api/types.ts index cf9146a1099..6eafab40f8f 100644 --- a/src/client/api/types.ts +++ b/src/client/api/types.ts @@ -19,7 +19,7 @@ export interface ILanguageServer extends Disposable { export const IPythonApiProvider = Symbol('IPythonApi'); export interface IPythonApiProvider { - onDidActivePythonExtension: Event; + onDidActivatePythonExtension: Event; getApi(): Promise; setApi(api: PythonApi): void; } diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts index 282b1c26979..6a88867d7af 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterStateStore.ts @@ -56,7 +56,7 @@ export class MigrateJupyterInterpreterStateService implements IExtensionSingleAc // Migrate the interpreter path selected for Jupyter server from the Python extension's globalState memento public async activate() { this.activateBackground().catch(noop); - this.api.onDidActivePythonExtension(this.activateBackground, this, this.disposables); + this.api.onDidActivatePythonExtension(this.activateBackground, this, this.disposables); } public async activateBackground() { // Migrate in the background. diff --git a/src/client/datascience/preWarmVariables.ts b/src/client/datascience/preWarmVariables.ts index b7d5da48ea7..25f6608deeb 100644 --- a/src/client/datascience/preWarmVariables.ts +++ b/src/client/datascience/preWarmVariables.ts @@ -33,7 +33,7 @@ export class PreWarmActivatedJupyterEnvironmentVariables implements IExtensionSi ); this.preWarmInterpreterVariables().ignoreErrors(); } - this.apiProvider.onDidActivePythonExtension(this.preWarmInterpreterVariables, this, this.disposables); + this.apiProvider.onDidActivatePythonExtension(this.preWarmInterpreterVariables, this, this.disposables); } private async preWarmInterpreterVariables() { diff --git a/src/client/datascience/telemetry/interpreterCountTracker.ts b/src/client/datascience/telemetry/interpreterCountTracker.ts index c80f45278fd..bcd7e24adee 100644 --- a/src/client/datascience/telemetry/interpreterCountTracker.ts +++ b/src/client/datascience/telemetry/interpreterCountTracker.ts @@ -25,7 +25,7 @@ export class InterpreterCountTracker implements IExtensionSingleActivationServic if (this.pythonExtensionChecker.isPythonExtensionActive) { this.trackInterpreters(); } else { - this.pythonApi.onDidActivePythonExtension(this.trackInterpreters, this, this.disposables); + this.pythonApi.onDidActivatePythonExtension(this.trackInterpreters, this, this.disposables); } } private trackInterpreters() { diff --git a/src/client/datascience/telemetry/interpreterPackageTracker.ts b/src/client/datascience/telemetry/interpreterPackageTracker.ts index eb811efdff7..f6ee1dbbbf5 100644 --- a/src/client/datascience/telemetry/interpreterPackageTracker.ts +++ b/src/client/datascience/telemetry/interpreterPackageTracker.ts @@ -37,7 +37,7 @@ export class InterpreterPackageTracker implements IExtensionSingleActivationServ this.installer.onInstalled(this.onDidInstallPackage, this, this.disposables); this.extensions.onDidChange(this.trackUponActivation, this, this.disposables); this.trackUponActivation().catch(noop); - this.apiProvider.onDidActivePythonExtension(this.trackUponActivation, this, this.disposables); + this.apiProvider.onDidActivatePythonExtension(this.trackUponActivation, this, this.disposables); } private async onDidChangeActiveNotebookKernel({ kernel }: { kernel: VSCNotebookKernel | undefined }) { if (!kernel || !isJupyterKernel(kernel) || !kernel.selection.interpreter) { diff --git a/src/client/datascience/telemetry/interpreterPackages.ts b/src/client/datascience/telemetry/interpreterPackages.ts index 2800a22cf78..ad148eac086 100644 --- a/src/client/datascience/telemetry/interpreterPackages.ts +++ b/src/client/datascience/telemetry/interpreterPackages.ts @@ -43,7 +43,7 @@ export class InterpreterPackages { @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry ) { InterpreterPackages.instance = this; - this.apiProvider.onDidActivePythonExtension( + this.apiProvider.onDidActivatePythonExtension( () => this.pendingInterpreterBeforeActivation.forEach((item) => this.trackPackages(item)), this, this.disposables diff --git a/src/client/interpreter/display/visibilityFilter.ts b/src/client/interpreter/display/visibilityFilter.ts index cd708f270cc..de073a9cf62 100644 --- a/src/client/interpreter/display/visibilityFilter.ts +++ b/src/client/interpreter/display/visibilityFilter.ts @@ -34,7 +34,7 @@ export class InterpreterStatusBarVisibility if (this.extensionChecker.isPythonExtensionActive) { this.registerStatusFilter(); } else { - this.pythonApi.onDidActivePythonExtension(this.registerStatusFilter, this, this.disposables); + this.pythonApi.onDidActivatePythonExtension(this.registerStatusFilter, this, this.disposables); } } public get changed(): Event { From 4a3a46d8127b2cc361e76377d1170abea1ebcb18 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 Mar 2021 19:30:03 -0700 Subject: [PATCH 08/14] Misc --- src/test/datascience/preWarmVariables.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/datascience/preWarmVariables.unit.test.ts b/src/test/datascience/preWarmVariables.unit.test.ts index c63b3ee17f4..56e29cf8858 100644 --- a/src/test/datascience/preWarmVariables.unit.test.ts +++ b/src/test/datascience/preWarmVariables.unit.test.ts @@ -36,7 +36,7 @@ suite('DataScience - PreWarm Env Vars', () => { when(jupyterInterpreter.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event); extensionChecker = mock(PythonExtensionChecker); const apiProvider = mock(); - when(apiProvider.onDidActivePythonExtension).thenReturn(new EventEmitter().event); + when(apiProvider.onDidActivatePythonExtension).thenReturn(new EventEmitter().event); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); when(extensionChecker.isPythonExtensionActive).thenReturn(true); zmqSupported = mock(); From bf350bc2f3824d51e9584813e78aabb57e9c18c6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Mar 2021 08:56:08 -0700 Subject: [PATCH 09/14] Format --- src/test/datascience/activation.unit.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/datascience/activation.unit.test.ts b/src/test/datascience/activation.unit.test.ts index ec38eee8127..e22b4c6502f 100644 --- a/src/test/datascience/activation.unit.test.ts +++ b/src/test/datascience/activation.unit.test.ts @@ -17,7 +17,12 @@ import { NativeEditor } from '../../client/datascience/interactive-ipynb/nativeE import { JupyterInterpreterService } from '../../client/datascience/jupyter/interpreter/jupyterInterpreterService'; import { KernelDaemonPreWarmer } from '../../client/datascience/kernel-launcher/kernelDaemonPreWarmer'; import { NativeEditorProvider } from '../../client/datascience/notebookStorage/nativeEditorProvider'; -import { INotebookCreationTracker, INotebookEditor, INotebookEditorProvider, IRawNotebookSupportedService } from '../../client/datascience/types'; +import { + INotebookCreationTracker, + INotebookEditor, + INotebookEditorProvider, + IRawNotebookSupportedService +} from '../../client/datascience/types'; import { PythonEnvironment } from '../../client/pythonEnvironments/info'; import { FakeClock } from '../common'; import { createPythonInterpreter } from '../utils/interpreters'; From b605e1917b36e6ea487ea201322993b472959446 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Mar 2021 08:59:23 -0700 Subject: [PATCH 10/14] More fixes --- .../notebookStorage/nativeEditorStorage.ts | 15 +++------------ .../nativeEditorStorage.unit.test.ts | 4 +--- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/client/datascience/notebookStorage/nativeEditorStorage.ts b/src/client/datascience/notebookStorage/nativeEditorStorage.ts index 8e90c143874..9d0b4257d9f 100644 --- a/src/client/datascience/notebookStorage/nativeEditorStorage.ts +++ b/src/client/datascience/notebookStorage/nativeEditorStorage.ts @@ -6,7 +6,6 @@ import * as path from 'path'; import * as uuid from 'uuid/v4'; import { CancellationToken, Memento, Uri } from 'vscode'; import { createCodeCell } from '../../../datascience-ui/common/cellFactory'; -import { IPythonExtensionChecker } from '../../api/types'; import { traceError } from '../../common/logger'; import { isFileNotFoundError } from '../../common/platform/errors'; import { IFileSystem } from '../../common/platform/types'; @@ -18,7 +17,6 @@ import { InvalidNotebookFileError } from '../jupyter/invalidNotebookFileError'; import { INotebookModelFactory } from '../notebookStorage/types'; import { CellState, - IJupyterExecution, IModelLoadOptions, INotebookModel, INotebookStorage, @@ -57,15 +55,13 @@ export class NativeEditorStorage implements INotebookStorage { private backupRequested: { model: INotebookModel; cancellation: CancellationToken } | undefined; constructor( - @inject(IJupyterExecution) private jupyterExecution: IJupyterExecution, @inject(IFileSystem) private fs: IFileSystem, @inject(ICryptoUtils) private crypto: ICryptoUtils, @inject(IExtensionContext) private context: IExtensionContext, @inject(IMemento) @named(GLOBAL_MEMENTO) private globalStorage: Memento, @inject(IMemento) @named(WORKSPACE_MEMENTO) private localStorage: Memento, @inject(ITrustService) private trustService: ITrustService, - @inject(INotebookModelFactory) private readonly factory: INotebookModelFactory, - @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker + @inject(INotebookModelFactory) private readonly factory: INotebookModelFactory ) {} private static isUntitledFile(file: Uri) { return isUntitledFile(file); @@ -208,7 +204,7 @@ export class NativeEditorStorage implements INotebookStorage { traceError(`Error writing storage for ${filePath}: `, exc); } } - private async extractPythonMainVersion(notebookData: Partial): Promise { + private extractPythonMainVersion(notebookData: Partial): number { if ( notebookData && notebookData.metadata && @@ -220,11 +216,6 @@ export class NativeEditorStorage implements INotebookStorage { // eslint-disable-next-line @typescript-eslint/no-explicit-any return (notebookData.metadata.language_info.codemirror_mode as any).version; } - // Use the active interpreter if allowed - if (this.extensionChecker.isPythonExtensionActive) { - const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython(); - return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3; - } return 3; } @@ -368,7 +359,7 @@ export class NativeEditorStorage implements INotebookStorage { remapped.splice(0, 0, this.createEmptyCell(uuid())); } } - const pythonNumber = json ? await this.extractPythonMainVersion(json) : 3; + const pythonNumber = json ? this.extractPythonMainVersion(json) : 3; const model = this.factory.createModel( { diff --git a/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts b/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts index a482f2381c2..2a571c335d2 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts @@ -352,15 +352,13 @@ suite('DataScience - Native Editor Storage', () => { const cellLanguageService = mock(); when(cellLanguageService.getPreferredLanguage(anything())).thenReturn(PYTHON_LANGUAGE); const notebookStorage = new NativeEditorStorage( - instance(executionProvider), fileSystem.object, // Use typemoq so can save values in returns instance(crypto), context.object, globalMemento, localMemento, instance(trustService), - new NotebookModelFactory(false, instance(mockVSC), instance(cellLanguageService)), - instance(extensionChecker) + new NotebookModelFactory(false, instance(mockVSC), instance(cellLanguageService)) ); const container = mock(); when(container.tryGet(anything())).thenReturn(undefined); From 0430769883279d2bb0bd63b16ea237baa308d9af Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Mar 2021 09:52:03 -0700 Subject: [PATCH 11/14] Oops --- src/client/api/pythonApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index 44f4f2e95a0..332186fbe7b 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -327,7 +327,7 @@ export class InterpreterService implements IInterpreterService { public get onDidChangeInterpreter(): Event { if (this.extensionChecker.isPythonExtensionInstalled) { - if (this.extensionChecker.isPythonExtensionActive && this.eventHandlerAdded) { + if (this.extensionChecker.isPythonExtensionActive && !this.eventHandlerAdded) { this.hookupOnDidChangeInterpreterEvent(); } if (!this.extensionChecker.isPythonExtensionActive) { From e0ce172cdcb00959a80c74518d68c1a2a0badb66 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Mar 2021 09:54:41 -0700 Subject: [PATCH 12/14] Fixes --- src/client/api/pythonApi.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/api/pythonApi.ts b/src/client/api/pythonApi.ts index 332186fbe7b..0a8e0d470a4 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -74,6 +74,7 @@ export class PythonApiProvider implements IPythonApiProvider { this.disposables ); } + this.disposables.push(this.didActivatePython); } public getApi(): Promise { From ace02bbf665f06cad2350b9166e49a8931b1a1dd Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Mar 2021 10:08:50 -0700 Subject: [PATCH 13/14] Misc changes --- .../datascience/kernel-launcher/localKernelFinder.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/client/datascience/kernel-launcher/localKernelFinder.ts b/src/client/datascience/kernel-launcher/localKernelFinder.ts index b132b5256cc..f1667938059 100644 --- a/src/client/datascience/kernel-launcher/localKernelFinder.ts +++ b/src/client/datascience/kernel-launcher/localKernelFinder.ts @@ -32,7 +32,7 @@ import { } from '../jupyter/kernels/types'; import { IJupyterKernelSpec } from '../types'; import { ILocalKernelFinder } from './types'; -import { tryGetRealPath } from '../common'; +import { getResourceType, tryGetRealPath } from '../common'; import { isPythonNotebook } from '../notebook/helpers/helpers'; const winJupyterPath = path.join('AppData', 'Roaming', 'jupyter', 'kernels'); @@ -91,13 +91,14 @@ export class LocalKernelFinder implements ILocalKernelFinder { try { // Get list of all of the specs const kernels = await this.listKernels(resource, cancelToken); - const isPythonNb = isPythonNotebook(option); + const isPythonNbOrInteractiveWindow = + isPythonNotebook(option) || getResourceType(resource) === 'interactive'; // Always include the interpreter in the search if we can const interpreter = option && isInterpreter(option) ? option - : resource && isPythonNb && this.extensionChecker.isPythonExtensionInstalled + : resource && isPythonNbOrInteractiveWindow && this.extensionChecker.isPythonExtensionInstalled ? await this.interpreterService.getActiveInterpreter(resource) : undefined; From 555e1ea44859d689e13581515f9c8165d3fd4d36 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Mar 2021 10:23:51 -0700 Subject: [PATCH 14/14] Misc --- .../datascience/notebookStorage/nativeEditorStorage.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/client/datascience/notebookStorage/nativeEditorStorage.ts b/src/client/datascience/notebookStorage/nativeEditorStorage.ts index 9d0b4257d9f..27e4739b1e8 100644 --- a/src/client/datascience/notebookStorage/nativeEditorStorage.ts +++ b/src/client/datascience/notebookStorage/nativeEditorStorage.ts @@ -15,13 +15,7 @@ import { sendNotebookOrKernelLanguageTelemetry } from '../common'; import { Identifiers, Telemetry } from '../constants'; import { InvalidNotebookFileError } from '../jupyter/invalidNotebookFileError'; import { INotebookModelFactory } from '../notebookStorage/types'; -import { - CellState, - IModelLoadOptions, - INotebookModel, - INotebookStorage, - ITrustService -} from '../types'; +import { CellState, IModelLoadOptions, INotebookModel, INotebookStorage, ITrustService } from '../types'; import { NativeEditorNotebookModel } from './notebookModel'; import { VSCodeNotebookModel } from './vscNotebookModel';