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', 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/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 4493483c4bf..0a8e0d470a4 100644 --- a/src/client/api/pythonApi.ts +++ b/src/client/api/pythonApi.ts @@ -49,13 +49,33 @@ import { @injectable() export class PythonApiProvider implements IPythonApiProvider { private readonly api = createDeferred(); + private readonly didActivatePython = new EventEmitter(); + public get onDidActivatePythonExtension() { + return this.didActivatePython.event; + } private initialized?: boolean; + private hooksRegistered?: boolean; constructor( @inject(IExtensions) private readonly extensions: IExtensions, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker - ) {} + ) { + const previouslyInstalled = this.extensionChecker.isPythonExtensionInstalled; + if (!previouslyInstalled) { + this.extensions.onDidChange( + async () => { + if (this.extensionChecker.isPythonExtensionInstalled) { + await this.registerHooks(); + } + }, + this, + this.disposables + ); + } + this.disposables.push(this.didActivatePython); + } public getApi(): Promise { this.init().catch(noop); @@ -78,11 +98,23 @@ export class PythonApiProvider implements IPythonApiProvider { if (!pythonExtension) { await this.extensionChecker.showPythonExtensionInstallRequiredPrompt(); } else { - if (!pythonExtension.isActive) { - await pythonExtension.activate(); - } - pythonExtension.exports.jupyter.registerHooks(); + await this.registerHooks(); + } + } + private async registerHooks() { + if (this.hooksRegistered) { + return; + } + const pythonExtension = this.extensions.getExtension<{ jupyter: { registerHooks(): void } }>(PythonExtension); + if (!pythonExtension) { + return; } + this.hooksRegistered = true; + if (!pythonExtension.isActive) { + await pythonExtension.activate(); + this.didActivatePython.fire(); + } + pythonExtension.exports.jupyter.registerHooks(); } } @@ -106,6 +138,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 +327,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.onDidActivatePythonExtension( + this.hookupOnDidChangeInterpreterEvent, + this, + this.disposables + ); + } } return this.didChangeInterpreter.event; } @@ -345,4 +373,18 @@ export class InterpreterService implements IInterpreterService { return undefined; } } + private hookupOnDidChangeInterpreterEvent() { + if (this.eventHandlerAdded) { + return; + } + 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..6eafab40f8f 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 { + onDidActivatePythonExtension: 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; } 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..6a88867d7af 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.onDidActivatePythonExtension(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..f1667938059 100644 --- a/src/client/datascience/kernel-launcher/localKernelFinder.ts +++ b/src/client/datascience/kernel-launcher/localKernelFinder.ts @@ -32,7 +32,8 @@ 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'); const linuxJupyterPath = path.join('.local', 'share', 'jupyter', 'kernels'); @@ -90,12 +91,14 @@ export class LocalKernelFinder implements ILocalKernelFinder { try { // Get list of all of the specs const kernels = await this.listKernels(resource, cancelToken); + const isPythonNbOrInteractiveWindow = + isPythonNotebook(option) || getResourceType(resource) === 'interactive'; // Always include the interpreter in the search if we can const interpreter = option && isInterpreter(option) ? option - : resource && this.extensionChecker.isPythonExtensionInstalled + : resource && isPythonNbOrInteractiveWindow && 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..27e4739b1e8 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'; @@ -16,14 +15,7 @@ import { sendNotebookOrKernelLanguageTelemetry } from '../common'; import { Identifiers, Telemetry } from '../constants'; import { InvalidNotebookFileError } from '../jupyter/invalidNotebookFileError'; import { INotebookModelFactory } from '../notebookStorage/types'; -import { - CellState, - IJupyterExecution, - IModelLoadOptions, - INotebookModel, - INotebookStorage, - ITrustService -} from '../types'; +import { CellState, IModelLoadOptions, INotebookModel, INotebookStorage, ITrustService } from '../types'; import { NativeEditorNotebookModel } from './notebookModel'; import { VSCodeNotebookModel } from './vscNotebookModel'; @@ -57,15 +49,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 +198,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 +210,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.isPythonExtensionInstalled) { - const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython(); - return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3; - } return 3; } @@ -368,7 +353,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/client/datascience/preWarmVariables.ts b/src/client/datascience/preWarmVariables.ts index 36731ba7138..25f6608deeb 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.onDidActivatePythonExtension(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..bcd7e24adee 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.onDidActivatePythonExtension(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..f6ee1dbbbf5 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.onDidActivatePythonExtension(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..ad148eac086 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.onDidActivatePythonExtension( + () => 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..de073a9cf62 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.onDidActivatePythonExtension(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..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 } 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,7 +55,10 @@ 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); + when(extensionChecker.isPythonExtensionActive).thenReturn(true); activator = new Activation( instance(notebookEditorProvider), instance(jupyterInterpreterService), @@ -58,6 +66,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/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); 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 973daad57ca..56e29cf8858 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,7 +35,10 @@ suite('DataScience - PreWarm Env Vars', () => { jupyterInterpreter = mock(JupyterInterpreterService); when(jupyterInterpreter.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event); extensionChecker = mock(PythonExtensionChecker); + const apiProvider = mock(); + when(apiProvider.onDidActivatePythonExtension).thenReturn(new EventEmitter().event); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); + when(extensionChecker.isPythonExtensionActive).thenReturn(true); zmqSupported = mock(); when(zmqSupported.supported()).thenReturn(false); activationService = new PreWarmActivatedJupyterEnvironmentVariables( @@ -42,6 +46,7 @@ suite('DataScience - PreWarm Env Vars', () => { instance(jupyterInterpreter), [], instance(extensionChecker), + instance(apiProvider), instance(zmqSupported) ); });