From cdcd8571e082195736339ebdece744d3d5dd588b Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 13 Feb 2020 17:09:47 -0800 Subject: [PATCH 01/14] First idea --- src/client/datascience/constants.ts | 3 +- .../interactive-common/interactiveBase.ts | 10 ++++- .../interactive-ipynb/nativeEditor.ts | 5 +++ .../interactive-window/interactiveWindow.ts | 14 +++++++ .../datascience/jupyter/jupyterDebugger.ts | 8 ++-- .../datascience/jupyter/jupyterExecution.ts | 3 ++ .../datascience/jupyter/jupyterNotebook.ts | 18 ++++----- .../datascience/jupyter/jupyterServer.ts | 13 +++--- .../jupyter/jupyterServerWrapper.ts | 3 +- .../datascience/jupyter/jupyterVariables.ts | 4 +- .../jupyter/kernels/kernelSelections.ts | 21 ++++++---- .../jupyter/kernels/kernelSelector.ts | 40 +++++++++++++++---- .../datascience/jupyter/kernels/types.ts | 3 +- .../jupyter/liveshare/guestJupyterNotebook.ts | 4 +- .../jupyter/liveshare/guestJupyterServer.ts | 7 ++-- .../jupyter/liveshare/hostJupyterNotebook.ts | 6 +-- .../jupyter/liveshare/hostJupyterServer.ts | 33 +++++++++++---- src/client/datascience/types.ts | 5 ++- src/client/telemetry/index.ts | 6 +++ .../datascience/dataScienceIocContainer.ts | 24 +++++++++-- src/test/datascience/mockJupyterNotebook.ts | 2 +- 21 files changed, 170 insertions(+), 62 deletions(-) diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 4ae92a96395e..56a7de18bd79 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -267,7 +267,8 @@ export enum Telemetry { KernelEnumeration = 'DS_INTERNAL.KERNEL_ENUMERATION', JupyterInstallFailed = 'DS_INTERNAL.JUPYTER_INSTALL_FAILED', UserInstalledModule = 'DATASCIENCE.USER_INSTALLED_MODULE', - JupyterCommandLineNonDefault = 'DS_INTERNAL.JUPYTER_CUSTOM_COMMAND_LINE' + JupyterCommandLineNonDefault = 'DS_INTERNAL.JUPYTER_CUSTOM_COMMAND_LINE', + NewFileForInteractiveWindow = 'DS_INTERNAL.NEW_FILE_USED_IN_INTERACTIVE' } export enum NativeKeyboardCommandTelemetry { diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 71a3a07476c6..36ac533916e1 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -466,6 +466,8 @@ export abstract class InteractiveBase extends WebViewHost; + protected abstract getNotebookResource(): Promise; + protected abstract closeBecauseOfFailure(exc: Error): Promise; protected async clearResult(id: string): Promise { @@ -1060,8 +1062,12 @@ export abstract class InteractiveBase extends WebViewHost { // Create a new notebook if we need to. if (!this._notebook) { - const [uri, options] = await Promise.all([this.getNotebookIdentity(), this.getNotebookOptions()]); - this._notebook = await server.createNotebook(uri, options.metadata); + const [resource, uri, options] = await Promise.all([ + this.getNotebookResource(), + this.getNotebookIdentity(), + this.getNotebookOptions() + ]); + this._notebook = await server.createNotebook(resource, uri, options.metadata); if (this._notebook) { this.postMessage(InteractiveWindowMessages.NotebookExecutionActivated, uri.toString()).ignoreErrors(); diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 9e9d4ef4d7d0..bfd140f95717 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -483,6 +483,11 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this._file; } + protected getNotebookResource(): Promise { + // Resource to use for loading and our identity are the same. + return this.getNotebookIdentity(); + } + protected async setLaunchingFile(_file: string): Promise { // For the native editor, use our own file as the path const notebook = this.getNotebook(); diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index cb00778dc8fe..5fd4fc6913f7 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -173,6 +173,9 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi } public async addCode(code: string, file: string, line: number, editor?: TextEditor): Promise { + if (this.lastFile && !this.fileSystem.arePathsSame(file, this.lastFile)) { + sendTelemetryEvent(Telemetry.NewFileForInteractiveWindow); + } // Save the last file we ran with. this.lastFile = file; @@ -299,6 +302,17 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi return Uri.parse(Identifiers.InteractiveWindowIdentity); } + protected async getNotebookResource(): Promise { + if (this.lastFile) { + return Uri.file(this.lastFile); + } + const root = this.workspaceService.rootPath; + if (root) { + return Uri.file(root); + } + return Uri.file('./'); + } + protected updateContexts(info: IInteractiveWindowInfo | undefined) { // This should be called by the python interactive window every // time state changes. We use this opportunity to update our diff --git a/src/client/datascience/jupyter/jupyterDebugger.ts b/src/client/datascience/jupyter/jupyterDebugger.ts index 5428c24c2775..31a92e14fb23 100644 --- a/src/client/datascience/jupyter/jupyterDebugger.ts +++ b/src/client/datascience/jupyter/jupyterDebugger.ts @@ -92,7 +92,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { } public async stopDebugging(notebook: INotebook): Promise { - const config = this.configs.get(notebook.resource.toString()); + const config = this.configs.get(notebook.identity.toString()); if (config) { traceInfo('stop debugging'); @@ -107,7 +107,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { } public onRestart(notebook: INotebook): void { - this.configs.delete(notebook.resource.toString()); + this.configs.delete(notebook.identity.toString()); } public async hashesUpdated(hashes: IFileHashes[]): Promise { @@ -142,7 +142,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { private async connect(notebook: INotebook): Promise { // If we already have configuration, we're already attached, don't do it again. - let result = this.configs.get(notebook.resource.toString()); + let result = this.configs.get(notebook.identity.toString()); if (result) { const settings = this.configService.getSettings(); result.justMyCode = settings.datascience.debugJustMyCode; @@ -170,7 +170,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { } if (result) { - this.configs.set(notebook.resource.toString(), result); + this.configs.set(notebook.identity.toString(), result); } return result; diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts index 7ddbcb773e1f..3ca07ace41b1 100644 --- a/src/client/datascience/jupyter/jupyterExecution.ts +++ b/src/client/datascience/jupyter/jupyterExecution.ts @@ -141,6 +141,7 @@ export class JupyterExecutionBase implements IJupyterExecution { // We can do this in parallel, while starting the server (faster). traceInfo(`Getting kernel specs for ${options ? options.purpose : 'unknown type of'} server`); kernelSpecInterpreterPromise = this.kernelSelector.getKernelForLocalConnection( + undefined, undefined, options?.metadata, !allowUI, @@ -169,6 +170,7 @@ export class JupyterExecutionBase implements IJupyterExecution { ); const sessionManager = await sessionManagerFactory.create(connection); kernelSpecInterpreter = await this.kernelSelector.getKernelForRemoteConnection( + undefined, sessionManager, options?.metadata, cancelToken @@ -221,6 +223,7 @@ export class JupyterExecutionBase implements IJupyterExecution { >(IJupyterSessionManagerFactory); const sessionManager = await sessionManagerFactory.create(connection); const kernelInterpreter = await this.kernelSelector.selectLocalKernel( + undefined, new StopWatch(), sessionManager, cancelToken, diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts index 9e2203f07115..c7014ef37c10 100644 --- a/src/client/datascience/jupyter/jupyterNotebook.ts +++ b/src/client/datascience/jupyter/jupyterNotebook.ts @@ -150,7 +150,7 @@ export class JupyterNotebookBase implements INotebook { private sessionStartTime: number; private pendingCellSubscriptions: CellSubscriber[] = []; private ranInitialSetup = false; - private _resource: Uri; + private _identity: Uri; private _disposed: boolean = false; private _workingDirectory: string | undefined; private _loggers: INotebookExecutionLogger[] = []; @@ -170,7 +170,7 @@ export class JupyterNotebookBase implements INotebook { private owner: INotebookServer, private launchInfo: INotebookServerLaunchInfo, loggers: INotebookExecutionLogger[], - resource: Uri, + identity: Uri, private getDisposedError: () => Error, private workspace: IWorkspaceService, private applicationService: IApplicationShell, @@ -184,7 +184,7 @@ export class JupyterNotebookBase implements INotebook { } }; this.sessionStatusChanged = this.session.onSessionStatusChanged(statusChangeHandler); - this._resource = resource; + this._identity = identity; this._loggers = [...loggers]; // Save our interpreter and don't change it. Later on when kernel changes // are possible, recompute it. @@ -202,7 +202,7 @@ export class JupyterNotebookBase implements INotebook { this.sessionStatusChanged.dispose(); } - traceInfo(`Shutting down session ${this.resource.toString()}`); + traceInfo(`Shutting down session ${this.identity.toString()}`); if (!this._disposed) { this._disposed = true; const dispose = this.session ? this.session.dispose() : undefined; @@ -225,8 +225,8 @@ export class JupyterNotebookBase implements INotebook { return ServerStatus.NotStarted; } - public get resource(): Uri { - return this._resource; + public get identity(): Uri { + return this._identity; } public waitForIdle(timeoutMs: number): Promise { @@ -253,7 +253,7 @@ export class JupyterNotebookBase implements INotebook { this.initializedMatplotlib = false; const configInit = !settings || settings.enablePlotViewer ? CodeSnippits.ConfigSvg : CodeSnippits.ConfigPng; - traceInfo(`Initialize config for plots for ${this.resource.toString()}`); + traceInfo(`Initialize config for plots for ${this.identity.toString()}`); await this.executeSilently(configInit, cancelToken); } @@ -266,7 +266,7 @@ export class JupyterNotebookBase implements INotebook { traceInfo(`Run startup code for notebook: ${cleanedUp} - results: ${cells.length}`); } - traceInfo(`Initial setup complete for ${this.resource.toString()}`); + traceInfo(`Initial setup complete for ${this.identity.toString()}`); } catch (e) { traceWarning(e); } @@ -625,7 +625,7 @@ export class JupyterNotebookBase implements INotebook { ? CodeSnippits.MatplotLibInitSvg : CodeSnippits.MatplotLibInitPng; - traceInfo(`Initialize matplotlib for ${this.resource.toString()}`); + traceInfo(`Initialize matplotlib for ${this.identity.toString()}`); // Force matplotlib to inline and save the default style. We'll use this later if we // get a request to update style await this.executeSilently(matplobInit, cancelToken); diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts index 81a8b2f6edb5..36e441cfcbde 100644 --- a/src/client/datascience/jupyter/jupyterServer.ts +++ b/src/client/datascience/jupyter/jupyterServer.ts @@ -82,6 +82,7 @@ export class JupyterServerBase implements INotebookServer { public createNotebook( resource: Uri, + identity: Uri, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken ): Promise { @@ -95,6 +96,7 @@ export class JupyterServerBase implements INotebookServer { // Create a notebook and return it. return this.createNotebookInstance( resource, + identity, this.sessionManager, savedSession, this.disposableRegistry, @@ -177,27 +179,28 @@ export class JupyterServerBase implements INotebookServer { return new Error(localize.DataScience.sessionDisposed()); } - public async getNotebook(resource: Uri): Promise { - return this.notebooks.get(resource.toString()); + public async getNotebook(identity: Uri): Promise { + return this.notebooks.get(identity.toString()); } protected getNotebooks(): INotebook[] { return [...this.notebooks.values()]; } - protected setNotebook(resource: Uri, notebook: INotebook) { + protected setNotebook(identity: Uri, notebook: INotebook) { const oldDispose = notebook.dispose; notebook.dispose = () => { - this.notebooks.delete(resource.toString()); + this.notebooks.delete(identity.toString()); return oldDispose(); }; // Save the notebook - this.notebooks.set(resource.toString(), notebook); + this.notebooks.set(identity.toString(), notebook); } protected createNotebookInstance( _resource: Uri, + _identity: Uri, _sessionManager: IJupyterSessionManager, _savedSession: IJupyterSession | undefined, _disposableRegistry: IDisposableRegistry, diff --git a/src/client/datascience/jupyter/jupyterServerWrapper.ts b/src/client/datascience/jupyter/jupyterServerWrapper.ts index 28ef4c842c14..58f0278a1dcf 100644 --- a/src/client/datascience/jupyter/jupyterServerWrapper.ts +++ b/src/client/datascience/jupyter/jupyterServerWrapper.ts @@ -108,11 +108,12 @@ export class JupyterServerWrapper implements INotebookServer, ILiveShareHasRole public async createNotebook( resource: Uri, + identity: Uri, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken ): Promise { const server = await this.serverFactory.get(); - return server.createNotebook(resource, notebookMetadata, cancelToken); + return server.createNotebook(resource, identity, notebookMetadata, cancelToken); } public async shutdown(): Promise { diff --git a/src/client/datascience/jupyter/jupyterVariables.ts b/src/client/datascience/jupyter/jupyterVariables.ts index 6e5d4e930bb9..8f11d7596038 100644 --- a/src/client/datascience/jupyter/jupyterVariables.ts +++ b/src/client/datascience/jupyter/jupyterVariables.ts @@ -260,7 +260,7 @@ export class JupyterVariables implements IJupyterVariables { request: IJupyterVariablesRequest ): Promise { // See if we already have the name list - let list = this.notebookState.get(notebook.resource); + let list = this.notebookState.get(notebook.identity); if (!list || list.currentExecutionCount !== request.executionCount) { // Refetch the list of names from the notebook. They might have changed. list = { @@ -315,7 +315,7 @@ export class JupyterVariables implements IJupyterVariables { } // Save in our cache - this.notebookState.set(notebook.resource, list); + this.notebookState.set(notebook.identity, list); // Update total count (exclusions will change this as types are computed) result.totalCount = list.variables.length; diff --git a/src/client/datascience/jupyter/kernels/kernelSelections.ts b/src/client/datascience/jupyter/kernels/kernelSelections.ts index 5e77f53c3c7b..fc4f49dbb9ea 100644 --- a/src/client/datascience/jupyter/kernels/kernelSelections.ts +++ b/src/client/datascience/jupyter/kernels/kernelSelections.ts @@ -7,7 +7,7 @@ import { inject, injectable } from 'inversify'; import { CancellationToken } from 'vscode'; import { PYTHON_LANGUAGE } from '../../../common/constants'; import { IFileSystem } from '../../../common/platform/types'; -import { IPathUtils } from '../../../common/types'; +import { IPathUtils, Resource } from '../../../common/types'; import * as localize from '../../../common/utils/localize'; import { IInterpreterSelector } from '../../../interpreter/configuration/types'; import { IJupyterKernelSpec, IJupyterSessionManager } from '../../types'; @@ -65,6 +65,7 @@ function getQuickPickItemForActiveKernel(kernel: LiveKernelModel, pathUtils: IPa export class ActiveJupyterSessionKernelSelectionListProvider implements IKernelSelectionListProvider { constructor(private readonly sessionManager: IJupyterSessionManager, private readonly pathUtils: IPathUtils) {} public async getKernelSelections( + _resource: Resource, _cancelToken?: CancellationToken | undefined ): Promise { const [activeKernels, activeSessions, kernelSpecs] = await Promise.all([ @@ -105,7 +106,10 @@ export class InstalledJupyterKernelSelectionListProvider implements IKernelSelec private readonly pathUtils: IPathUtils, private readonly sessionManager?: IJupyterSessionManager ) {} - public async getKernelSelections(cancelToken?: CancellationToken | undefined): Promise { + public async getKernelSelections( + _resource: Resource, + cancelToken?: CancellationToken | undefined + ): Promise { const items = await this.kernelService.getKernelSpecs(this.sessionManager, cancelToken); return items .filter(item => (item.language || '').toLowerCase() === PYTHON_LANGUAGE.toLowerCase()) @@ -124,9 +128,10 @@ export class InstalledJupyterKernelSelectionListProvider implements IKernelSelec export class InterpreterKernelSelectionListProvider implements IKernelSelectionListProvider { constructor(private readonly interpreterSelector: IInterpreterSelector) {} public async getKernelSelections( + resource: Resource, _cancelToken?: CancellationToken | undefined ): Promise { - const items = await this.interpreterSelector.getSuggestions(undefined); + const items = await this.interpreterSelector.getSuggestions(resource); return items.map(item => { return { ...item, @@ -163,6 +168,7 @@ export class KernelSelectionProvider { * @memberof KernelSelectionProvider */ public async getKernelSelectionsForRemoteSession( + resource: Resource, sessionManager: IJupyterSessionManager, cancelToken?: CancellationToken ): Promise { @@ -171,11 +177,11 @@ export class KernelSelectionProvider { this.kernelService, this.pathUtils, sessionManager - ).getKernelSelections(cancelToken); + ).getKernelSelections(resource, cancelToken); const liveKernelsPromise = new ActiveJupyterSessionKernelSelectionListProvider( sessionManager, this.pathUtils - ).getKernelSelections(cancelToken); + ).getKernelSelections(resource, cancelToken); const [installedKernels, liveKernels] = await Promise.all([installedKernelsPromise, liveKernelsPromise]); // Sorty by name. @@ -199,6 +205,7 @@ export class KernelSelectionProvider { * @memberof KernelSelectionProvider */ public async getKernelSelectionsForLocalSession( + resource: Resource, sessionManager?: IJupyterSessionManager, cancelToken?: CancellationToken ): Promise { @@ -207,10 +214,10 @@ export class KernelSelectionProvider { this.kernelService, this.pathUtils, sessionManager - ).getKernelSelections(cancelToken); + ).getKernelSelections(resource, cancelToken); const interpretersPromise = new InterpreterKernelSelectionListProvider( this.interpreterSelector - ).getKernelSelections(cancelToken); + ).getKernelSelections(resource, cancelToken); // tslint:disable-next-line: prefer-const let [installedKernels, interpreters] = await Promise.all([installedKernelsPromise, interpretersPromise]); diff --git a/src/client/datascience/jupyter/kernels/kernelSelector.ts b/src/client/datascience/jupyter/kernels/kernelSelector.ts index 33cbaa3bf258..dc609140e413 100644 --- a/src/client/datascience/jupyter/kernels/kernelSelector.ts +++ b/src/client/datascience/jupyter/kernels/kernelSelector.ts @@ -9,7 +9,7 @@ import { CancellationToken } from 'vscode-jsonrpc'; import { IApplicationShell } from '../../../common/application/types'; import { traceError, traceInfo, traceVerbose } from '../../../common/logger'; -import { IInstaller, Product } from '../../../common/types'; +import { IInstaller, Product, Resource } from '../../../common/types'; import * as localize from '../../../common/utils/localize'; import { noop } from '../../../common/utils/misc'; import { StopWatch } from '../../../common/utils/stopWatch'; @@ -90,14 +90,20 @@ export class KernelSelector { * @memberof KernelSelector */ public async selectRemoteKernel( + resource: Resource, stopWatch: StopWatch, session: IJupyterSessionManager, cancelToken?: CancellationToken, currentKernel?: IJupyterKernelSpec | LiveKernelModel ): Promise { - let suggestions = await this.selectionProvider.getKernelSelectionsForRemoteSession(session, cancelToken); + let suggestions = await this.selectionProvider.getKernelSelectionsForRemoteSession( + resource, + session, + cancelToken + ); suggestions = suggestions.filter(item => !this.kernelIdsToHide.has(item.selection.kernelModel?.id || '')); return this.selectKernel( + resource, stopWatch, Telemetry.SelectRemoteJupyterKernel, suggestions, @@ -115,14 +121,20 @@ export class KernelSelector { * @memberof KernelSelector */ public async selectLocalKernel( + resource: Resource, stopWatch: StopWatch, session?: IJupyterSessionManager, cancelToken?: CancellationToken, currentKernel?: IJupyterKernelSpec | LiveKernelModel ): Promise { - let suggestions = await this.selectionProvider.getKernelSelectionsForLocalSession(session, cancelToken); + let suggestions = await this.selectionProvider.getKernelSelectionsForLocalSession( + resource, + session, + cancelToken + ); suggestions = suggestions.filter(item => !this.kernelIdsToHide.has(item.selection.kernelModel?.id || '')); return this.selectKernel( + resource, stopWatch, Telemetry.SelectLocalJupyterKernel, suggestions, @@ -143,6 +155,7 @@ export class KernelSelector { */ @reportAction(ReportableAction.KernelsGetKernelForLocalConnection) public async getKernelForLocalConnection( + resource: Resource, sessionManager?: IJupyterSessionManager, notebookMetadata?: nbformat.INotebookMetadata, disableUI?: boolean, @@ -156,7 +169,7 @@ export class KernelSelector { }; // When this method is called, we know we've started a local jupyter server. // Lets pre-warm the list of local kernels. - this.selectionProvider.getKernelSelectionsForLocalSession(sessionManager, cancelToken).ignoreErrors(); + this.selectionProvider.getKernelSelectionsForLocalSession(resource, sessionManager, cancelToken).ignoreErrors(); let selection: KernelSpecInterpreter = {}; if (notebookMetadata?.kernelspec) { @@ -176,6 +189,7 @@ export class KernelSelector { const activeInterpreter = await this.interpreterService.getActiveInterpreter(undefined); if (activeInterpreter) { selection = await this.useInterpreterAsKernel( + resource, activeInterpreter, notebookMetadata.kernelspec.display_name, sessionManager, @@ -184,7 +198,7 @@ export class KernelSelector { ); } else { telemetryProps.promptedToSelect = true; - selection = await this.selectLocalKernel(stopWatch, sessionManager, cancelToken); + selection = await this.selectLocalKernel(resource, stopWatch, sessionManager, cancelToken); } } } else { @@ -223,12 +237,13 @@ export class KernelSelector { // tslint:disable-next-line: cyclomatic-complexity @reportAction(ReportableAction.KernelsGetKernelForRemoteConnection) public async getKernelForRemoteConnection( + resource: Resource, sessionManager?: IJupyterSessionManager, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken ): Promise { const [interpreter, specs] = await Promise.all([ - this.interpreterService.getActiveInterpreter(undefined), + this.interpreterService.getActiveInterpreter(resource), this.kernelService.getKernelSpecs(sessionManager, cancelToken) ]); let bestMatch: IJupyterKernelSpec | undefined; @@ -279,6 +294,7 @@ export class KernelSelector { }; } private async selectKernel( + resource: Resource, stopWatch: StopWatch, telemetryEvent: Telemetry, suggestions: IKernelSpecQuickPickItem[], @@ -297,7 +313,14 @@ export class KernelSelector { // Check if ipykernel is installed in this kernel. if (selection.selection.interpreter) { sendTelemetryEvent(Telemetry.SwitchToInterpreterAsKernel); - return this.useInterpreterAsKernel(selection.selection.interpreter, undefined, session, false, cancelToken); + return this.useInterpreterAsKernel( + resource, + selection.selection.interpreter, + undefined, + session, + false, + cancelToken + ); } else if (selection.selection.kernelModel) { sendTelemetryEvent(Telemetry.SwitchToExistingKernel, undefined, { language: this.computeLanguage(selection.selection.kernelModel.language) @@ -338,6 +361,7 @@ export class KernelSelector { * @memberof KernelSelector */ private async useInterpreterAsKernel( + resource: Resource, interpreter: PythonInterpreter, displayNameOfKernelNotFound?: string, session?: IJupyterSessionManager, @@ -392,7 +416,7 @@ export class KernelSelector { // When this method is called, we know a new kernel may have been registered. // Lets pre-warm the list of local kernels (with the new list). - this.selectionProvider.getKernelSelectionsForLocalSession(session, cancelToken).ignoreErrors(); + this.selectionProvider.getKernelSelectionsForLocalSession(resource, session, cancelToken).ignoreErrors(); return { kernelSpec, interpreter }; } diff --git a/src/client/datascience/jupyter/kernels/types.ts b/src/client/datascience/jupyter/kernels/types.ts index bc541b01b853..e6950493c494 100644 --- a/src/client/datascience/jupyter/kernels/types.ts +++ b/src/client/datascience/jupyter/kernels/types.ts @@ -5,6 +5,7 @@ import { Session } from '@jupyterlab/services'; import { CancellationToken, QuickPickItem } from 'vscode'; +import { Resource } from '../../../common/types'; import { PythonInterpreter } from '../../../interpreter/contracts'; import { IJupyterKernel, IJupyterKernelSpec } from '../../types'; @@ -30,5 +31,5 @@ export interface IKernelSpecQuickPickItem extends QuickPickItem { } export interface IKernelSelectionListProvider { - getKernelSelections(cancelToken?: CancellationToken): Promise; + getKernelSelections(resource: Resource, cancelToken?: CancellationToken): Promise; } diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts index eea2517f349b..b15762d42215 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts @@ -50,7 +50,7 @@ export class GuestJupyterNotebook super(liveShare); } - public get resource(): Uri { + public get identity(): Uri { return this._resource; } @@ -171,7 +171,7 @@ export class GuestJupyterNotebook public async waitForServiceName(): Promise { // Use our base name plus our id. This means one unique server per notebook // Live share will not accept a '.' in the name so remove any - const uriString = this.resource.toString(); + const uriString = this.identity.toString(); return Promise.resolve(`${LiveShare.JupyterNotebookSharedService}${uriString}`); } diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts index 34b711da9946..761565109fe3 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts @@ -54,12 +54,13 @@ export class GuestJupyterServer return Promise.resolve(); } - public async createNotebook(resource: Uri): Promise { + public async createNotebook(resource: Uri, identity: Uri): Promise { // Tell the host side to generate a notebook for this uri const service = await this.waitForService(); if (service) { - const uriString = resource.toString(); - await service.request(LiveShareCommands.createNotebook, [uriString]); + const resourceString = resource.toString(); + const identityString = identity.toString(); + await service.request(LiveShareCommands.createNotebook, [resourceString, identityString]); } // Return a new notebook to listen to diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts index e9cf4397f202..9721a1f1913a 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts @@ -49,7 +49,7 @@ export class HostJupyterNotebook owner: INotebookServer, launchInfo: INotebookServerLaunchInfo, loggers: INotebookExecutionLogger[], - resource: vscode.Uri, + identity: vscode.Uri, getDisposedError: () => Error, workspace: IWorkspaceService, appService: IApplicationShell, @@ -63,7 +63,7 @@ export class HostJupyterNotebook owner, launchInfo, loggers, - resource, + identity, getDisposedError, workspace, appService, @@ -127,7 +127,7 @@ export class HostJupyterNotebook // Use our base name plus our id. This means one unique server per notebook // Convert to our shared URI to match the guest and remove any '.' as live share won't support them const sharedUri = - this.resource.scheme === 'file' ? this.finishedApi!.convertLocalUriToShared(this.resource) : this.resource; + this.identity.scheme === 'file' ? this.finishedApi!.convertLocalUriToShared(this.identity) : this.identity; return Promise.resolve(`${LiveShare.JupyterNotebookSharedService}${sharedUri.toString()}`); } diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index 4a571b695139..6a9b626be3fb 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -93,14 +93,12 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas service.onRequest( LiveShareCommands.createNotebook, async (args: any[], cancellation: CancellationToken) => { - const uri = vscode.Uri.parse(args[0]); - const resource = - uri.scheme && uri.scheme !== Identifiers.InteractiveWindowIdentityScheme - ? this.finishedApi!.convertSharedUriToLocal(uri) - : uri; + const resource = this.parseUri(args[0]); + const identity = this.parseUri(args[1]); // Don't return the notebook. We don't want it to be serialized. We just want its live share server to be started. const notebook = (await this.createNotebook( resource, + identity, undefined, cancellation )) as HostJupyterNotebook; @@ -150,6 +148,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas protected async createNotebookInstance( resource: vscode.Uri, + identity: vscode.Uri, sessionManager: IJupyterSessionManager, possibleSession: IJupyterSession | undefined, disposableRegistry: IDisposableRegistry, @@ -159,7 +158,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas cancelToken?: CancellationToken ): Promise { // See if already exists. - const existing = await this.getNotebook(resource); + const existing = await this.getNotebook(identity); if (existing) { // Dispose the possible session as we don't need it if (possibleSession) { @@ -189,8 +188,19 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas let defaultKernelInfoToUse = launchInfo.kernelSpec; if (notebookMetadata?.kernelspec) { const kernelInfo = await (launchInfo.connectionInfo.localLaunch - ? this.kernelSelector.getKernelForLocalConnection(sessionManager, notebookMetadata, false, cancelToken) - : this.kernelSelector.getKernelForRemoteConnection(sessionManager, notebookMetadata, cancelToken)); + ? this.kernelSelector.getKernelForLocalConnection( + resource, + sessionManager, + notebookMetadata, + false, + cancelToken + ) + : this.kernelSelector.getKernelForRemoteConnection( + resource, + sessionManager, + notebookMetadata, + cancelToken + )); const kernelInfoToUse = kernelInfo?.kernelSpec || kernelInfo?.kernelModel; if (kernelInfoToUse) { @@ -245,6 +255,13 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas throw this.getDisposedError(); } + private parseUri(uri: string): vscode.Uri { + const parsed = vscode.Uri.parse(uri); + return parsed.scheme && parsed.scheme !== Identifiers.InteractiveWindowIdentityScheme + ? this.finishedApi!.convertSharedUriToLocal(parsed) + : parsed; + } + private async attemptToForwardPort(api: vsls.LiveShare | null | undefined, port: number): Promise { if (port !== 0 && api && api.session && api.session.role === vsls.Role.Host) { this.portToForward = 0; diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index abce31accf20..3510e722b42c 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -94,10 +94,11 @@ export interface INotebookServer extends IAsyncDisposable { readonly id: string; createNotebook( resource: Uri, + identity: Uri, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken ): Promise; - getNotebook(resource: Uri): Promise; + getNotebook(identity: Uri): Promise; connect(launchInfo: INotebookServerLaunchInfo, cancelToken?: CancellationToken): Promise; getConnectionInfo(): IConnection | undefined; waitForConnect(): Promise; @@ -105,7 +106,7 @@ export interface INotebookServer extends IAsyncDisposable { } export interface INotebook extends IAsyncDisposable { - readonly resource: Uri; + readonly identity: Uri; readonly server: INotebookServer; readonly status: ServerStatus; onSessionStatusChanged: Event; diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index ae4c6a774d23..a028e086191a 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1820,4 +1820,10 @@ export interface IEventNamePropertyMapping { * @memberof IEventNamePropertyMapping */ [Telemetry.JupyterCommandLineNonDefault]: undefined | never; + /** + * Telemetry event sent when a user runs the interactive window with a new file + * @type {(undefined | never)} + * @memberof IEventNamePropertyMapping + */ + [Telemetry.NewFileForInteractiveWindow]: undefined | never; } diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index ff2e5b559a0d..97183d3c0030 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -383,6 +383,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private extraListeners: ((m: string, p: any) => void)[] = []; private webPanelProvider: TypeMoq.IMock | undefined; + private settingsMap = new Map(); constructor() { super(); @@ -754,7 +755,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.pythonSettings.downloadLanguageServer = false; const workspaceConfig = (this.mockedWorkspaceConfig = mock(MockWorkspaceConfiguration)); - configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => this.pythonSettings); + configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(this.getSettings.bind(this)); when(workspaceConfig.get(anything(), anything())).thenCall((_, defaultValue) => defaultValue); when(workspaceConfig.has(anything())).thenReturn(false); when((workspaceConfig as any).then).thenReturn(undefined); @@ -1133,8 +1134,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { return false; } - public getSettings() { - return this.pythonSettings; + public getSettings(resource: Uri | undefined) { + const setting = resource ? this.settingsMap.get(resource.toString()) : this.pythonSettings; + return setting ? setting : this.pythonSettings; } public forceSettingsChanged(newPath: string, datascienceSettings?: IDataScienceSettings) { @@ -1148,6 +1150,22 @@ export class DataScienceIocContainer extends UnitTestIocContainer { }); } + public async addNewSetting(resource: Uri, pythonPath: string | undefined) { + // Force a new config setting to appear. + if (!pythonPath) { + const active = await this.get(IInterpreterService).getActiveInterpreter(undefined); + const list = await this.get(IInterpreterService).getInterpreters(); + + // Should support jupyter? How to enforce this + const supportsJupyter = list.filter(l => l.path !== active?.path).filter(f => hasJupyter(f.path)); + pythonPath = supportsJupyter ? supportsJupyter[0].path : undefined; + } + if (pythonPath) { + const newSettings: PythonSettings = { ...this.pythonSettings, pythonPath }; + this.settingsMap.set(resource.toString(), newSettings); + } + } + public get mockJupyter(): MockJupyterManager | undefined { return this.jupyterMock ? this.jupyterMock.getManager() : undefined; } diff --git a/src/test/datascience/mockJupyterNotebook.ts b/src/test/datascience/mockJupyterNotebook.ts index 5e6f9ee5f397..03958ea35005 100644 --- a/src/test/datascience/mockJupyterNotebook.ts +++ b/src/test/datascience/mockJupyterNotebook.ts @@ -32,7 +32,7 @@ export class MockJupyterNotebook implements INotebook { return this.owner; } - public get resource(): Uri { + public get identity(): Uri { return Uri.parse(Identifiers.InteractiveWindowIdentity); } From d4508ac9be40acd0594e62549d6c682c4473a50f Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 14 Feb 2020 10:48:46 -0800 Subject: [PATCH 02/14] Framing for a test when we get to it --- .../interactive-common/interactiveBase.ts | 26 ++--- .../interactive-ipynb/nativeEditor.ts | 100 +++++++++--------- .../interactive-window/interactiveWindow.ts | 32 +++--- .../datascience/dataScienceIocContainer.ts | 18 +++- .../interactiveWindow.functional.test.tsx | 44 ++++++++ .../interactiveWindowTestHelpers.tsx | 19 +++- .../datascience/notebook.functional.test.ts | 13 +-- 7 files changed, 158 insertions(+), 94 deletions(-) diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 36ac533916e1..ba1fe76c67ff 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -96,6 +96,17 @@ import { InteractiveWindowMessageListener } from './interactiveWindowMessageList @injectable() export abstract class InteractiveBase extends WebViewHost implements IInteractiveBase { + public get notebook(): INotebook | undefined { + return this._notebook; + } + + public get id(): string { + return this._id; + } + + public get onExecutedCode(): Event { + return this.executeEvent.event; + } private unfinishedCells: ICell[] = []; private restartingKernel: boolean = false; private potentiallyUnfinishedStatus: Disposable[] = []; @@ -106,9 +117,6 @@ export abstract class InteractiveBase extends WebViewHost | undefined; private notebookPromise: Promise | undefined; private setDarkPromise: Deferred | undefined; - public get notebook(): INotebook | undefined { - return this._notebook; - } constructor( @unmanaged() private readonly progressReporter: ProgressReporter, @@ -182,10 +190,6 @@ export abstract class InteractiveBase extends WebViewHost { // Verify a server that matches us hasn't started already this.checkForServerStart().ignoreErrors(); @@ -194,10 +198,6 @@ export abstract class InteractiveBase extends WebViewHost { - return this.executeEvent.event; - } - // tslint:disable-next-line: no-any no-empty cyclomatic-complexity max-func-body-length public onMessage(message: string, payload: any) { switch (message) { @@ -419,6 +419,8 @@ export abstract class InteractiveBase extends WebViewHost; + protected onViewStateChanged(args: WebViewViewChangeEventArgs) { // Only activate if the active editor is empty. This means that // vscode thinks we are actually supposed to have focus. It would be @@ -466,8 +468,6 @@ export abstract class InteractiveBase extends WebViewHost; - protected abstract getNotebookResource(): Promise; - protected abstract closeBecauseOfFailure(exc: Error): Promise; protected async clearResult(id: string): Promise { diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index bfd140f95717..991aace53eaf 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -101,6 +101,51 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { public get onDidChangeViewState(): Event { return this._onDidChangeViewState.event; } + + public get visible(): boolean { + return this.viewState.visible; + } + + public get active(): boolean { + return this.viewState.active; + } + + public get file(): Uri { + return this._file; + } + + public get isUntitled(): boolean { + const baseName = path.basename(this.file.fsPath); + return baseName.includes(localize.DataScience.untitledNotebookFileName()); + } + + public get cells(): ICell[] { + return this.visibleCells; + } + + public get closed(): Event { + return this.closedEvent.event; + } + + public get executed(): Event { + return this.executedEvent.event; + } + + public get modified(): Event { + return this.modifiedEvent.event; + } + + public get saved(): Event { + return this.savedEvent.event; + } + + public get metadataUpdated(): Event { + return this.metadataUpdatedEvent.event; + } + + public get isDirty(): boolean { + return this._dirty; + } private _onDidChangeViewState = new EventEmitter(); private closedEvent: EventEmitter = new EventEmitter(); private executedEvent: EventEmitter = new EventEmitter(); @@ -184,23 +229,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { experimentsManager ); } - - public get visible(): boolean { - return this.viewState.visible; - } - - public get active(): boolean { - return this.viewState.active; - } - - public get file(): Uri { - return this._file; - } - - public get isUntitled(): boolean { - const baseName = path.basename(this.file.fsPath); - return baseName.includes(localize.DataScience.untitledNotebookFileName()); - } public dispose(): Promise { super.dispose(); return this.close(); @@ -210,10 +238,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this.generateNotebookContent(this.visibleCells); } - public get cells(): ICell[] { - return this.visibleCells; - } - public async load(contents: string, file: Uri): Promise { // Save our uri this._file = file; @@ -248,30 +272,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } } - public get closed(): Event { - return this.closedEvent.event; - } - - public get executed(): Event { - return this.executedEvent.event; - } - - public get modified(): Event { - return this.modifiedEvent.event; - } - - public get saved(): Event { - return this.savedEvent.event; - } - - public get metadataUpdated(): Event { - return this.metadataUpdatedEvent.event; - } - - public get isDirty(): boolean { - return this._dirty; - } - // tslint:disable-next-line: no-any public onMessage(message: string, payload: any) { super.onMessage(message, payload); @@ -355,6 +355,11 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this.setDirty(); } + public getNotebookResource(): Promise { + // Resource to use for loading and our identity are the same. + return this.getNotebookIdentity(); + } + protected addSysInfo(_reason: SysInfoReason): Promise { // These are not supported. return Promise.resolve(); @@ -483,11 +488,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this._file; } - protected getNotebookResource(): Promise { - // Resource to use for loading and our identity are the same. - return this.getNotebookIdentity(); - } - protected async setLaunchingFile(_file: string): Promise { // For the native editor, use our own file as the path const notebook = this.getNotebook(); diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index 5fd4fc6913f7..1030685cc9c5 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -59,13 +59,17 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi public get onDidChangeViewState(): Event { return this._onDidChangeViewState.event; } - private _onDidChangeViewState = new EventEmitter(); public get visible(): boolean { return this.viewState.visible; } public get active(): boolean { return this.viewState.active; } + + public get closed(): Event { + return this.closedEvent.event; + } + private _onDidChangeViewState = new EventEmitter(); private closedEvent: EventEmitter = new EventEmitter(); private waitingForExportCells: boolean = false; private trackedJupyterStart: boolean = false; @@ -148,10 +152,6 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi } } - public get closed(): Event { - return this.closedEvent.event; - } - public addMessage(message: string): Promise { this.addMessageImpl(message); return Promise.resolve(); @@ -263,6 +263,17 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi public scrollToCell(id: string): void { this.postMessage(InteractiveWindowMessages.ScrollToCell, { id }).ignoreErrors(); } + + public async getNotebookResource(): Promise { + if (this.lastFile) { + return Uri.file(this.lastFile); + } + const root = this.workspaceService.rootPath; + if (root) { + return Uri.file(root); + } + return Uri.file('./'); + } protected async onViewStateChanged(args: WebViewViewChangeEventArgs) { super.onViewStateChanged(args); this._onDidChangeViewState.fire(); @@ -302,17 +313,6 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi return Uri.parse(Identifiers.InteractiveWindowIdentity); } - protected async getNotebookResource(): Promise { - if (this.lastFile) { - return Uri.file(this.lastFile); - } - const root = this.workspaceService.rootPath; - if (root) { - return Uri.file(root); - } - return Uri.file('./'); - } - protected updateContexts(info: IInteractiveWindowInfo | undefined) { // This should be called by the python interactive window every // time state changes. We use this opportunity to update our diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 97183d3c0030..93d3154bab09 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -383,7 +383,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private extraListeners: ((m: string, p: any) => void)[] = []; private webPanelProvider: TypeMoq.IMock | undefined; - private settingsMap = new Map(); + private settingsMap = new Map(); constructor() { super(); @@ -1134,7 +1134,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { return false; } - public getSettings(resource: Uri | undefined) { + public getSettings(resource?: Uri) { const setting = resource ? this.settingsMap.get(resource.toString()) : this.pythonSettings; return setting ? setting : this.pythonSettings; } @@ -1157,11 +1157,11 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const list = await this.get(IInterpreterService).getInterpreters(); // Should support jupyter? How to enforce this - const supportsJupyter = list.filter(l => l.path !== active?.path).filter(f => hasJupyter(f.path)); + const supportsJupyter = list.filter(l => l.path !== active?.path).filter(f => this.hasJupyter(f.path)); pythonPath = supportsJupyter ? supportsJupyter[0].path : undefined; } if (pythonPath) { - const newSettings: PythonSettings = { ...this.pythonSettings, pythonPath }; + const newSettings = { ...this.pythonSettings, pythonPath }; this.settingsMap.set(resource.toString(), newSettings); } } @@ -1214,6 +1214,16 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } } + private hasJupyter(pythonPath: string) { + try { + // Try importing jupyter + const output = child_process.execFileSync(pythonPath, ['-c', 'import jupyter;'], { encoding: 'utf8' }); + return !output.includes('ModuleNotFoundError'); + } catch (ex) { + return false; + } + } + private findPythonPath(): string { try { // Give preference to the CI test python (could also be set in launch.json for debugging). diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 1af28c676826..2e88563856de 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -566,6 +566,50 @@ Type: builtin_function_or_method`, } ); + runMountedTest( + 'Multiple Interpreters', + async _wrapper => { + // if (!ioc.mockJupyter) { + // const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); + // const interpreterService = ioc.get(IInterpreterService); + // const window = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; + // await addCode(ioc, wrapper, 'a=1\na'); + // const activeInterpreter = await interpreterService.getActiveInterpreter( + // await window.getNotebookResource() + // ); + // verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); + // assert.equal( + // window.notebook!.getMatchingInterpreter()?.path, + // activeInterpreter?.path, + // 'Active intrepreter not used to launch notebook' + // ); + // await closeInteractiveWindow(window, wrapper); + + // // Add another python path (hopefully there's more than one on the machine?) + // const secondUri = Uri.file('bar.py'); + // await ioc.addNewSetting(secondUri, undefined); + // const newWrapper = mountWebView(ioc, 'interactive'); + // assert.ok(newWrapper, 'Could not mount a second time'); + // const newWindow = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; + // await addCode(ioc, wrapper, 'a=1\na', false, secondUri); + // verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); + // assert.notEqual( + // newWindow.notebook!.getMatchingInterpreter()?.path, + // activeInterpreter?.path, + // 'Active intrepreter used to launch second notebook when it should not have' + // ); + // } else { + // tslint:disable-next-line: no-console + console.log( + 'Multiple interpreters test skipped for now. Reenable after fixing https://github.com/microsoft/vscode-python/issues/10134' + ); + // } + }, + () => { + return ioc; + } + ); + runMountedTest( 'Dispose test', async () => { diff --git a/src/test/datascience/interactiveWindowTestHelpers.tsx b/src/test/datascience/interactiveWindowTestHelpers.tsx index 2911078e3732..28d96909779a 100644 --- a/src/test/datascience/interactiveWindowTestHelpers.tsx +++ b/src/test/datascience/interactiveWindowTestHelpers.tsx @@ -23,9 +23,17 @@ export function getInteractiveCellResults( export async function getOrCreateInteractiveWindow(ioc: DataScienceIocContainer): Promise { const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); - const window = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; - await window.show(); - return window; + return (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; +} + +export function closeInteractiveWindow( + window: IInteractiveWindow, + // tslint:disable-next-line: no-any + wrapper: ReactWrapper, React.Component> +) { + const promise = window.dispose(); + wrapper.unmount(); + return promise; } export function runMountedTest( @@ -53,7 +61,8 @@ export async function addCode( // tslint:disable-next-line: no-any wrapper: ReactWrapper, React.Component>, code: string, - expectError: boolean = false + expectError: boolean = false, + uri: Uri = Uri.file('foo.py') // tslint:disable-next-line: no-any ): Promise, React.Component>> { // Adding code should cause 5 renders to happen. @@ -64,7 +73,7 @@ export async function addCode( // 5) Status finished return getInteractiveCellResults(ioc, wrapper, async () => { const history = await getOrCreateInteractiveWindow(ioc); - const success = await history.addCode(code, Uri.file('foo.py').fsPath, 2); + const success = await history.addCode(code, uri.fsPath, 2); if (expectError) { assert.equal(success, false, `${code} did not produce an error`); } diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index c93e233cbf7e..c5eec4385a36 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -66,6 +66,7 @@ suite('DataScience notebook tests', () => { let processFactory: IProcessServiceFactory; let ioc: DataScienceIocContainer; let modifiedConfig = false; + const baseUri = Uri.file('foo.py'); setup(() => { ioc = new DataScienceIocContainer(); @@ -305,7 +306,7 @@ suite('DataScience notebook tests', () => { assert.ok(false, `Expected server to not be created`); } if (server) { - const notebook = await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)); + const notebook = await server.createNotebook(baseUri, Uri.parse(Identifiers.InteractiveWindowIdentity)); // If specified set our launch file if (launchingFile) { await notebook.setLaunchingFile(launchingFile); @@ -409,7 +410,7 @@ suite('DataScience notebook tests', () => { // We have a connection string here, so try to connect jupyterExecution to the notebook server const server = await jupyterExecution.connectToNotebookServer({ uri, useDefaultConfig: true, purpose: '' }); const notebook = server - ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) + ? await server.createNotebook(baseUri, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; if (!notebook) { assert.fail('Failed to connect to remote self cert server'); @@ -466,7 +467,7 @@ suite('DataScience notebook tests', () => { purpose: '' }); const notebook = server - ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) + ? await server.createNotebook(baseUri, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; if (!notebook) { assert.fail('Failed to connect to remote password server'); @@ -547,7 +548,7 @@ suite('DataScience notebook tests', () => { // We have a connection string here, so try to connect jupyterExecution to the notebook server const server = await jupyterExecution.connectToNotebookServer({ uri, useDefaultConfig: true, purpose: '' }); const notebook = server - ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) + ? await server.createNotebook(baseUri, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; if (!notebook) { assert.fail('Failed to connect to remote password server'); @@ -594,7 +595,7 @@ suite('DataScience notebook tests', () => { // We have a connection string here, so try to connect jupyterExecution to the notebook server const server = await jupyterExecution.connectToNotebookServer({ uri, useDefaultConfig: true, purpose: '' }); const notebook = server - ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) + ? await server.createNotebook(baseUri, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; if (!notebook) { assert.fail('Failed to connect to remote server'); @@ -908,7 +909,7 @@ suite('DataScience notebook tests', () => { const nonCancelSource = new CancellationTokenSource(); const server = await jupyterExecution.connectToNotebookServer(undefined, nonCancelSource.token); const notebook = server - ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) + ? await server.createNotebook(baseUri, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; assert.ok(notebook, 'Server not found with a cancel token that does not cancel'); From c306769e9a2fa9a81cb3541dd02403e5a9e39319 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 14 Feb 2020 12:00:43 -0800 Subject: [PATCH 03/14] Make kernelspec change if interpreter different. --- .../interactiveWindowCommandListener.ts | 2 +- .../datascience/jupyter/jupyterNotebook.ts | 8 +- .../jupyter/kernels/kernelSelector.ts | 4 +- .../jupyter/kernels/kernelSwitcher.ts | 17 +- .../jupyter/liveshare/guestJupyterNotebook.ts | 9 +- .../jupyter/liveshare/guestJupyterServer.ts | 1 + .../jupyter/liveshare/hostJupyterNotebook.ts | 4 +- .../jupyter/liveshare/hostJupyterServer.ts | 116 +++++++----- src/client/datascience/types.ts | 5 +- .../dataviewer.functional.test.tsx | 4 +- .../gotocell.functional.test.ts | 4 +- ...eractiveWindowCommandListener.unit.test.ts | 4 +- .../kernels/kernelSelections.unit.test.ts | 15 +- .../kernels/kernelSelector.unit.test.ts | 175 ++++++++++++++---- .../kernels/kernelSwitcher.unit.test.ts | 11 +- src/test/datascience/mockJupyterNotebook.ts | 5 + 16 files changed, 281 insertions(+), 103 deletions(-) diff --git a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts index cc1854c1c967..6fe3dce56529 100644 --- a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts +++ b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts @@ -326,7 +326,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList cancelToken ); const notebook = server - ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) + ? await server.createNotebook(undefined, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; // If that works, then execute all of the cells. diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts index c7014ef37c10..14e78d5a0562 100644 --- a/src/client/datascience/jupyter/jupyterNotebook.ts +++ b/src/client/datascience/jupyter/jupyterNotebook.ts @@ -14,7 +14,7 @@ import { CancellationError, createPromiseFromCancellation } from '../../common/c import '../../common/extensions'; import { traceError, traceInfo, traceWarning } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, IDisposableRegistry } from '../../common/types'; +import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; import { createDeferred, Deferred, waitForPromise } from '../../common/utils/async'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -150,6 +150,7 @@ export class JupyterNotebookBase implements INotebook { private sessionStartTime: number; private pendingCellSubscriptions: CellSubscriber[] = []; private ranInitialSetup = false; + private _resource: Resource; private _identity: Uri; private _disposed: boolean = false; private _workingDirectory: string | undefined; @@ -170,6 +171,7 @@ export class JupyterNotebookBase implements INotebook { private owner: INotebookServer, private launchInfo: INotebookServerLaunchInfo, loggers: INotebookExecutionLogger[], + resource: Resource, identity: Uri, private getDisposedError: () => Error, private workspace: IWorkspaceService, @@ -185,6 +187,7 @@ export class JupyterNotebookBase implements INotebook { }; this.sessionStatusChanged = this.session.onSessionStatusChanged(statusChangeHandler); this._identity = identity; + this._resource = resource; this._loggers = [...loggers]; // Save our interpreter and don't change it. Later on when kernel changes // are possible, recompute it. @@ -225,6 +228,9 @@ export class JupyterNotebookBase implements INotebook { return ServerStatus.NotStarted; } + public get resource(): Resource { + return this._resource; + } public get identity(): Uri { return this._identity; } diff --git a/src/client/datascience/jupyter/kernels/kernelSelector.ts b/src/client/datascience/jupyter/kernels/kernelSelector.ts index dc609140e413..d628ed71cb70 100644 --- a/src/client/datascience/jupyter/kernels/kernelSelector.ts +++ b/src/client/datascience/jupyter/kernels/kernelSelector.ts @@ -186,7 +186,7 @@ export class KernelSelector { sendTelemetryEvent(Telemetry.UseExistingKernel); } else { // No kernel info, hence prmopt to use current interpreter as a kernel. - const activeInterpreter = await this.interpreterService.getActiveInterpreter(undefined); + const activeInterpreter = await this.interpreterService.getActiveInterpreter(resource); if (activeInterpreter) { selection = await this.useInterpreterAsKernel( resource, @@ -203,7 +203,7 @@ export class KernelSelector { } } else { // No kernel info, hence use current interpreter as a kernel. - const activeInterpreter = await this.interpreterService.getActiveInterpreter(undefined); + const activeInterpreter = await this.interpreterService.getActiveInterpreter(resource); if (activeInterpreter) { selection.interpreter = activeInterpreter; selection.kernelSpec = await this.kernelService.searchAndRegisterKernel( diff --git a/src/client/datascience/jupyter/kernels/kernelSwitcher.ts b/src/client/datascience/jupyter/kernels/kernelSwitcher.ts index 586442e61c17..98fe4ae0c403 100644 --- a/src/client/datascience/jupyter/kernels/kernelSwitcher.ts +++ b/src/client/datascience/jupyter/kernels/kernelSwitcher.ts @@ -6,7 +6,7 @@ import { inject, injectable } from 'inversify'; import { ProgressLocation, ProgressOptions } from 'vscode'; import { IApplicationShell } from '../../../common/application/types'; -import { IConfigurationService } from '../../../common/types'; +import { IConfigurationService, Resource } from '../../../common/types'; import { Common, DataScience } from '../../../common/utils/localize'; import { StopWatch } from '../../../common/utils/stopWatch'; import { Commands, Settings } from '../../constants'; @@ -40,30 +40,32 @@ export class KernelSwitcher { settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch; if (isLocalConnection) { - kernel = await this.selectLocalJupyterKernel(notebook?.getKernelSpec()); + kernel = await this.selectLocalJupyterKernel(notebook.resource, notebook?.getKernelSpec()); } else if (notebook) { const connInfo = notebook.server.getConnectionInfo(); const currentKernel = notebook.getKernelSpec(); if (connInfo) { - kernel = await this.selectRemoteJupyterKernel(connInfo, currentKernel); + kernel = await this.selectRemoteJupyterKernel(notebook.resource, connInfo, currentKernel); } } return kernel; } private async selectLocalJupyterKernel( + resource: Resource, currentKernel?: IJupyterKernelSpec | LiveKernelModel ): Promise { - return this.kernelSelector.selectLocalKernel(new StopWatch(), undefined, undefined, currentKernel); + return this.kernelSelector.selectLocalKernel(resource, new StopWatch(), undefined, undefined, currentKernel); } private async selectRemoteJupyterKernel( + resource: Resource, connInfo: IConnection, currentKernel?: IJupyterKernelSpec | LiveKernelModel ): Promise { const stopWatch = new StopWatch(); const session = await this.jupyterSessionManagerFactory.create(connInfo); - return this.kernelSelector.selectRemoteKernel(stopWatch, session, undefined, currentKernel); + return this.kernelSelector.selectRemoteKernel(resource, stopWatch, session, undefined, currentKernel); } private async switchKernelWithRetry(notebook: INotebook, kernel: KernelSpecInterpreter): Promise { const settings = this.configService.getSettings(); @@ -102,7 +104,10 @@ export class KernelSwitcher { const cancel = Common.cancel(); const selection = await this.appShell.showErrorMessage(message, selectKernel, cancel); if (selection === selectKernel) { - kernel = await this.selectLocalJupyterKernel(kernel.kernelSpec || kernel.kernelModel); + kernel = await this.selectLocalJupyterKernel( + notebook.resource, + kernel.kernelSpec || kernel.kernelModel + ); if (Object.keys(kernel).length > 0) { continue; } diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts index b15762d42215..8406f4c14e0a 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts @@ -10,7 +10,7 @@ import { ServerStatus } from '../../../../datascience-ui/interactive-common/main import { ILiveShareApi } from '../../../common/application/types'; import { CancellationError } from '../../../common/cancellation'; import { traceInfo } from '../../../common/logger'; -import { IConfigurationService, IDisposableRegistry } from '../../../common/types'; +import { IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types'; import { createDeferred } from '../../../common/utils/async'; import * as localize from '../../../common/utils/localize'; import { noop } from '../../../common/utils/misc'; @@ -43,7 +43,8 @@ export class GuestJupyterNotebook liveShare: ILiveShareApi, private disposableRegistry: IDisposableRegistry, private configService: IConfigurationService, - private _resource: Uri, + private _resource: Resource, + private _identity: Uri, private _owner: INotebookServer, private startTime: number ) { @@ -51,6 +52,10 @@ export class GuestJupyterNotebook } public get identity(): Uri { + return this._identity; + } + + public get resource(): Resource { return this._resource; } diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts index 761565109fe3..72b7c4e4e3ce 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts @@ -69,6 +69,7 @@ export class GuestJupyterServer this.disposableRegistry, this.configService, resource, + identity, this, this.dataScience.activationStartTime ); diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts index 9721a1f1913a..9eef63c72f46 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts @@ -9,7 +9,7 @@ import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../../co import '../../../common/extensions'; import { traceError } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; -import { IConfigurationService, IDisposableRegistry } from '../../../common/types'; +import { IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types'; import { createDeferred } from '../../../common/utils/async'; import { Identifiers, LiveShare, LiveShareCommands } from '../../constants'; import { IExecuteInfo } from '../../interactive-common/interactiveWindowTypes'; @@ -49,6 +49,7 @@ export class HostJupyterNotebook owner: INotebookServer, launchInfo: INotebookServerLaunchInfo, loggers: INotebookExecutionLogger[], + resource: Resource, identity: vscode.Uri, getDisposedError: () => Error, workspace: IWorkspaceService, @@ -63,6 +64,7 @@ export class HostJupyterNotebook owner, launchInfo, loggers, + resource, identity, getDisposedError, workspace, diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index 6a9b626be3fb..03791eb5d60c 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -14,6 +14,7 @@ import { traceInfo } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types'; import * as localize from '../../../common/utils/localize'; +import { IInterpreterService } from '../../../interpreter/contracts'; import { Identifiers, LiveShare, LiveShareCommands, RegExpValues } from '../../constants'; import { IDataScience, @@ -50,7 +51,8 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas loggers: INotebookExecutionLogger[], private appService: IApplicationShell, private fs: IFileSystem, - private readonly kernelSelector: KernelSelector + private readonly kernelSelector: KernelSelector, + private readonly interpreterService: IInterpreterService ) { super(liveShare, asyncRegistry, disposableRegistry, configService, sessionManager, loggers); } @@ -169,53 +171,24 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas return existing; } - // Otherwise create a new notebook. - - // First we need our launch information so we can start a new session (that's what our notebook is really) - let launchInfo = await this.waitForConnect(); - if (!launchInfo) { - throw this.getDisposedError(); + // Compute launch information from the resource and the notebook metadata + const { info, changedKernel } = await this.computeLaunchInfo( + resource, + sessionManager, + notebookMetadata, + cancelToken + ); + + // If we switched kernels, try switching the possible session + if (changedKernel && possibleSession && info.kernelSpec) { + await possibleSession.changeKernel( + info.kernelSpec, + this.configService.getSettings().datascience.jupyterLaunchTimeout + ); } - // Create a copy of launch info, cuz we're modifying it here. - // This launch info contains the server connection info (that could be shared across other nbs). - // However the kernel info is different. The kernel info is stored as a property of this, hence create a separate instance for each nb. - launchInfo = { - ...launchInfo - }; - // Find a kernel that can be used. - // Do this only if kernel information has been provided in the metadata, else use the default. - let defaultKernelInfoToUse = launchInfo.kernelSpec; - if (notebookMetadata?.kernelspec) { - const kernelInfo = await (launchInfo.connectionInfo.localLaunch - ? this.kernelSelector.getKernelForLocalConnection( - resource, - sessionManager, - notebookMetadata, - false, - cancelToken - ) - : this.kernelSelector.getKernelForRemoteConnection( - resource, - sessionManager, - notebookMetadata, - cancelToken - )); - - const kernelInfoToUse = kernelInfo?.kernelSpec || kernelInfo?.kernelModel; - if (kernelInfoToUse) { - defaultKernelInfoToUse = kernelInfoToUse; - } - if (possibleSession && kernelInfoToUse) { - await possibleSession.changeKernel( - kernelInfoToUse, - this.configService.getSettings().datascience.jupyterLaunchTimeout - ); - } - launchInfo.kernelSpec = defaultKernelInfoToUse; - } // Start a session (or use the existing one) - const session = possibleSession || (await sessionManager.startNew(defaultKernelInfoToUse, cancelToken)); + const session = possibleSession || (await sessionManager.startNew(info.kernelSpec, cancelToken)); traceInfo(`Started session ${this.id}`); if (session) { @@ -226,9 +199,10 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas configService, disposableRegistry, this, - launchInfo, + info, loggers, resource, + identity, this.getDisposedError.bind(this), this.workspaceService, this.appService, @@ -255,6 +229,56 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas throw this.getDisposedError(); } + private async computeLaunchInfo( + resource: vscode.Uri, + sessionManager: IJupyterSessionManager, + notebookMetadata?: nbformat.INotebookMetadata, + cancelToken?: CancellationToken + ): Promise<{ info: INotebookServerLaunchInfo; changedKernel: boolean }> { + // First we need our launch information so we can start a new session (that's what our notebook is really) + let launchInfo = await this.waitForConnect(); + if (!launchInfo) { + throw this.getDisposedError(); + } + // Create a copy of launch info, cuz we're modifying it here. + // This launch info contains the server connection info (that could be shared across other nbs). + // However the kernel info is different. The kernel info is stored as a property of this, hence create a separate instance for each nb. + launchInfo = { + ...launchInfo + }; + + // Determine the interpreter for our resource. If different, we need a different kernel. + const resourceInterpreter = await this.interpreterService.getActiveInterpreter(resource); + + // Find a kernel that can be used. + // Do this only if kernel information has been provided in the metadata, or the resource's interpreter is different. + let changedKernel = false; + if (notebookMetadata?.kernelspec || resourceInterpreter?.displayName !== launchInfo.interpreter?.displayName) { + const kernelInfo = await (launchInfo.connectionInfo.localLaunch + ? this.kernelSelector.getKernelForLocalConnection( + resource, + sessionManager, + notebookMetadata, + false, + cancelToken + ) + : this.kernelSelector.getKernelForRemoteConnection( + resource, + sessionManager, + notebookMetadata, + cancelToken + )); + + const kernelInfoToUse = kernelInfo?.kernelSpec || kernelInfo?.kernelModel; + if (kernelInfoToUse) { + launchInfo.kernelSpec = kernelInfoToUse; + changedKernel = true; + } + } + + return { info: launchInfo, changedKernel }; + } + private parseUri(uri: string): vscode.Uri { const parsed = vscode.Uri.parse(uri); return parsed.scheme && parsed.scheme !== Identifiers.InteractiveWindowIdentityScheme diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 3510e722b42c..fcb6c8671fb3 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -21,7 +21,7 @@ import { import { ServerStatus } from '../../datascience-ui/interactive-common/mainState'; import { ICommandManager } from '../common/application/types'; import { ExecutionResult, ObservableExecutionResult, SpawnOptions } from '../common/process/types'; -import { IAsyncDisposable, IDataScienceSettings, IDisposable } from '../common/types'; +import { IAsyncDisposable, IDataScienceSettings, IDisposable, Resource } from '../common/types'; import { StopWatch } from '../common/utils/stopWatch'; import { PythonInterpreter } from '../interpreter/contracts'; import { JupyterCommands } from './constants'; @@ -93,7 +93,7 @@ export const INotebookServer = Symbol('INotebookServer'); export interface INotebookServer extends IAsyncDisposable { readonly id: string; createNotebook( - resource: Uri, + resource: Resource, identity: Uri, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken @@ -106,6 +106,7 @@ export interface INotebookServer extends IAsyncDisposable { } export interface INotebook extends IAsyncDisposable { + readonly resource: Resource; readonly identity: Uri; readonly server: INotebookServer; readonly status: ServerStatus; diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index 2a8f83a3842e..09799641ffb3 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -100,7 +100,9 @@ suite('DataScience DataViewer tests', () => { const exec = ioc.get(IJupyterExecution); const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); const server = await exec.connectToNotebookServer(await interactiveWindowProvider.getNotebookOptions()); - notebook = server ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; + notebook = server + ? await server.createNotebook(undefined, Uri.parse(Identifiers.InteractiveWindowIdentity)) + : undefined; if (notebook) { const cells = await notebook.execute(code, Identifiers.EmptyFileName, 0, uuid()); assert.equal(cells.length, 1, `Wrong number of cells returned`); diff --git a/src/test/datascience/editor-integration/gotocell.functional.test.ts b/src/test/datascience/editor-integration/gotocell.functional.test.ts index d3f8385eb6bf..56acb1b96bbb 100644 --- a/src/test/datascience/editor-integration/gotocell.functional.test.ts +++ b/src/test/datascience/editor-integration/gotocell.functional.test.ts @@ -92,7 +92,9 @@ suite('DataScience gotocell tests', () => { if (expectFailure) { assert.ok(false, `Expected server to not be created`); } - return server ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; + return server + ? await server.createNotebook(undefined, Uri.parse(Identifiers.InteractiveWindowIdentity)) + : undefined; } catch (exc) { if (!expectFailure) { assert.ok(false, `Expected server to be created, but got ${exc}`); diff --git a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts index fafe3cf97d86..0e6e02220fcb 100644 --- a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts +++ b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts @@ -261,7 +261,9 @@ suite('Interactive window command listener', async () => { await documentManager.showTextDocument(doc); when(jupyterExecution.connectToNotebookServer(anything(), anything())).thenResolve(server.object); const notebook = createTypeMoq('jupyter notebook'); - server.setup(s => s.createNotebook(TypeMoq.It.isAny())).returns(() => Promise.resolve(notebook.object)); + server + .setup(s => s.createNotebook(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => Promise.resolve(notebook.object)); notebook .setup(n => n.execute( diff --git a/src/test/datascience/jupyter/kernels/kernelSelections.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSelections.unit.test.ts index 4957e3bc6e73..4219fed1dfe0 100644 --- a/src/test/datascience/jupyter/kernels/kernelSelections.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSelections.unit.test.ts @@ -138,7 +138,10 @@ suite('Data Science - KernelSelections', () => { when(sessionManager.getRunningKernels()).thenResolve([]); when(sessionManager.getRunningSessions()).thenResolve([]); - const items = await kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager)); + const items = await kernelSelectionProvider.getKernelSelectionsForRemoteSession( + undefined, + instance(sessionManager) + ); assert.equal(items.length, 0); }); @@ -193,7 +196,10 @@ suite('Data Science - KernelSelections', () => { ]; expectedItems.sort((a, b) => (a.label === b.label ? 0 : a.label > b.label ? 1 : -1)); - const items = await kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager)); + const items = await kernelSelectionProvider.getKernelSelectionsForRemoteSession( + undefined, + instance(sessionManager) + ); verify(sessionManager.getRunningKernels()).once(); verify(sessionManager.getKernelSpecs()).once(); @@ -229,7 +235,10 @@ suite('Data Science - KernelSelections', () => { const expectedList = [...expectedKernelItems, ...expectedInterpreterItems]; expectedList.sort((a, b) => (a.label === b.label ? 0 : a.label > b.label ? 1 : -1)); - const items = await kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager)); + const items = await kernelSelectionProvider.getKernelSelectionsForLocalSession( + undefined, + instance(sessionManager) + ); verify(kernelService.getKernelSpecs(anything(), anything())).once(); assert.deepEqual(items, expectedList); diff --git a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts index 6ec0510af9e2..6918b67048f9 100644 --- a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts @@ -10,7 +10,7 @@ import { ApplicationShell } from '../../../../client/common/application/applicat import { IApplicationShell } from '../../../../client/common/application/types'; import { PYTHON_LANGUAGE } from '../../../../client/common/constants'; import { ProductInstaller } from '../../../../client/common/installer/productInstaller'; -import { IInstaller, Product } from '../../../../client/common/types'; +import { IInstaller, Product, Resource } from '../../../../client/common/types'; import * as localize from '../../../../client/common/utils/localize'; import { noop } from '../../../../client/common/utils/misc'; import { Architecture } from '../../../../client/common/utils/platform'; @@ -70,35 +70,59 @@ suite('Data Science - KernelSelector', () => { suite('Select Remote Kernel', () => { test('Should display quick pick and return nothing when nothing is selected (remote sessions)', async () => { when( - kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForRemoteSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when(appShell.showQuickPick(anything(), anything(), anything())).thenResolve(); - const kernel = await kernelSelector.selectRemoteKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectRemoteKernel( + undefined, + new StopWatch(), + instance(sessionManager) + ); assert.isEmpty(kernel); verify( - kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForRemoteSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); }); test('Should display quick pick and return nothing when nothing is selected (local sessions)', async () => { when( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when(appShell.showQuickPick(anything(), anything(), anything())).thenResolve(); - const kernel = await kernelSelector.selectLocalKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectLocalKernel(undefined, new StopWatch(), instance(sessionManager)); assert.isEmpty(kernel); verify( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); }); test('Should return the selected remote kernelspec along with a matching interpreter', async () => { when( - kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForRemoteSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when(kernelService.findMatchingInterpreter(kernelSpec, anything())).thenResolve(interpreter); when(appShell.showQuickPick(anything(), anything(), anything())).thenResolve({ @@ -106,12 +130,20 @@ suite('Data Science - KernelSelector', () => { // tslint:disable-next-line: no-any } as any); - const kernel = await kernelSelector.selectRemoteKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectRemoteKernel( + undefined, + new StopWatch(), + instance(sessionManager) + ); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isOk(kernel.interpreter === interpreter); verify( - kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForRemoteSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); verify(kernelService.findMatchingInterpreter(kernelSpec, anything())).once(); @@ -165,7 +197,11 @@ suite('Data Science - KernelSelector', () => { }); when( - kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForRemoteSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve(quickPickItems); when(appShell.showQuickPick(anything(), anything(), anything())).thenResolve(undefined); @@ -173,11 +209,19 @@ suite('Data Science - KernelSelector', () => { kernelSelector.addKernelToIgnoreList({ id: 'id2' } as any); // tslint:disable-next-line: no-any kernelSelector.addKernelToIgnoreList({ clientId: 'id4' } as any); - const kernel = await kernelSelector.selectRemoteKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectRemoteKernel( + undefined, + new StopWatch(), + instance(sessionManager) + ); assert.isEmpty(kernel); verify( - kernelSelectionProvider.getKernelSelectionsForRemoteSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForRemoteSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); const suggestions = capture(appShell.showQuickPick).first()[0] as IKernelSpecQuickPickItem[]; @@ -233,7 +277,11 @@ suite('Data Science - KernelSelector', () => { }); when( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve(quickPickItems); when(appShell.showQuickPick(anything(), anything(), anything())).thenResolve(undefined); @@ -241,11 +289,15 @@ suite('Data Science - KernelSelector', () => { kernelSelector.addKernelToIgnoreList({ id: 'id2' } as any); // tslint:disable-next-line: no-any kernelSelector.addKernelToIgnoreList({ clientId: 'id4' } as any); - const kernel = await kernelSelector.selectLocalKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectLocalKernel(undefined, new StopWatch(), instance(sessionManager)); assert.isEmpty(kernel); verify( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); const suggestions = capture(appShell.showQuickPick).first()[0] as IKernelSpecQuickPickItem[]; @@ -258,7 +310,11 @@ suite('Data Science - KernelSelector', () => { suite('Select Local Kernel', () => { test('Should return the selected local kernelspec along with a matching interpreter', async () => { when( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when(kernelService.findMatchingInterpreter(kernelSpec, anything())).thenResolve(interpreter); when(appShell.showQuickPick(anything(), anything(), anything())).thenResolve({ @@ -266,12 +322,16 @@ suite('Data Science - KernelSelector', () => { // tslint:disable-next-line: no-any } as any); - const kernel = await kernelSelector.selectLocalKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectLocalKernel(undefined, new StopWatch(), instance(sessionManager)); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isOk(kernel.interpreter === interpreter); verify( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); verify(kernelService.findMatchingInterpreter(kernelSpec, anything())).once(); @@ -282,7 +342,11 @@ suite('Data Science - KernelSelector', () => { kernelSpec ); when( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when( appShell.showInformationMessage(localize.DataScience.fallbackToUseActiveInterpeterAsKernel()) @@ -292,13 +356,17 @@ suite('Data Science - KernelSelector', () => { // tslint:disable-next-line: no-any } as any); - const kernel = await kernelSelector.selectLocalKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectLocalKernel(undefined, new StopWatch(), instance(sessionManager)); assert.isOk(kernel.kernelSpec === kernelSpec); verify(installer.isInstalled(Product.ipykernel, interpreter)).once(); verify(kernelService.findMatchingKernelSpec(interpreter, instance(sessionManager), anything())).once(); verify( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).once(); verify(appShell.showQuickPick(anything(), anything(), anything())).once(); verify(kernelService.registerKernel(anything(), anything())).never(); @@ -314,7 +382,11 @@ suite('Data Science - KernelSelector', () => { when(kernelService.findMatchingKernelSpec(interpreter, instance(sessionManager), anything())).thenResolve(); when(kernelService.registerKernel(interpreter, anything(), anything())).thenResolve(kernelSpec); when( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when( appShell.showInformationMessage(localize.DataScience.fallBackToRegisterAndUseActiveInterpeterAsKernel()) @@ -324,14 +396,18 @@ suite('Data Science - KernelSelector', () => { // tslint:disable-next-line: no-any } as any); - const kernel = await kernelSelector.selectLocalKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectLocalKernel(undefined, new StopWatch(), instance(sessionManager)); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isOk(kernel.interpreter === interpreter); verify(installer.isInstalled(Product.ipykernel, interpreter)).once(); verify(kernelService.findMatchingKernelSpec(interpreter, instance(sessionManager), anything())).once(); verify( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).twice(); // Once for caching. verify(appShell.showQuickPick(anything(), anything(), anything())).once(); verify( @@ -345,7 +421,11 @@ suite('Data Science - KernelSelector', () => { when(installer.isInstalled(Product.ipykernel, interpreter)).thenResolve(false); when(kernelService.registerKernel(interpreter, anything(), anything())).thenResolve(kernelSpec); when( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).thenResolve([]); when( appShell.showInformationMessage(localize.DataScience.fallBackToRegisterAndUseActiveInterpeterAsKernel()) @@ -355,12 +435,16 @@ suite('Data Science - KernelSelector', () => { // tslint:disable-next-line: no-any } as any); - const kernel = await kernelSelector.selectLocalKernel(new StopWatch(), instance(sessionManager)); + const kernel = await kernelSelector.selectLocalKernel(undefined, new StopWatch(), instance(sessionManager)); assert.isOk(kernel.kernelSpec === kernelSpec); verify(installer.isInstalled(Product.ipykernel, interpreter)).once(); verify( - kernelSelectionProvider.getKernelSelectionsForLocalSession(instance(sessionManager), anything()) + kernelSelectionProvider.getKernelSelectionsForLocalSession( + anything(), + instance(sessionManager), + anything() + ) ).twice(); // once for caching. verify(appShell.showQuickPick(anything(), anything(), anything())).once(); verify(kernelService.findMatchingKernelSpec(interpreter, instance(sessionManager), anything())).never(); @@ -382,6 +466,7 @@ suite('Data Science - KernelSelector', () => { let nbMetadata: nbformat.INotebookMetadata = {} as any; let selectLocalKernelStub: sinon.SinonStub< [ + Resource, StopWatch, (IJupyterSessionManager | undefined)?, (CancellationToken | undefined)?, @@ -412,7 +497,11 @@ suite('Data Science - KernelSelector', () => { when(kernelService.findMatchingInterpreter(kernelSpec, anything())).thenResolve(interpreter); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForLocalConnection(instance(sessionManager), nbMetadata); + const kernel = await kernelSelector.getKernelForLocalConnection( + anything(), + instance(sessionManager), + nbMetadata + ); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isOk(kernel.interpreter === interpreter); @@ -431,7 +520,11 @@ suite('Data Science - KernelSelector', () => { when(kernelService.findMatchingInterpreter(kernelSpec, anything())).thenResolve(); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForLocalConnection(instance(sessionManager), nbMetadata); + const kernel = await kernelSelector.getKernelForLocalConnection( + undefined, + instance(sessionManager), + nbMetadata + ); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isUndefined(kernel.interpreter); @@ -462,7 +555,11 @@ suite('Data Science - KernelSelector', () => { ).thenResolve(); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForLocalConnection(instance(sessionManager), nbMetadata); + const kernel = await kernelSelector.getKernelForLocalConnection( + undefined, + instance(sessionManager), + nbMetadata + ); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isOk(kernel.interpreter === interpreter); @@ -495,7 +592,11 @@ suite('Data Science - KernelSelector', () => { when(kernelService.searchAndRegisterKernel(interpreter, anything(), anything())).thenResolve(kernelSpec); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForLocalConnection(instance(sessionManager), undefined); + const kernel = await kernelSelector.getKernelForLocalConnection( + undefined, + instance(sessionManager), + undefined + ); assert.isOk(kernel.kernelSpec === kernelSpec); assert.isOk(kernel.interpreter === interpreter); @@ -534,7 +635,11 @@ suite('Data Science - KernelSelector', () => { when(kernelService.searchAndRegisterKernel(interpreter, anything(), anything())).thenResolve(kernelSpec); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForRemoteConnection(instance(sessionManager), undefined); + const kernel = await kernelSelector.getKernelForRemoteConnection( + undefined, + instance(sessionManager), + undefined + ); assert.ok(kernel.kernelSpec, 'No kernel spec found for remote'); assert.equal(kernel.kernelSpec?.display_name, 'foo', 'Did not find the python kernel spec'); @@ -581,7 +686,7 @@ suite('Data Science - KernelSelector', () => { when(kernelService.searchAndRegisterKernel(interpreter, anything())).thenResolve(kernelSpec); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForRemoteConnection(instance(sessionManager), { + const kernel = await kernelSelector.getKernelForRemoteConnection(undefined, instance(sessionManager), { orig_nbformat: 4, kernelspec: { display_name: 'foo', name: 'foo' } }); @@ -631,7 +736,7 @@ suite('Data Science - KernelSelector', () => { when(kernelService.searchAndRegisterKernel(interpreter, anything())).thenResolve(kernelSpec); when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); - const kernel = await kernelSelector.getKernelForRemoteConnection(instance(sessionManager), { + const kernel = await kernelSelector.getKernelForRemoteConnection(undefined, instance(sessionManager), { orig_nbformat: 4, kernelspec: { display_name: 'foo', name: 'foo' } }); diff --git a/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts index c446b5100e0a..82c486cb9649 100644 --- a/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts @@ -130,6 +130,7 @@ suite('Data Science - Kernel Switcher', () => { if (isLocalConnection) { verify( kernelSelector.selectLocalKernel( + anything(), anything(), undefined, undefined, @@ -146,6 +147,7 @@ suite('Data Science - Kernel Switcher', () => { test('Prompt to select local kernel', async () => { when( kernelSelector.selectLocalKernel( + anything(), anything(), undefined, undefined, @@ -163,6 +165,7 @@ suite('Data Science - Kernel Switcher', () => { if (isLocalConnection) { when( kernelSelector.selectLocalKernel( + anything(), anything(), undefined, undefined, @@ -298,7 +301,13 @@ suite('Data Science - Kernel Switcher', () => { } }); when( - kernelSelector.selectLocalKernel(anything(), undefined, anything(), anything()) + kernelSelector.selectLocalKernel( + anything(), + anything(), + undefined, + anything(), + anything() + ) ).thenCall(() => { // When selecting a kernel the second time, then return a different selection. firstTimeSelectingAKernel = false; diff --git a/src/test/datascience/mockJupyterNotebook.ts b/src/test/datascience/mockJupyterNotebook.ts index 03958ea35005..8b0a385d95b3 100644 --- a/src/test/datascience/mockJupyterNotebook.ts +++ b/src/test/datascience/mockJupyterNotebook.ts @@ -4,6 +4,7 @@ import { JSONObject } from '@phosphor/coreutils/lib/json'; import { Observable } from 'rxjs/Observable'; import { CancellationToken, Event, EventEmitter, Uri } from 'vscode'; +import { Resource } from '../../client/common/types'; import { Identifiers } from '../../client/datascience/constants'; import { LiveKernelModel } from '../../client/datascience/jupyter/kernels/types'; import { @@ -36,6 +37,10 @@ export class MockJupyterNotebook implements INotebook { return Uri.parse(Identifiers.InteractiveWindowIdentity); } + public get resource(): Resource { + return Uri.file('foo.py'); + } + public clear(_id: string): void { noop(); } From f2bbca7e3ed89301c3ded91c3144efb29db23f65 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 14 Feb 2020 13:39:15 -0800 Subject: [PATCH 04/14] Add news entry --- news/2 Fixes/3123.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/3123.md diff --git a/news/2 Fixes/3123.md b/news/2 Fixes/3123.md new file mode 100644 index 000000000000..b1f23c546ae2 --- /dev/null +++ b/news/2 Fixes/3123.md @@ -0,0 +1 @@ +Change interactive window to use the python interpreter associated with the file being run. \ No newline at end of file From 3271be430f9dd70d3dd3a0ab095f9921f79195e6 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 14 Feb 2020 14:47:42 -0800 Subject: [PATCH 05/14] Fix unit test missing parameters --- src/test/datascience/execution.unit.test.ts | 4 ++- .../kernels/kernelSelector.unit.test.ts | 28 ++++++++++++++----- .../kernels/kernelSwitcher.unit.test.ts | 24 ++++++++++++++-- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index ff346b7f7676..2bb66ab22673 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -915,7 +915,9 @@ suite('Jupyter Execution', async () => { name: 'hello', path: '' }; - when(kernelSelector.getKernelForLocalConnection(anything(), anything(), anything(), anything())).thenResolve({ + when( + kernelSelector.getKernelForLocalConnection(anything(), anything(), anything(), anything(), anything()) + ).thenResolve({ kernelSpec }); const jupyterCmdExecutionService = new JupyterCommandFinderInterpreterExecutionService( diff --git a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts index 6918b67048f9..40b26e258c83 100644 --- a/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts @@ -495,7 +495,9 @@ suite('Data Science - KernelSelector', () => { kernelService.findMatchingKernelSpec(nbMetadataKernelSpec, instance(sessionManager), anything()) ).thenResolve(kernelSpec); when(kernelService.findMatchingInterpreter(kernelSpec, anything())).thenResolve(interpreter); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForLocalConnection( anything(), @@ -518,7 +520,9 @@ suite('Data Science - KernelSelector', () => { kernelService.findMatchingKernelSpec(nbMetadataKernelSpec, instance(sessionManager), anything()) ).thenResolve(kernelSpec); when(kernelService.findMatchingInterpreter(kernelSpec, anything())).thenResolve(); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForLocalConnection( undefined, @@ -553,7 +557,9 @@ suite('Data Science - KernelSelector', () => { ) ) ).thenResolve(); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForLocalConnection( undefined, @@ -590,7 +596,9 @@ suite('Data Science - KernelSelector', () => { ).thenResolve(undefined); when(interpreterService.getActiveInterpreter(undefined)).thenResolve(interpreter); when(kernelService.searchAndRegisterKernel(interpreter, anything(), anything())).thenResolve(kernelSpec); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForLocalConnection( undefined, @@ -633,7 +641,9 @@ suite('Data Science - KernelSelector', () => { ]); when(interpreterService.getActiveInterpreter(undefined)).thenResolve(interpreter); when(kernelService.searchAndRegisterKernel(interpreter, anything(), anything())).thenResolve(kernelSpec); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForRemoteConnection( undefined, @@ -684,7 +694,9 @@ suite('Data Science - KernelSelector', () => { ]); when(interpreterService.getActiveInterpreter(undefined)).thenResolve(interpreter); when(kernelService.searchAndRegisterKernel(interpreter, anything())).thenResolve(kernelSpec); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForRemoteConnection(undefined, instance(sessionManager), { orig_nbformat: 4, @@ -734,7 +746,9 @@ suite('Data Science - KernelSelector', () => { ]); when(interpreterService.getActiveInterpreter(undefined)).thenResolve(interpreter); when(kernelService.searchAndRegisterKernel(interpreter, anything())).thenResolve(kernelSpec); - when(kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything())).thenResolve(); + when( + kernelSelectionProvider.getKernelSelectionsForLocalSession(anything(), anything(), anything()) + ).thenResolve(); const kernel = await kernelSelector.getKernelForRemoteConnection(undefined, instance(sessionManager), { orig_nbformat: 4, diff --git a/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts index 82c486cb9649..285132334c9d 100644 --- a/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts @@ -139,7 +139,13 @@ suite('Data Science - Kernel Switcher', () => { ).once(); } else { verify( - kernelSelector.selectRemoteKernel(anything(), anything(), anything(), anything()) + kernelSelector.selectRemoteKernel( + anything(), + anything(), + anything(), + anything(), + anything() + ) ).once(); } }); @@ -178,7 +184,13 @@ suite('Data Science - Kernel Switcher', () => { }); } else { when( - kernelSelector.selectRemoteKernel(anything(), anything(), anything(), anything()) + kernelSelector.selectRemoteKernel( + anything(), + anything(), + anything(), + anything(), + anything() + ) ).thenResolve({ kernelModel: selectedKernel, kernelSpec: undefined, @@ -341,7 +353,13 @@ suite('Data Science - Kernel Switcher', () => { ).once(); // first time when user select a kernel, second time is when user selects after failing to switch to the first kernel. verify( - kernelSelector.selectLocalKernel(anything(), anything(), anything(), anything()) + kernelSelector.selectLocalKernel( + anything(), + anything(), + anything(), + anything(), + anything() + ) ).twice(); }); }); From e973aba93232e257765ab5d601770654048876f5 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 14 Feb 2020 15:52:40 -0800 Subject: [PATCH 06/14] Fix functional tests from barfing on shutdown --- src/datascience-ui/react-common/monacoEditor.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 59f211a09b7b..69783669e168 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -17,6 +17,7 @@ const debounce = require('lodash/debounce') as typeof import('lodash/debounce'); // tslint:disable-next-line:no-require-imports no-var-requires const throttle = require('lodash/throttle') as typeof import('lodash/throttle'); +import { noop } from '../../client/common/utils/misc'; import './monacoEditor.css'; const LINE_HEIGHT = 18; @@ -136,6 +137,14 @@ export class MonacoEditor extends React.Component Date: Fri, 14 Feb 2020 16:07:28 -0800 Subject: [PATCH 07/14] Resource instead of identity was being set as the key for a server --- src/client/datascience/jupyter/liveshare/hostJupyterServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index 03791eb5d60c..07d55ad2623b 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -220,7 +220,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas traceInfo(`Finished connecting ${this.id}`); // Save the notebook - this.setNotebook(resource, notebook); + this.setNotebook(identity, notebook); // Return the result. return notebook; From 76cd15bb2fe8bcf87722ea43d487bb4b3cc3b8cd Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 18 Feb 2020 10:02:59 -0800 Subject: [PATCH 08/14] Fix live share and hangs --- src/client/datascience/jupyter/jupyterServer.ts | 6 +++--- .../datascience/jupyter/jupyterServerWrapper.ts | 4 ++-- .../jupyter/liveshare/guestJupyterServer.ts | 10 +++++----- .../jupyter/liveshare/hostJupyterServer.ts | 17 ++++++++++------- .../interactiveWindow.functional.test.tsx | 6 ++++-- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts index 36e441cfcbde..60b2a383ab97 100644 --- a/src/client/datascience/jupyter/jupyterServer.ts +++ b/src/client/datascience/jupyter/jupyterServer.ts @@ -8,7 +8,7 @@ import { CancellationToken } from 'vscode-jsonrpc'; import { ILiveShareApi } from '../../common/application/types'; import '../../common/extensions'; import { traceError, traceInfo } from '../../common/logger'; -import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types'; +import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -81,7 +81,7 @@ export class JupyterServerBase implements INotebookServer { } public createNotebook( - resource: Uri, + resource: Resource, identity: Uri, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken @@ -199,7 +199,7 @@ export class JupyterServerBase implements INotebookServer { } protected createNotebookInstance( - _resource: Uri, + _resource: Resource, _identity: Uri, _sessionManager: IJupyterSessionManager, _savedSession: IJupyterSession | undefined, diff --git a/src/client/datascience/jupyter/jupyterServerWrapper.ts b/src/client/datascience/jupyter/jupyterServerWrapper.ts index 58f0278a1dcf..3d121f9aaf7c 100644 --- a/src/client/datascience/jupyter/jupyterServerWrapper.ts +++ b/src/client/datascience/jupyter/jupyterServerWrapper.ts @@ -10,7 +10,7 @@ import * as vsls from 'vsls/vscode'; import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../common/application/types'; import '../../common/extensions'; import { IFileSystem } from '../../common/platform/types'; -import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types'; +import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; import { IInterpreterService } from '../../interpreter/contracts'; import { IConnection, @@ -107,7 +107,7 @@ export class JupyterServerWrapper implements INotebookServer, ILiveShareHasRole } public async createNotebook( - resource: Uri, + resource: Resource, identity: Uri, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts index 72b7c4e4e3ce..79ec86f16be3 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts @@ -6,7 +6,7 @@ import { Uri } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import * as vsls from 'vsls/vscode'; import { ILiveShareApi, IWorkspaceService } from '../../../common/application/types'; -import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types'; +import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types'; import { createDeferred, Deferred } from '../../../common/utils/async'; import * as localize from '../../../common/utils/localize'; import { LiveShare, LiveShareCommands } from '../../constants'; @@ -54,11 +54,11 @@ export class GuestJupyterServer return Promise.resolve(); } - public async createNotebook(resource: Uri, identity: Uri): Promise { + public async createNotebook(resource: Resource, identity: Uri): Promise { // Tell the host side to generate a notebook for this uri const service = await this.waitForService(); if (service) { - const resourceString = resource.toString(); + const resourceString = resource ? resource.toString() : undefined; const identityString = identity.toString(); await service.request(LiveShareCommands.createNotebook, [resourceString, identityString]); } @@ -73,10 +73,10 @@ export class GuestJupyterServer this, this.dataScience.activationStartTime ); - this.notebooks.set(resource.toString(), result); + this.notebooks.set(identity.toString(), result); const oldDispose = result.dispose; result.dispose = () => { - this.notebooks.delete(resource.toString()); + this.notebooks.delete(identity.toString()); return oldDispose(); }; diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index 07d55ad2623b..da42600d63ff 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -12,7 +12,7 @@ import { nbformat } from '@jupyterlab/coreutils'; import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../../common/application/types'; import { traceInfo } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; -import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types'; +import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types'; import * as localize from '../../../common/utils/localize'; import { IInterpreterService } from '../../../interpreter/contracts'; import { Identifiers, LiveShare, LiveShareCommands, RegExpValues } from '../../constants'; @@ -100,7 +100,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas // Don't return the notebook. We don't want it to be serialized. We just want its live share server to be started. const notebook = (await this.createNotebook( resource, - identity, + identity!, undefined, cancellation )) as HostJupyterNotebook; @@ -149,7 +149,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas } protected async createNotebookInstance( - resource: vscode.Uri, + resource: Resource, identity: vscode.Uri, sessionManager: IJupyterSessionManager, possibleSession: IJupyterSession | undefined, @@ -230,7 +230,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas } private async computeLaunchInfo( - resource: vscode.Uri, + resource: Resource, sessionManager: IJupyterSessionManager, notebookMetadata?: nbformat.INotebookMetadata, cancelToken?: CancellationToken @@ -279,9 +279,12 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas return { info: launchInfo, changedKernel }; } - private parseUri(uri: string): vscode.Uri { - const parsed = vscode.Uri.parse(uri); - return parsed.scheme && parsed.scheme !== Identifiers.InteractiveWindowIdentityScheme + private parseUri(uri: string | undefined): Resource { + const parsed = uri ? vscode.Uri.parse(uri) : undefined; + return parsed && + parsed.scheme && + parsed.scheme !== Identifiers.InteractiveWindowIdentityScheme && + parsed.scheme === 'vsls' ? this.finishedApi!.convertSharedUriToLocal(parsed) : parsed; } diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 2e88563856de..98f83fc23526 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -80,9 +80,11 @@ suite('DataScience Interactive Window output tests', () => { }); async function forceSettingsChange(newSettings: IDataScienceSettings) { - await getOrCreateInteractiveWindow(ioc); + const window = await getOrCreateInteractiveWindow(ioc); + await window.show(); + const update = waitForMessage(ioc, InteractiveWindowMessages.SettingsUpdated); ioc.forceSettingsChanged(ioc.getSettings().pythonPath, newSettings); - return waitForMessage(ioc, InteractiveWindowMessages.SettingsUpdated); + return update; } // Uncomment this to debug hangs on exit From 4928e056a7412693a09b41810311f922f033fb93 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 18 Feb 2020 13:32:03 -0800 Subject: [PATCH 09/14] Add support for getting the config from a resource --- src/client/common/application/commands.ts | 2 +- src/client/datascience/cellFactory.ts | 12 ++++- src/client/datascience/codeCssGenerator.ts | 13 ++--- .../datascience/data-viewing/dataViewer.ts | 6 ++- .../editor-integration/cellhashprovider.ts | 14 ++--- .../editor-integration/codeLensFactory.ts | 17 +++--- .../editor-integration/codelensprovider.ts | 10 ++-- .../editor-integration/codewatcher.ts | 22 +++++--- .../editor-integration/decorator.ts | 7 +-- .../datascience/gather/gatherListener.ts | 2 +- .../intellisense/intellisenseProvider.ts | 2 +- .../interactive-common/interactiveBase.ts | 54 ++++++++++--------- .../interactive-ipynb/nativeEditor.ts | 11 ++-- .../interactive-ipynb/nativeEditorProvider.ts | 7 +-- .../interactive-window/interactiveWindow.ts | 11 ++-- .../interactiveWindowCommandListener.ts | 6 +-- .../interactiveWindowProvider.ts | 7 +-- .../jupyter/commandLineSelector.ts | 22 +++++--- .../datascience/jupyter/jupyterDebugger.ts | 7 +-- .../datascience/jupyter/jupyterNotebook.ts | 21 +++++--- .../datascience/jupyter/jupyterVariables.ts | 6 ++- .../jupyter/kernels/kernelSwitcher.ts | 4 +- .../jupyter/liveshare/guestJupyterNotebook.ts | 2 +- .../jupyter/liveshare/hostJupyterServer.ts | 2 +- .../datascience/jupyter/serverPreload.ts | 8 ++- src/client/datascience/plotting/plotViewer.ts | 6 ++- src/client/datascience/types.ts | 8 +-- src/client/datascience/webViewHost.ts | 27 ++++++---- src/test/datascience/color.test.ts | 6 +-- .../dataviewer.functional.test.tsx | 4 +- .../nativeEditor.functional.test.tsx | 2 +- .../testInteractiveWindowProvider.ts | 11 ++-- .../datascience/testNativeEditorProvider.ts | 11 ++-- 33 files changed, 213 insertions(+), 137 deletions(-) diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 8ca6bd356e30..0a23e01c6ef0 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -51,7 +51,6 @@ interface ICommandNameWithoutArgumentTypeMapping { [DSCommands.RunCurrentCellAdvance]: []; [DSCommands.ExecSelectionInInteractiveWindow]: []; [DSCommands.SelectJupyterURI]: []; - [DSCommands.SelectJupyterCommandLine]: []; [DSCommands.ShowHistoryPane]: []; [DSCommands.UndoCells]: []; [DSCommands.RedoCells]: []; @@ -154,4 +153,5 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.ScrollToCell]: [string, string]; [DSCommands.ViewJupyterOutput]: []; [DSCommands.SwitchJupyterKernel]: [INotebook | undefined]; + [DSCommands.SelectJupyterCommandLine]: [undefined | Uri]; } diff --git a/src/client/datascience/cellFactory.ts b/src/client/datascience/cellFactory.ts index e62c584dd4b2..01f3b5b130b1 100644 --- a/src/client/datascience/cellFactory.ts +++ b/src/client/datascience/cellFactory.ts @@ -4,13 +4,14 @@ import '../common/extensions'; import * as uuid from 'uuid/v4'; -import { Range, TextDocument } from 'vscode'; +import { Range, TextDocument, Uri } from 'vscode'; import { parseForComments } from '../../datascience-ui/common'; import { createCodeCell, createMarkdownCell } from '../../datascience-ui/common/cellFactory'; -import { IDataScienceSettings } from '../common/types'; +import { IDataScienceSettings, Resource } from '../common/types'; import { noop } from '../common/utils/misc'; import { CellMatcher } from './cellMatcher'; +import { Identifiers } from './constants'; import { CellState, ICell } from './types'; function generateCodeCell( @@ -40,6 +41,13 @@ function generateMarkdownCell(code: string[], file: string, line: number, id: st }; } +export function getCellResource(cell: ICell): Resource { + if (cell.file !== Identifiers.EmptyFileName) { + return Uri.file(cell.file); + } + return undefined; +} + export function generateCells( settings: IDataScienceSettings | undefined, code: string, diff --git a/src/client/datascience/codeCssGenerator.ts b/src/client/datascience/codeCssGenerator.ts index 05a4eb376baa..edb32c591c5d 100644 --- a/src/client/datascience/codeCssGenerator.ts +++ b/src/client/datascience/codeCssGenerator.ts @@ -10,7 +10,7 @@ import * as path from 'path'; import { IWorkspaceService } from '../common/application/types'; import { traceError, traceInfo, traceWarning } from '../common/logger'; import { IFileSystem } from '../common/platform/types'; -import { IConfigurationService } from '../common/types'; +import { IConfigurationService, Resource } from '../common/types'; import { DefaultTheme } from './constants'; import { ICodeCssGenerator, IThemeFinder } from './types'; @@ -102,15 +102,16 @@ export class CodeCssGenerator implements ICodeCssGenerator { @inject(IFileSystem) private fs: IFileSystem ) {} - public generateThemeCss(isDark: boolean, theme: string): Promise { - return this.applyThemeData(isDark, theme, '', this.generateCss.bind(this)); + public generateThemeCss(resource: Resource, isDark: boolean, theme: string): Promise { + return this.applyThemeData(resource, isDark, theme, '', this.generateCss.bind(this)); } - public generateMonacoTheme(isDark: boolean, theme: string): Promise { - return this.applyThemeData(isDark, theme, {} as any, this.generateMonacoThemeObject.bind(this)); + public generateMonacoTheme(resource: Resource, isDark: boolean, theme: string): Promise { + return this.applyThemeData(resource, isDark, theme, {} as any, this.generateMonacoThemeObject.bind(this)); } private async applyThemeData( + resource: Resource, isDark: boolean, theme: string, defaultT: T, @@ -119,7 +120,7 @@ export class CodeCssGenerator implements ICodeCssGenerator { let result = defaultT; try { // First compute our current theme. - const ignoreTheme = this.configService.getSettings().datascience.ignoreVscodeTheme ? true : false; + const ignoreTheme = this.configService.getSettings(resource).datascience.ignoreVscodeTheme ? true : false; theme = ignoreTheme ? DefaultTheme : theme; const editor = this.workspaceService.getConfiguration('editor', undefined); const fontFamily = editor diff --git a/src/client/datascience/data-viewing/dataViewer.ts b/src/client/datascience/data-viewing/dataViewer.ts index f5c195c1fec8..4efb066f031b 100644 --- a/src/client/datascience/data-viewing/dataViewer.ts +++ b/src/client/datascience/data-viewing/dataViewer.ts @@ -11,7 +11,7 @@ import { IApplicationShell, IWebPanelProvider, IWorkspaceService } from '../../c import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { WebHostNotebook } from '../../common/experimentGroups'; import { traceError } from '../../common/logger'; -import { IConfigurationService, IDisposable, IExperimentsManager } from '../../common/types'; +import { IConfigurationService, IDisposable, IExperimentsManager, Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { StopWatch } from '../../common/utils/stopWatch'; @@ -84,6 +84,10 @@ export class DataViewer extends WebViewHost implements IData } } + protected getOwningResource(): Promise { + return Promise.resolve(undefined); + } + //tslint:disable-next-line:no-any protected onMessage(message: string, payload: any) { switch (message) { diff --git a/src/client/datascience/editor-integration/cellhashprovider.ts b/src/client/datascience/editor-integration/cellhashprovider.ts index bbb0c302831e..0d88be6142ba 100644 --- a/src/client/datascience/editor-integration/cellhashprovider.ts +++ b/src/client/datascience/editor-integration/cellhashprovider.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import { nbformat } from '@jupyterlab/coreutils'; import * as hashjs from 'hash.js'; import { inject, injectable, multiInject, optional } from 'inversify'; import { Event, EventEmitter, Position, Range, TextDocumentChangeEvent, TextDocumentContentChangeEvent } from 'vscode'; @@ -12,6 +11,7 @@ import { traceError, traceInfo } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; import { IConfigurationService } from '../../common/types'; import { noop } from '../../common/utils/misc'; +import { getCellResource } from '../cellFactory'; import { CellMatcher } from '../cellMatcher'; import { Identifiers } from '../constants'; import { InteractiveWindowMessages, SysInfoReason } from '../interactive-common/interactiveWindowTypes'; @@ -106,7 +106,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi try { if (!silent) { // Don't log empty cells - const stripped = this.extractExecutableLines(cell.data.source); + const stripped = this.extractExecutableLines(cell); if (stripped.length > 0 && stripped.find(s => s.trim().length > 0)) { // When the user adds new code, we know the execution count is increasing this.executionCount += 1; @@ -139,9 +139,9 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi } } - private extractExecutableLines(code: nbformat.MultilineString): string[] { - const cellMatcher = new CellMatcher(this.configService.getSettings().datascience); - const lines = splitMultilineString(code); + private extractExecutableLines(cell: ICell): string[] { + const cellMatcher = new CellMatcher(this.configService.getSettings(getCellResource(cell)).datascience); + const lines = splitMultilineString(cell.data.source); // Only strip this off the first line. Otherwise we want the markers in the code. if (lines.length > 0 && (cellMatcher.isCode(lines[0]) || cellMatcher.isMarkdown(lines[0]))) { return lines.slice(1); @@ -193,7 +193,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi if (doc) { // Compute the code that will really be sent to jupyter const lines = splitMultilineString(cell.data.source); - const stripped = this.extractExecutableLines(cell.data.source); + const stripped = this.extractExecutableLines(cell); // Figure out our true 'start' line. This is what we need to tell the debugger is // actually the start of the code as that's what Jupyter will be getting. @@ -292,7 +292,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi ): number { if ( this.debugService.activeDebugSession && - this.configService.getSettings().datascience.stopOnFirstLineWhileDebugging + this.configService.getSettings(getCellResource(cell)).datascience.stopOnFirstLineWhileDebugging ) { // Inject the breakpoint line source.splice(0, 0, 'breakpoint()\n'); diff --git a/src/client/datascience/editor-integration/codeLensFactory.ts b/src/client/datascience/editor-integration/codeLensFactory.ts index f7e5341dd560..7522e89021bf 100644 --- a/src/client/datascience/editor-integration/codeLensFactory.ts +++ b/src/client/datascience/editor-integration/codeLensFactory.ts @@ -6,7 +6,7 @@ import { CodeLens, Command, Event, EventEmitter, Range, TextDocument } from 'vsc import { traceWarning } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService } from '../../common/types'; +import { IConfigurationService, Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { generateCellRangesFromDocument } from '../cellFactory'; @@ -67,9 +67,12 @@ export class CodeLensFactory implements ICodeLensFactory, IInteractiveWindowList } public createCodeLenses(document: TextDocument): CodeLens[] { - const ranges = generateCellRangesFromDocument(document, this.configService.getSettings().datascience); - const commands = this.enumerateCommands(); - const hashes = this.configService.getSettings().datascience.addGotoCodeLenses + const ranges = generateCellRangesFromDocument( + document, + this.configService.getSettings(document.uri).datascience + ); + const commands = this.enumerateCommands(document.uri); + const hashes = this.configService.getSettings(document.uri).datascience.addGotoCodeLenses ? this.hashProvider.getHashes() : []; const codeLenses: CodeLens[] = []; @@ -89,10 +92,10 @@ export class CodeLensFactory implements ICodeLensFactory, IInteractiveWindowList return codeLenses; } - private enumerateCommands(): string[] { + private enumerateCommands(resource: Resource): string[] { let fullCommandList: string[]; // Add our non-debug commands - const commands = this.configService.getSettings().datascience.codeLenses; + const commands = this.configService.getSettings(resource).datascience.codeLenses; if (commands) { fullCommandList = commands.split(',').map(s => s.trim()); } else { @@ -100,7 +103,7 @@ export class CodeLensFactory implements ICodeLensFactory, IInteractiveWindowList } // Add our debug commands - const debugCommands = this.configService.getSettings().datascience.debugCodeLenses; + const debugCommands = this.configService.getSettings(resource).datascience.debugCodeLenses; if (debugCommands) { fullCommandList = fullCommandList.concat(debugCommands.split(',').map(s => s.trim())); } else { diff --git a/src/client/datascience/editor-integration/codelensprovider.ts b/src/client/datascience/editor-integration/codelensprovider.ts index 989a811e232d..05089f8ffd71 100644 --- a/src/client/datascience/editor-integration/codelensprovider.ts +++ b/src/client/datascience/editor-integration/codelensprovider.ts @@ -59,7 +59,11 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider // IDataScienceCodeLensProvider interface public getCodeWatcher(document: vscode.TextDocument): ICodeWatcher | undefined { - return this.matchWatcher(document.fileName, document.version, this.configuration.getSettings().datascience); + return this.matchWatcher( + document.fileName, + document.version, + this.configuration.getSettings(document.uri).datascience + ); } private onDebugLocationUpdated() { @@ -90,7 +94,7 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider editorContext.set(result && result.length > 0).catch(); // Don't provide any code lenses if we have not enabled data science - const settings = this.configuration.getSettings(); + const settings = this.configuration.getSettings(document.uri); if (!settings.datascience.enabled || !settings.datascience.enableCellCodeLens) { // Clear out any existing code watchers, providecodelenses is called on settings change // so we don't need to watch the settings change specifically here @@ -143,7 +147,7 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider const codeWatcher: ICodeWatcher | undefined = this.matchWatcher( document.fileName, document.version, - this.configuration.getSettings().datascience + this.configuration.getSettings(document.uri).datascience ); if (codeWatcher) { return codeWatcher.getCodeLenses(); diff --git a/src/client/datascience/editor-integration/codewatcher.ts b/src/client/datascience/editor-integration/codewatcher.ts index b39baaf3efe3..211c8346f9b9 100644 --- a/src/client/datascience/editor-integration/codewatcher.ts +++ b/src/client/datascience/editor-integration/codewatcher.ts @@ -16,7 +16,7 @@ import { import { IDocumentManager } from '../../common/application/types'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, IDataScienceSettings } from '../../common/types'; +import { IConfigurationService, IDataScienceSettings, Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { StopWatch } from '../../common/utils/stopWatch'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; @@ -53,7 +53,7 @@ export class CodeWatcher implements ICodeWatcher { this.version = document.version; // Get document cells here. Make a copy of our settings. - this.cachedSettings = JSON.parse(JSON.stringify(this.configService.getSettings().datascience)); + this.cachedSettings = JSON.parse(JSON.stringify(this.configService.getSettings(document.uri).datascience)); // Use the factory to generate our new code lenses. this.codeLenses = this.codeLensFactory.createCodeLenses(document); @@ -269,7 +269,8 @@ export class CodeWatcher implements ICodeWatcher { // Run the cell clicked. Advance if the cursor is inside this cell and we're allowed to const advance = range.contains(this.documentManager.activeTextEditor.selection.start) && - this.configService.getSettings().datascience.enableAutoMoveToNextCell; + this.configService.getSettings(this.documentManager.activeTextEditor.document.uri).datascience + .enableAutoMoveToNextCell; return this.runMatchingCell(range, advance); } @@ -305,7 +306,7 @@ export class CodeWatcher implements ICodeWatcher { public async addEmptyCellToBottom(): Promise { const editor = this.documentManager.activeTextEditor; - const cellDelineator = this.defaultCellMarker; + const cellDelineator = this.getDefaultCellMarker(editor?.document.uri); if (editor) { editor.edit(editBuilder => { editBuilder.insert(new Position(editor.document.lineCount, 0), `\n\n${cellDelineator}\n`); @@ -324,7 +325,7 @@ export class CodeWatcher implements ICodeWatcher { const editor = this.documentManager.activeTextEditor; const cellMatcher = new CellMatcher(); let index = 0; - const cellDelineator = this.defaultCellMarker; + const cellDelineator = this.getDefaultCellMarker(editor.document.uri); if (editor) { editor.edit(editBuilder => { @@ -353,8 +354,10 @@ export class CodeWatcher implements ICodeWatcher { ); } - private get defaultCellMarker(): string { - return this.configService.getSettings().datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker; + private getDefaultCellMarker(resource: Resource): string { + return ( + this.configService.getSettings(resource).datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker + ); } private onCodeLensFactoryUpdated(): void { @@ -482,7 +485,10 @@ export class CodeWatcher implements ICodeWatcher { if (editor) { editor.edit(editBuilder => { - editBuilder.insert(new Position(currentRange.end.line + 1, 0), `\n\n${this.defaultCellMarker}\n`); + editBuilder.insert( + new Position(currentRange.end.line + 1, 0), + `\n\n${this.getDefaultCellMarker(editor.document.uri)}\n` + ); }); } diff --git a/src/client/datascience/editor-integration/decorator.ts b/src/client/datascience/editor-integration/decorator.ts index b80c4e67f921..9ebab5381afb 100644 --- a/src/client/datascience/editor-integration/decorator.ts +++ b/src/client/datascience/editor-integration/decorator.ts @@ -104,13 +104,10 @@ export class Decorator implements IExtensionSingleActivationService, IDisposable this.cellSeparatorType && this.activeCellBottom ) { - const settings = this.configuration.getSettings().datascience; + const settings = this.configuration.getSettings(editor.document.uri).datascience; if (settings.decorateCells && settings.enabled) { // Find all of the cells - const cells = generateCellRangesFromDocument( - editor.document, - this.configuration.getSettings().datascience - ); + const cells = generateCellRangesFromDocument(editor.document, settings); // Find the range for our active cell. const currentRange = cells.map(c => c.range).filter(r => r.contains(editor.selection.anchor)); diff --git a/src/client/datascience/gather/gatherListener.ts b/src/client/datascience/gather/gatherListener.ts index 612897d32bc9..a0026dbf2ec1 100644 --- a/src/client/datascience/gather/gatherListener.ts +++ b/src/client/datascience/gather/gatherListener.ts @@ -96,7 +96,7 @@ export class GatherListener implements IInteractiveWindowListener { // First get the active server const activeServer = await this.jupyterExecution.getServer( - await this.interactiveWindowProvider.getNotebookOptions() + await this.interactiveWindowProvider.getNotebookOptions(this.notebookUri) ); let nb: INotebook | undefined; diff --git a/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts b/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts index ba1da07710d5..a1156d870361 100644 --- a/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts +++ b/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts @@ -681,7 +681,7 @@ export class IntellisenseProvider implements IInteractiveWindowListener { private async getNotebook(): Promise { // First get the active server const activeServer = await this.jupyterExecution.getServer( - await this.interactiveWindowProvider.getNotebookOptions() + await this.interactiveWindowProvider.getNotebookOptions(this.potentialResource) ); // If that works, see if there's a matching notebook running diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 7f2229daf264..e0cbe55c566a 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -359,7 +359,7 @@ export abstract class InteractiveBase extends WebViewHost { if (this._notebook && !this.restartingKernel) { - if (this.shouldAskForRestart()) { + if (await this.shouldAskForRestart()) { // Ask the user if they want us to restart or not. const message = localize.DataScience.restartKernelMessage(); const yes = localize.DataScience.restartKernelMessageYes(); @@ -368,7 +368,7 @@ export abstract class InteractiveBase extends WebViewHost; - protected onViewStateChanged(args: WebViewViewChangeEventArgs) { // Only activate if the active editor is empty. This means that // vscode thinks we are actually supposed to have focus. It would be @@ -505,7 +503,7 @@ export abstract class InteractiveBase extends WebViewHost { + async (cells: ICell[]) => { // Combine the cell data with the possible input data (so we don't lose anything that might have already been in the cells) const combined = cells.map(this.combineData.bind(undefined, data)); @@ -586,7 +584,7 @@ export abstract class InteractiveBase extends WebViewHost c.state === CellState.error) === undefined; } }, @@ -694,7 +692,7 @@ export abstract class InteractiveBase extends WebViewHost { + const settings = this.configuration.getSettings(await this.getOwningResource()); return settings && settings.datascience && settings.datascience.askForKernelRestart === true; } - private disableAskForRestart() { - const settings = this.configuration.getSettings(); + private async disableAskForRestart(): Promise { + const settings = this.configuration.getSettings(await this.getOwningResource()); if (settings && settings.datascience) { settings.datascience.askForKernelRestart = false; this.configuration @@ -915,13 +913,13 @@ export abstract class InteractiveBase extends WebViewHost { + const settings = this.configuration.getSettings(await this.getOwningResource()); return settings && settings.datascience && settings.datascience.askForLargeDataFrames === true; } - private disableAskForLargeData() { - const settings = this.configuration.getSettings(); + private async disableAskForLargeData(): Promise { + const settings = this.configuration.getSettings(await this.getOwningResource()); if (settings && settings.datascience) { settings.datascience.askForLargeDataFrames = false; this.configuration @@ -931,7 +929,7 @@ export abstract class InteractiveBase extends WebViewHost { - if (columnSize > ColumnWarningSize && this.shouldAskForLargeData()) { + if (columnSize > ColumnWarningSize && (await this.shouldAskForLargeData())) { const message = localize.DataScience.tooManyColumnsMessage(); const yes = localize.DataScience.tooManyColumnsYes(); const no = localize.DataScience.tooManyColumnsNo(); @@ -939,7 +937,7 @@ export abstract class InteractiveBase extends WebViewHost { @@ -1215,12 +1215,16 @@ export abstract class InteractiveBase extends WebViewHost 0; const line = editor.selection.start.line; const revealLine = line + 1; const defaultCellMarker = - this.configService.getSettings().datascience.defaultCellMarker || Identifiers.DefaultCodeCellMarker; + this.configService.getSettings(await this.getOwningResource()).datascience.defaultCellMarker || + Identifiers.DefaultCodeCellMarker; let newCode = `${source}${os.EOL}`; if (hasCellsAlready) { // See if inside of a range or not. diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 31cad22f3017..0ed52b2efa75 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -31,6 +31,7 @@ import { IExperimentsManager, IExtensionContext, IMemento, + Resource, WORKSPACE_MEMENTO } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; @@ -54,12 +55,12 @@ import { IInsertCell, INativeCommand, InteractiveWindowMessages, + IReExecuteCell, IRemoveCell, ISaveAll, ISubmitNewCell, ISwapCells, - SysInfoReason, - IReExecuteCell + SysInfoReason } from '../interactive-common/interactiveWindowTypes'; import { InvalidNotebookFileError } from '../jupyter/invalidNotebookFileError'; import { ProgressReporter } from '../progress/progressReporter'; @@ -328,7 +329,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } public async getNotebookOptions(): Promise { - const options = await this.ipynbProvider.getNotebookOptions(); + const options = await this.ipynbProvider.getNotebookOptions(await this.getOwningResource()); await this.contentsLoadedPromise.promise; const metadata = this.notebookJson.metadata; return { @@ -356,7 +357,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this.setDirty(); } - public getNotebookResource(): Promise { + public getOwningResource(): Promise { // Resource to use for loading and our identity are the same. return this.getNotebookIdentity(); } @@ -885,7 +886,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Restart our kernel so that execution counts are reset let oldAsk: boolean | undefined = false; - const settings = this.configuration.getSettings(); + const settings = this.configuration.getSettings(await this.getOwningResource()); if (settings && settings.datascience) { oldAsk = settings.datascience.askForKernelRestart; settings.datascience.askForKernelRestart = false; diff --git a/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts b/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts index 17475c52dbea..343cdd940a5f 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts @@ -12,7 +12,8 @@ import { IAsyncDisposable, IAsyncDisposableRegistry, IConfigurationService, - IDisposableRegistry + IDisposableRegistry, + Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; @@ -136,8 +137,8 @@ export class NativeEditorProvider implements INotebookEditorProvider, IAsyncDisp } } - public async getNotebookOptions(): Promise { - const settings = this.configuration.getSettings(); + public async getNotebookOptions(resource: Resource): Promise { + const settings = this.configuration.getSettings(resource); let serverURI: string | undefined = settings.datascience.jupyterServerURI; const useDefaultConfig: boolean | undefined = settings.datascience.useDefaultConfigForJupyter; diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index 0f788ebbfea6..7212626118d2 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -22,7 +22,8 @@ import { IDisposableRegistry, IExperimentsManager, IMemento, - IPersistentStateFactory + IPersistentStateFactory, + Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { EXTENSION_ROOT_DIR } from '../../constants'; @@ -261,7 +262,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi this.postMessage(InteractiveWindowMessages.ScrollToCell, { id }).ignoreErrors(); } - public async getNotebookResource(): Promise { + protected async getOwningResource(): Promise { if (this.lastFile) { return Uri.file(this.lastFile); } @@ -269,7 +270,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi if (root) { return Uri.file(root); } - return Uri.file('./'); + return undefined; } protected async onViewStateChanged(args: WebViewViewChangeEventArgs) { super.onViewStateChanged(args); @@ -301,8 +302,8 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi } } - protected getNotebookOptions(): Promise { - return this.interactiveWindowProvider.getNotebookOptions(); + protected async getNotebookOptions(): Promise { + return this.interactiveWindowProvider.getNotebookOptions(await this.getOwningResource()); } protected async getNotebookIdentity(): Promise { diff --git a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts index 6fe3dce56529..a056fdb17f9d 100644 --- a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts +++ b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts @@ -177,7 +177,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList if (activeEditor && this.fileSystem.arePathsSame(activeEditor.document.fileName, file)) { const cells = generateCellsFromDocument( activeEditor.document, - this.configuration.getSettings().datascience + this.configuration.getSettings(activeEditor.document.uri).datascience ); if (cells) { const filtersKey = localize.DataScience.exportDialogFilter(); @@ -193,7 +193,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList async () => { if (uri) { let directoryChange; - const settings = this.configuration.getSettings(); + const settings = this.configuration.getSettings(activeEditor.document.uri); if (settings.datascience.changeDirOnImportExport) { directoryChange = uri.fsPath; } @@ -316,7 +316,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList ): Promise { let server: INotebookServer | undefined; try { - const settings = this.configuration.getSettings(); + const settings = this.configuration.getSettings(document.uri); const useDefaultConfig: boolean | undefined = settings.datascience.useDefaultConfigForJupyter; // Try starting a server. Purpose should be unique so we diff --git a/src/client/datascience/interactive-window/interactiveWindowProvider.ts b/src/client/datascience/interactive-window/interactiveWindowProvider.ts index 5e368900fff0..641d8b3c4093 100644 --- a/src/client/datascience/interactive-window/interactiveWindowProvider.ts +++ b/src/client/datascience/interactive-window/interactiveWindowProvider.ts @@ -11,7 +11,8 @@ import { IAsyncDisposable, IAsyncDisposableRegistry, IConfigurationService, - IDisposableRegistry + IDisposableRegistry, + Resource } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import * as localize from '../../common/utils/localize'; @@ -89,9 +90,9 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA throw new Error(localize.DataScience.pythonInteractiveCreateFailed()); } - public async getNotebookOptions(): Promise { + public async getNotebookOptions(resource: Resource): Promise { // Find the settings that we are going to launch our server with - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(resource); let serverURI: string | undefined = settings.datascience.jupyterServerURI; const useDefaultConfig: boolean | undefined = settings.datascience.useDefaultConfigForJupyter; diff --git a/src/client/datascience/jupyter/commandLineSelector.ts b/src/client/datascience/jupyter/commandLineSelector.ts index ad30367a821f..9b8c8dfc1b8b 100644 --- a/src/client/datascience/jupyter/commandLineSelector.ts +++ b/src/client/datascience/jupyter/commandLineSelector.ts @@ -6,7 +6,7 @@ import { inject, injectable } from 'inversify'; // tslint:disable-next-line: import-name import parseArgsStringToArgv from 'string-argv'; -import { ConfigurationChangeEvent, ConfigurationTarget, QuickPickItem } from 'vscode'; +import { ConfigurationChangeEvent, ConfigurationTarget, QuickPickItem, Uri } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types'; import { IConfigurationService } from '../../common/types'; import { DataScience } from '../../common/utils/localize'; @@ -34,9 +34,9 @@ export class JupyterCommandLineSelector { } @captureTelemetry(Telemetry.SelectJupyterURI) - public selectJupyterCommandLine(): Promise { + public selectJupyterCommandLine(file: Uri): Promise { const multiStep = this.multiStepFactory.create<{}>(); - return multiStep.run(this.startSelectingCommandLine.bind(this), {}); + return multiStep.run(this.startSelectingCommandLine.bind(this, file), {}); } private async onDidChangeConfiguration(e: ConfigurationChangeEvent) { @@ -52,7 +52,11 @@ export class JupyterCommandLineSelector { } } - private async startSelectingCommandLine(input: IMultiStepInput<{}>, _state: {}): Promise | void> { + private async startSelectingCommandLine( + file: Uri, + input: IMultiStepInput<{}>, + _state: {} + ): Promise | void> { // First step, show a quick pick to choose either the custom or the default. // newChoice element will be set if the user picked 'enter a new server' const item = await input.showQuickPick>({ @@ -63,14 +67,18 @@ export class JupyterCommandLineSelector { if (item.label === this.defaultLabel) { await this.setJupyterCommandLine(''); } else { - return this.selectCustomCommandLine.bind(this); + return this.selectCustomCommandLine.bind(this, file); } } - private async selectCustomCommandLine(input: IMultiStepInput<{}>, _state: {}): Promise | void> { + private async selectCustomCommandLine( + file: Uri, + input: IMultiStepInput<{}>, + _state: {} + ): Promise | void> { // Ask the user to enter a command line const result = await input.showInputBox({ title: DataScience.jupyterCommandLinePrompt(), - value: this.configuration.getSettings().datascience.jupyterCommandLineArguments.join(' '), + value: this.configuration.getSettings(file).datascience.jupyterCommandLineArguments.join(' '), validate: this.validate, prompt: '' }); diff --git a/src/client/datascience/jupyter/jupyterDebugger.ts b/src/client/datascience/jupyter/jupyterDebugger.ts index 31a92e14fb23..13de4068d516 100644 --- a/src/client/datascience/jupyter/jupyterDebugger.ts +++ b/src/client/datascience/jupyter/jupyterDebugger.ts @@ -16,6 +16,7 @@ import { IConfigurationService, IExperimentsManager, Version } from '../../commo import * as localize from '../../common/utils/localize'; import { EXTENSION_ROOT_DIR } from '../../constants'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; +import { getCellResource } from '../cellFactory'; import { Identifiers, Telemetry } from '../constants'; import { CellState, @@ -144,7 +145,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { // If we already have configuration, we're already attached, don't do it again. let result = this.configs.get(notebook.identity.toString()); if (result) { - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(notebook.resource); result.justMyCode = settings.datascience.debugJustMyCode; return result; } @@ -207,7 +208,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { // Add the settings path first as it takes precedence over the ptvsd extension path // tslint:disable-next-line:no-multiline-string - let settingsPath = this.configService.getSettings().datascience.ptvsdDistPath; + let settingsPath = this.configService.getSettings(notebook.resource).datascience.ptvsdDistPath; // Escape windows path chars so they end up in the source escaped if (settingsPath) { if (this.platform.isWindows) { @@ -407,7 +408,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { const debugInfoRegEx = /\('(.*?)', ([0-9]*)\)/; const debugInfoMatch = debugInfoRegEx.exec(enableAttachString); - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(getCellResource(cells[0])); if (debugInfoMatch) { const localConfig: DebugConfiguration = { name: 'IPython', diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts index b386935bfbfa..52fc6259f798 100644 --- a/src/client/datascience/jupyter/jupyterNotebook.ts +++ b/src/client/datascience/jupyter/jupyterNotebook.ts @@ -251,7 +251,7 @@ export class JupyterNotebookBase implements INotebook { // When we start our notebook initial, change to our workspace or user specified root directory await this.updateWorkingDirectory(); - const settings = this.configService.getSettings().datascience; + const settings = this.configService.getSettings(this.resource).datascience; if (settings && settings.themeMatplotlibPlots) { // We're theming matplotlibs, so we have to setup our default state. await this.initializeMatplotlib(cancelToken); @@ -533,7 +533,7 @@ export class JupyterNotebookBase implements INotebook { await this.initializeMatplotlib(); } - const settings = this.configService.getSettings().datascience; + const settings = this.configService.getSettings(this.resource).datascience; if (settings.themeMatplotlibPlots && !settings.ignoreVscodeTheme) { // Reset the matplotlib style based on if dark or not. await this.executeSilently( @@ -624,7 +624,7 @@ export class JupyterNotebookBase implements INotebook { } private async initializeMatplotlib(cancelToken?: CancellationToken): Promise { - const settings = this.configService.getSettings().datascience; + const settings = this.configService.getSettings(this.resource).datascience; if (settings && settings.themeMatplotlibPlots) { const matplobInit = !settings || settings.enablePlotViewer @@ -710,7 +710,14 @@ export class JupyterNotebookBase implements INotebook { // If we have a session, execute the code now. if (this.session) { // Generate our cells ahead of time - const cells = generateCells(this.configService.getSettings().datascience, code, file, line, true, id); + const cells = generateCells( + this.configService.getSettings(this.resource).datascience, + code, + file, + line, + true, + id + ); // Might have more than one (markdown might be split) if (cells.length > 1) { @@ -744,7 +751,7 @@ export class JupyterNotebookBase implements INotebook { ): Kernel.IShellFuture | undefined => { //traceInfo(`Executing code in jupyter : ${code}`); try { - const cellMatcher = new CellMatcher(this.configService.getSettings().datascience); + const cellMatcher = new CellMatcher(this.configService.getSettings(this.resource).datascience); return this.session ? this.session.requestExecute( { @@ -1256,7 +1263,7 @@ export class JupyterNotebookBase implements INotebook { cell.state = CellState.error; // In the error scenario, we want to stop all other pending cells. - if (this.configService.getSettings().datascience.stopOnError) { + if (this.configService.getSettings(this.resource).datascience.stopOnError) { this.pendingCellSubscriptions.forEach(c => { if (c.cell.id !== cell.id) { c.cancel(); @@ -1268,7 +1275,7 @@ export class JupyterNotebookBase implements INotebook { // We have a set limit for the number of output text characters that we display by default // trim down strings to that limit, assuming at this point we have compressed down to a single string private trimOutput(outputString: string): string { - const outputLimit = this.configService.getSettings().datascience.textOutputLimit; + const outputLimit = this.configService.getSettings(this.resource).datascience.textOutputLimit; if (!outputLimit || outputLimit === 0 || outputString.length <= outputLimit) { return outputString; diff --git a/src/client/datascience/jupyter/jupyterVariables.ts b/src/client/datascience/jupyter/jupyterVariables.ts index 8f11d7596038..a765f1cd2c45 100644 --- a/src/client/datascience/jupyter/jupyterVariables.ts +++ b/src/client/datascience/jupyter/jupyterVariables.ts @@ -222,7 +222,9 @@ export class JupyterVariables implements IJupyterVariables { // We may have cached this information let result = this.languageToQueryMap.get(language); if (!result) { - let query = this.configService.getSettings().datascience.variableQueries.find(v => v.language === language); + let query = this.configService + .getSettings(notebook.resource) + .datascience.variableQueries.find(v => v.language === language); if (!query) { query = Settings.DefaultVariableQuery; } @@ -280,7 +282,7 @@ export class JupyterVariables implements IJupyterVariables { }; } - const exclusionList = this.configService.getSettings().datascience.variableExplorerExclude + const exclusionList = this.configService.getSettings(notebook.resource).datascience.variableExplorerExclude ? this.configService.getSettings().datascience.variableExplorerExclude?.split(';') : []; diff --git a/src/client/datascience/jupyter/kernels/kernelSwitcher.ts b/src/client/datascience/jupyter/kernels/kernelSwitcher.ts index 98fe4ae0c403..817b79de876d 100644 --- a/src/client/datascience/jupyter/kernels/kernelSwitcher.ts +++ b/src/client/datascience/jupyter/kernels/kernelSwitcher.ts @@ -34,7 +34,7 @@ export class KernelSwitcher { private async selectJupyterKernel(notebook: INotebook): Promise { let kernel: KernelSpecInterpreter | undefined; - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(notebook.resource); const isLocalConnection = notebook.server.getConnectionInfo()?.localLaunch ?? settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch; @@ -68,7 +68,7 @@ export class KernelSwitcher { return this.kernelSelector.selectRemoteKernel(resource, stopWatch, session, undefined, currentKernel); } private async switchKernelWithRetry(notebook: INotebook, kernel: KernelSpecInterpreter): Promise { - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(notebook.resource); const isLocalConnection = notebook.server.getConnectionInfo()?.localLaunch ?? settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch; diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts index 8406f4c14e0a..52d0294ef9b9 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts @@ -166,7 +166,7 @@ export class GuestJupyterNotebook } public async interruptKernel(_timeoutMs: number): Promise { - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(this.resource); const interruptTimeout = settings.datascience.jupyterInterruptTimeout; const response = await this.sendRequest(LiveShareCommands.interrupt, [interruptTimeout]); diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index da42600d63ff..8cd714a7bf19 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -183,7 +183,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas if (changedKernel && possibleSession && info.kernelSpec) { await possibleSession.changeKernel( info.kernelSpec, - this.configService.getSettings().datascience.jupyterLaunchTimeout + this.configService.getSettings(resource).datascience.jupyterLaunchTimeout ); } diff --git a/src/client/datascience/jupyter/serverPreload.ts b/src/client/datascience/jupyter/serverPreload.ts index 779de06e7b68..e58b3caee8ab 100644 --- a/src/client/datascience/jupyter/serverPreload.ts +++ b/src/client/datascience/jupyter/serverPreload.ts @@ -52,7 +52,7 @@ export class ServerPreload implements IExtensionSingleActivationService { private async createServerIfNecessary() { try { traceInfo(`Attempting to start a server because of preload conditions ...`); - const options = await this.interactiveProvider.getNotebookOptions(); + const options = await this.interactiveProvider.getNotebookOptions(undefined); // Turn off any UI display const optionsCopy = { ...options }; @@ -62,7 +62,11 @@ export class ServerPreload implements IExtensionSingleActivationService { let server = await this.execution.getServer(optionsCopy); // If it didn't start, attempt for local and if allowed. - if (!server && !optionsCopy.uri && !this.configService.getSettings().datascience.disableJupyterAutoStart) { + if ( + !server && + !optionsCopy.uri && + !this.configService.getSettings(undefined).datascience.disableJupyterAutoStart + ) { // Local case, try creating one server = await this.execution.connectToNotebookServer(optionsCopy); } diff --git a/src/client/datascience/plotting/plotViewer.ts b/src/client/datascience/plotting/plotViewer.ts index e2e700984db0..d4619acbbe2a 100644 --- a/src/client/datascience/plotting/plotViewer.ts +++ b/src/client/datascience/plotting/plotViewer.ts @@ -14,7 +14,7 @@ import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { WebHostNotebook } from '../../common/experimentGroups'; import { traceError } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, IDisposable, IExperimentsManager } from '../../common/types'; +import { IConfigurationService, IDisposable, IExperimentsManager, Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { ICodeCssGenerator, IPlotViewer, IThemeFinder } from '../types'; import { WebViewHost } from '../webViewHost'; @@ -86,6 +86,10 @@ export class PlotViewer extends WebViewHost implements IPlot } } + protected getOwningResource(): Promise { + return Promise.resolve(undefined); + } + //tslint:disable-next-line:no-any protected onMessage(message: string, payload: any) { switch (message) { diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index fcb6c8671fb3..3f17a5caed45 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -303,7 +303,7 @@ export interface IInteractiveWindowProvider { onExecutedCode: Event; getActive(): IInteractiveWindow | undefined; getOrCreateActive(): Promise; - getNotebookOptions(): Promise; + getNotebookOptions(resource: Resource): Promise; } export const IDataScienceErrorHandler = Symbol('IDataScienceErrorHandler'); @@ -361,7 +361,7 @@ export interface INotebookEditorProvider { open(file: Uri, contents: string): Promise; show(file: Uri): Promise; createNew(contents?: string): Promise; - getNotebookOptions(): Promise; + getNotebookOptions(resource: Resource): Promise; } // For native editing, the INotebookEditor acts like a TextEditor and a TextDocument together @@ -489,8 +489,8 @@ export interface IMessageCell extends nbformat.IBaseCell { export const ICodeCssGenerator = Symbol('ICodeCssGenerator'); export interface ICodeCssGenerator { - generateThemeCss(isDark: boolean, theme: string): Promise; - generateMonacoTheme(isDark: boolean, theme: string): Promise; + generateThemeCss(resource: Resource, isDark: boolean, theme: string): Promise; + generateMonacoTheme(resource: Resource, isDark: boolean, theme: string): Promise; } export const IThemeFinder = Symbol('IThemeFinder'); diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index e0ae2244ec5d..1ce8b4eff77c 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -8,7 +8,7 @@ import { ConfigurationChangeEvent, ViewColumn, WorkspaceConfiguration } from 'vs import { IWebPanel, IWebPanelMessageListener, IWebPanelProvider, IWorkspaceService } from '../common/application/types'; import { traceInfo, traceWarning } from '../common/logger'; -import { IConfigurationService, IDisposable } from '../common/types'; +import { IConfigurationService, IDisposable, Resource } from '../common/types'; import { createDeferred, Deferred } from '../common/utils/async'; import * as localize from '../common/utils/localize'; import { noop } from '../common/utils/misc'; @@ -19,7 +19,7 @@ import { CssMessages, IGetCssRequest, IGetMonacoThemeRequest, SharedMessages } f import { ICodeCssGenerator, IDataScienceExtraSettings, IThemeFinder, WebViewViewChangeEventArgs } from './types'; @injectable() // For some reason this is necessary to get the class hierarchy to work. -export class WebViewHost implements IDisposable { +export abstract class WebViewHost implements IDisposable { protected viewState: { visible: boolean; active: boolean } = { visible: false, active: false }; private disposed: boolean = false; private webPanel: IWebPanel | undefined; @@ -120,6 +120,8 @@ export class WebViewHost implements IDisposable { } } + protected abstract getOwningResource(): Promise; + protected get isDisposed(): boolean { return this.disposed; } @@ -169,12 +171,13 @@ export class WebViewHost implements IDisposable { } } - protected generateDataScienceExtraSettings(): IDataScienceExtraSettings { + protected async generateDataScienceExtraSettings(): Promise { + const resource = await this.getOwningResource(); const editor = this.workspaceService.getConfiguration('editor'); const workbench = this.workspaceService.getConfiguration('workbench'); const theme = !workbench ? DefaultTheme : workbench.get('colorTheme', DefaultTheme); return { - ...this.configService.getSettings().datascience, + ...this.configService.getSettings(resource).datascience, extraSettings: { editor: { cursor: this.getValue(editor, 'cursorStyle', 'line'), @@ -230,9 +233,11 @@ export class WebViewHost implements IDisposable { // Create our web panel (it's the UI that shows up for the history) if (this.webPanel === undefined) { + const resource = await this.getOwningResource(); + // Get our settings to pass along to the react control - const settings = this.generateDataScienceExtraSettings(); - const insiders = this.configService.getSettings().insidersChannel; + const settings = await this.generateDataScienceExtraSettings(); + const insiders = this.configService.getSettings(resource).insidersChannel; traceInfo('Loading web view...'); @@ -285,13 +290,14 @@ export class WebViewHost implements IDisposable { @captureTelemetry(Telemetry.WebviewStyleUpdate) private async handleCssRequest(request: IGetCssRequest): Promise { - const settings = this.generateDataScienceExtraSettings(); + const settings = await this.generateDataScienceExtraSettings(); const requestIsDark = settings.ignoreVscodeTheme ? false : request.isDark; this.setTheme(requestIsDark); const isDark = settings.ignoreVscodeTheme ? false : await this.themeFinder.isThemeDark(settings.extraSettings.theme); - const css = await this.cssGenerator.generateThemeCss(requestIsDark, settings.extraSettings.theme); + const resource = await this.getOwningResource(); + const css = await this.cssGenerator.generateThemeCss(resource, requestIsDark, settings.extraSettings.theme); return this.postMessageInternal(CssMessages.GetCssResponse, { css, theme: settings.extraSettings.theme, @@ -301,10 +307,11 @@ export class WebViewHost implements IDisposable { @captureTelemetry(Telemetry.WebviewMonacoStyleUpdate) private async handleMonacoThemeRequest(request: IGetMonacoThemeRequest): Promise { - const settings = this.generateDataScienceExtraSettings(); + const settings = await this.generateDataScienceExtraSettings(); const isDark = settings.ignoreVscodeTheme ? false : request.isDark; this.setTheme(isDark); - const monacoTheme = await this.cssGenerator.generateMonacoTheme(isDark, settings.extraSettings.theme); + const resource = await this.getOwningResource(); + const monacoTheme = await this.cssGenerator.generateMonacoTheme(resource, isDark, settings.extraSettings.theme); return this.postMessageInternal(CssMessages.GetMonacoThemeResponse, { theme: monacoTheme }); } diff --git a/src/test/datascience/color.test.ts b/src/test/datascience/color.test.ts index 6b85571f249e..8df97088ebe8 100644 --- a/src/test/datascience/color.test.ts +++ b/src/test/datascience/color.test.ts @@ -116,9 +116,9 @@ suite('Theme colors', () => { .returns((_s, d) => { return d; }); - const theme = await cssGenerator.generateMonacoTheme(isDark, themeName); + const theme = await cssGenerator.generateMonacoTheme(undefined, isDark, themeName); assert.ok(theme, `Cannot find monaco theme for ${themeName}`); - const colors = await cssGenerator.generateThemeCss(isDark, themeName); + const colors = await cssGenerator.generateThemeCss(undefined, isDark, themeName); assert.ok(colors, 'Cannot find theme colors for Kimbie Dark'); // Make sure we have a string value that is not set to a variable @@ -164,7 +164,7 @@ suite('Theme colors', () => { const fs = new FileSystem(); cssGenerator = new CodeCssGenerator(workspaceService.object, mockThemeFinder.object, configService.object, fs); - const colors = await cssGenerator.generateThemeCss(false, 'Kimbie Dark'); + const colors = await cssGenerator.generateThemeCss(undefined, false, 'Kimbie Dark'); assert.ok(colors, 'Cannot find theme colors for Kimbie Dark'); // Make sure we have a string value that is not set to a variable diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index 09799641ffb3..43879ef0088b 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -99,7 +99,9 @@ suite('DataScience DataViewer tests', () => { async function injectCode(code: string): Promise { const exec = ioc.get(IJupyterExecution); const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); - const server = await exec.connectToNotebookServer(await interactiveWindowProvider.getNotebookOptions()); + const server = await exec.connectToNotebookServer( + await interactiveWindowProvider.getNotebookOptions(undefined) + ); notebook = server ? await server.createNotebook(undefined, Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined; diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index bbdc49175a8f..9172428cc3e5 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -528,7 +528,7 @@ for _ in range(50): await closeNotebook(editor, wrapper); const jupyterExecution = ioc.serviceManager.get(IJupyterExecution); const editorProvider = ioc.serviceManager.get(INotebookEditorProvider); - const server = await jupyterExecution.getServer(await editorProvider.getNotebookOptions()); + const server = await jupyterExecution.getServer(await editorProvider.getNotebookOptions(undefined)); assert.ok(server, 'Server was destroyed on notebook shutdown'); // Reopen, and rerun diff --git a/src/test/datascience/testInteractiveWindowProvider.ts b/src/test/datascience/testInteractiveWindowProvider.ts index 6ec73d19ec2f..6e2811f316ce 100644 --- a/src/test/datascience/testInteractiveWindowProvider.ts +++ b/src/test/datascience/testInteractiveWindowProvider.ts @@ -5,7 +5,12 @@ import { inject, injectable } from 'inversify'; import { Event } from 'vscode'; import { ILiveShareApi } from '../../client/common/application/types'; -import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../client/common/types'; +import { + IAsyncDisposableRegistry, + IConfigurationService, + IDisposableRegistry, + Resource +} from '../../client/common/types'; import { InteractiveWindowMessageListener } from '../../client/datascience/interactive-common/interactiveWindowMessageListener'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { InteractiveWindow } from '../../client/datascience/interactive-window/interactiveWindow'; @@ -67,7 +72,7 @@ export class TestInteractiveWindowProvider implements IInteractiveWindowProvider return this.realProvider.getOrCreateActive(); } - public getNotebookOptions(): Promise { - return this.realProvider.getNotebookOptions(); + public getNotebookOptions(resource: Resource): Promise { + return this.realProvider.getNotebookOptions(resource); } } diff --git a/src/test/datascience/testNativeEditorProvider.ts b/src/test/datascience/testNativeEditorProvider.ts index ff9d2267501a..fadf36c9ca5b 100644 --- a/src/test/datascience/testNativeEditorProvider.ts +++ b/src/test/datascience/testNativeEditorProvider.ts @@ -6,7 +6,12 @@ import { Event, Uri } from 'vscode'; import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; import { IFileSystem } from '../../client/common/platform/types'; -import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../client/common/types'; +import { + IAsyncDisposableRegistry, + IConfigurationService, + IDisposableRegistry, + Resource +} from '../../client/common/types'; import { InteractiveWindowMessageListener } from '../../client/datascience/interactive-common/interactiveWindowMessageListener'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { NativeEditor } from '../../client/datascience/interactive-ipynb/nativeEditor'; @@ -96,7 +101,7 @@ export class TestNativeEditorProvider implements INotebookEditorProvider { return result; } - public async getNotebookOptions(): Promise { - return this.realProvider.getNotebookOptions(); + public async getNotebookOptions(resource: Resource): Promise { + return this.realProvider.getNotebookOptions(resource); } } From 9f420a85ed082feb68267a1154f252daac408ff7 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 18 Feb 2020 14:19:59 -0800 Subject: [PATCH 10/14] Fix unit tests and missing settings --- src/client/datascience/datascience.ts | 2 +- src/client/datascience/editor-integration/decorator.ts | 2 +- src/client/datascience/gather/gather.ts | 4 +++- src/client/datascience/interactive-ipynb/nativeEditor.ts | 4 +++- .../datascience/interactive-ipynb/nativeEditorProvider.ts | 2 +- src/client/datascience/webViewHost.ts | 6 +++--- .../datascience/editor-integration/codewatcher.unit.test.ts | 2 +- src/test/datascience/execution.unit.test.ts | 2 +- .../datascience/interactive-ipynb/nativeEditor.unit.test.ts | 2 +- .../interactive-ipynb/nativeEditorProvider.unit.test.ts | 4 ++-- .../interactiveWindowCommandListener.unit.test.ts | 2 +- src/test/datascience/jupyter/jupyterConnection.unit.test.ts | 2 +- .../datascience/jupyter/kernels/kernelSwitcher.unit.test.ts | 2 +- src/test/datascience/jupyter/serverCache.unit.test.ts | 4 ++-- src/test/datascience/jupyterVariables.unit.test.ts | 2 +- src/test/datascience/mockJupyterManager.ts | 2 +- 16 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts index ac05df177560..5ebec44f9e29 100644 --- a/src/client/datascience/datascience.ts +++ b/src/client/datascience/datascience.ts @@ -47,7 +47,7 @@ export class DataScience implements IDataScience { // Set our initial settings and sign up for changes this.onSettingsChanged(); - this.changeHandler = this.configuration.getSettings().onDidChange(this.onSettingsChanged.bind(this)); + this.changeHandler = this.configuration.getSettings(undefined).onDidChange(this.onSettingsChanged.bind(this)); this.disposableRegistry.push(this); // Listen for active editor changes so we can detect have code cells or not diff --git a/src/client/datascience/editor-integration/decorator.ts b/src/client/datascience/editor-integration/decorator.ts index 9ebab5381afb..339ad1b097f4 100644 --- a/src/client/datascience/editor-integration/decorator.ts +++ b/src/client/datascience/editor-integration/decorator.ts @@ -24,7 +24,7 @@ export class Decorator implements IExtensionSingleActivationService, IDisposable ) { this.computeDecorations(); disposables.push(this); - disposables.push(this.configuration.getSettings().onDidChange(this.settingsChanged, this)); + disposables.push(this.configuration.getSettings(undefined).onDidChange(this.settingsChanged, this)); disposables.push(this.documentManager.onDidChangeActiveTextEditor(this.changedEditor, this)); disposables.push(this.documentManager.onDidChangeTextEditorSelection(this.changedSelection, this)); disposables.push(this.documentManager.onDidChangeTextDocument(this.changedDocument, this)); diff --git a/src/client/datascience/gather/gather.ts b/src/client/datascience/gather/gather.ts index e070aa103f98..0fa4f65fb775 100644 --- a/src/client/datascience/gather/gather.ts +++ b/src/client/datascience/gather/gather.ts @@ -32,7 +32,9 @@ export class GatherExecution implements IGatherExecution { this._executionSlicer = new ExecutionLogSlicer(this.dataflowAnalyzer); if (this._enabled) { - this.disposables.push(this.configService.getSettings().onDidChange(e => this.updateEnableGather(e))); + this.disposables.push( + this.configService.getSettings(undefined).onDidChange(e => this.updateEnableGather(e)) + ); } traceInfo('Gathering tools have been activated'); diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 0ed52b2efa75..6170a8397682 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -490,7 +490,9 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } protected async getNotebookIdentity(): Promise { - await this.loadedPromise.promise; + if (this.loadedPromise) { + await this.loadedPromise.promise; + } // File should be set now return this._file; diff --git a/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts b/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts index 343cdd940a5f..14430d5d1133 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts @@ -244,7 +244,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, IAsyncDisp closeDocumentBeforeOpeningNotebook: boolean ) => { // See if this is an ipynb file - if (this.isNotebook(document) && this.configuration.getSettings().datascience.useNotebookEditor) { + if (this.isNotebook(document) && this.configuration.getSettings(document.uri).datascience.useNotebookEditor) { const closeActiveEditorCommand = 'workbench.action.closeActiveEditor'; try { const contents = document.getText(); diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index 1ce8b4eff77c..421f1b6934dd 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -60,7 +60,7 @@ export abstract class WebViewHost implements IDisposable { // Listen for settings changes this.settingsChangeHandler = this.configService - .getSettings() + .getSettings(undefined) .onDidChange(this.onDataScienceSettingsChanged.bind(this)); // Send the first settings message @@ -354,9 +354,9 @@ export abstract class WebViewHost implements IDisposable { }; // Post a message to our webpanel and update our new datascience settings - private onDataScienceSettingsChanged = () => { + private onDataScienceSettingsChanged = async () => { // Stringify our settings to send over to the panel - const dsSettings = JSON.stringify(this.generateDataScienceExtraSettings()); + const dsSettings = JSON.stringify(await this.generateDataScienceExtraSettings()); this.postMessageInternal(SharedMessages.UpdateSettings, dsSettings).ignoreErrors(); }; } diff --git a/src/test/datascience/editor-integration/codewatcher.unit.test.ts b/src/test/datascience/editor-integration/codewatcher.unit.test.ts index 576e6bd073cf..52d63d3a8d50 100644 --- a/src/test/datascience/editor-integration/codewatcher.unit.test.ts +++ b/src/test/datascience/editor-integration/codewatcher.unit.test.ts @@ -132,7 +132,7 @@ suite('DataScience Code Watcher Unit Tests', () => { documentManager.setup(dm => dm.activeTextEditor).returns(() => textEditor.object); // Setup config service - configService.setup(c => c.getSettings()).returns(() => pythonSettings); + configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings); commandManager .setup(c => c.executeCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index 2bb66ab22673..0aab2cc65b44 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -803,7 +803,7 @@ suite('Jupyter Execution', async () => { when(serviceContainer.get(IFileSystem)).thenReturn(instance(fileSystem)); when(serviceContainer.get(IWorkspaceService)).thenReturn(instance(workspaceService)); when(serviceContainer.get(IApplicationShell)).thenReturn(instance(application)); - when(configService.getSettings()).thenReturn(pythonSettings); + when(configService.getSettings(anything())).thenReturn(pythonSettings); when(workspaceService.onDidChangeConfiguration).thenReturn(configChangeEvent.event); when(application.withProgress(anything(), anything())).thenCall( (_, cb: (_: any, token: any) => Promise) => { diff --git a/src/test/datascience/interactive-ipynb/nativeEditor.unit.test.ts b/src/test/datascience/interactive-ipynb/nativeEditor.unit.test.ts index 48fdf6a5365c..08a4ad799f6c 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditor.unit.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditor.unit.test.ts @@ -287,7 +287,7 @@ suite('Data Science - Native Editor', () => { when(experimentsManager.inExperiment(anything())).thenReturn(false); when(settings.onDidChange).thenReturn(settingsChangedEvent.event); - when(configService.getSettings()).thenReturn(instance(settings)); + when(configService.getSettings(anything())).thenReturn(instance(settings)); const configChangeEvent = new EventEmitter(); when(workspace.onDidChangeConfiguration).thenReturn(configChangeEvent.event); diff --git a/src/test/datascience/interactive-ipynb/nativeEditorProvider.unit.test.ts b/src/test/datascience/interactive-ipynb/nativeEditorProvider.unit.test.ts index 5930d3b5c650..f4447535ce7d 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditorProvider.unit.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditorProvider.unit.test.ts @@ -6,7 +6,7 @@ // tslint:disable: no-any import { expect } from 'chai'; -import { instance, mock, when } from 'ts-mockito'; +import { anything, instance, mock, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; import { EventEmitter, TextDocument, TextEditor, Uri } from 'vscode'; import { CommandManager } from '../../../client/common/application/commandManager'; @@ -51,7 +51,7 @@ suite('Data Science - Native Editor Provider', () => { function createNotebookProvider(shouldOpenNotebookEditor: boolean) { editor = typemoq.Mock.ofType(); - when(configService.getSettings()).thenReturn({ datascience: { useNotebookEditor: true } } as any); + when(configService.getSettings(anything())).thenReturn({ datascience: { useNotebookEditor: true } } as any); when(docManager.onDidChangeActiveTextEditor).thenReturn(changeActiveTextEditorEventEmitter.event); when(docManager.visibleTextEditors).thenReturn([]); editor.setup(e => e.closed).returns(() => new EventEmitter().event); diff --git a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts index 0e6e02220fcb..45b0bafc04d1 100644 --- a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts +++ b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts @@ -113,7 +113,7 @@ suite('Interactive window command listener', async () => { // Service container needs logger, file system, and config service when(serviceContainer.get(IConfigurationService)).thenReturn(instance(configService)); when(serviceContainer.get(IFileSystem)).thenReturn(instance(fileSystem)); - when(configService.getSettings()).thenReturn(pythonSettings); + when(configService.getSettings(anything())).thenReturn(pythonSettings); // Setup default settings pythonSettings.datascience = { diff --git a/src/test/datascience/jupyter/jupyterConnection.unit.test.ts b/src/test/datascience/jupyter/jupyterConnection.unit.test.ts index 71ff0a473265..8a8674c849b9 100644 --- a/src/test/datascience/jupyter/jupyterConnection.unit.test.ts +++ b/src/test/datascience/jupyter/jupyterConnection.unit.test.ts @@ -83,7 +83,7 @@ suite('Data Science - JupyterConnection', () => { getServerInfoStub.resolves(dummyServerInfos); when(fs.arePathsSame(anything(), anything())).thenCall((path1, path2) => path1 === path2); when(settings.datascience).thenReturn(dsSettings); - when(configService.getSettings()).thenReturn(instance(settings)); + when(configService.getSettings(anything())).thenReturn(instance(settings)); when(serviceContainer.get(IFileSystem)).thenReturn(instance(fs)); when(serviceContainer.get(IConfigurationService)).thenReturn(instance(configService)); }); diff --git a/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts index 285132334c9d..007419d145b0 100644 --- a/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/kernelSwitcher.unit.test.ts @@ -84,7 +84,7 @@ suite('Data Science - Kernel Switcher', () => { // tslint:disable-next-line: no-any when(settings.datascience).thenReturn({} as any); when(notebook.server).thenReturn(instance(notebookServer)); - when(configService.getSettings()).thenReturn(instance(settings)); + when(configService.getSettings(anything())).thenReturn(instance(settings)); kernelSwitcher = new KernelSwitcher( instance(configService), instance(sessionManagerFactory), diff --git a/src/test/datascience/jupyter/serverCache.unit.test.ts b/src/test/datascience/jupyter/serverCache.unit.test.ts index 563ead2faa07..456f6e25e1fa 100644 --- a/src/test/datascience/jupyter/serverCache.unit.test.ts +++ b/src/test/datascience/jupyter/serverCache.unit.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { assert } from 'chai'; -import { instance, mock, when } from 'ts-mockito'; +import { anything, instance, mock, when } from 'ts-mockito'; import { CancellationToken } from 'vscode'; import { WorkspaceService } from '../../../client/common/application/workspace'; import { PythonSettings } from '../../../client/common/configSettings'; @@ -52,7 +52,7 @@ suite('Data Science - ServerCache', () => { variableQueries: [], jupyterCommandLineArguments: [] }; - when(configService.getSettings()).thenReturn(pythonSettings); + when(configService.getSettings(anything())).thenReturn(pythonSettings); serverCache = new ServerCache(instance(configService), instance(workspaceService), instance(fileSystem)); }); diff --git a/src/test/datascience/jupyterVariables.unit.test.ts b/src/test/datascience/jupyterVariables.unit.test.ts index 38892a92fa01..11924dcb9c86 100644 --- a/src/test/datascience/jupyterVariables.unit.test.ts +++ b/src/test/datascience/jupyterVariables.unit.test.ts @@ -106,7 +106,7 @@ suite('JupyterVariables', () => { fileSystem = typemoq.Mock.ofType(); fileSystem.setup(fs => fs.readFile(typemoq.It.isAnyString())).returns(() => Promise.resolve('test')); - config.setup(s => s.getSettings()).returns(() => pythonSettings); + config.setup(s => s.getSettings(typemoq.It.isAny())).returns(() => pythonSettings); jupyterVariables = new JupyterVariables(fileSystem.object, config.object); }); diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts index 9df4d1d02d06..ab849cc9fe7d 100644 --- a/src/test/datascience/mockJupyterManager.ts +++ b/src/test/datascience/mockJupyterManager.ts @@ -116,7 +116,7 @@ export class MockJupyterManager implements IJupyterSessionManager { // Listen to configuration changes like the real interpreter service does so that we fire our settings changed event const configService = serviceManager.get(IConfigurationService); if (configService && configService !== null) { - configService.getSettings().onDidChange(this.onConfigChanged.bind(this, configService)); + configService.getSettings(undefined).onDidChange(this.onConfigChanged.bind(this, configService)); } // Stick our services into the service manager From f1208095142f8fb4d928664700f1979393d9f6eb Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 19 Feb 2020 09:24:39 -0800 Subject: [PATCH 11/14] Fix live share tests and execution unit tests --- .../interactive-common/interactiveBase.ts | 12 ++- .../interpreter/jupyterCommandFinder.ts | 6 +- ...pyterCommandInterpreterExecutionService.ts | 2 +- .../datascience/jupyter/jupyterConnection.ts | 2 +- .../datascience/jupyter/jupyterExecution.ts | 6 +- .../jupyter/kernels/kernelSwitcher.ts | 2 +- .../jupyter/liveshare/serverCache.ts | 2 +- src/test/datascience/execution.unit.test.ts | 90 ++++++++++++------- 8 files changed, 76 insertions(+), 46 deletions(-) diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index e0cbe55c566a..ac6efe04bd32 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -1072,8 +1072,14 @@ export abstract class InteractiveBase extends WebViewHost { try { - this.stopServer().ignoreErrors(); + await this.stopServer(); } catch { // We just switched from host to guest mode. Don't really care // if closing the host server kills it. @@ -1365,7 +1371,7 @@ export abstract class InteractiveBase extends WebViewHost { const [interpreter, activeInterpreter] = await Promise.all([ this.getSelectedInterpreter(token), - this.interpreterService.getActiveInterpreter() + this.interpreterService.getActiveInterpreter(undefined) ]); if (!interpreter) { return; diff --git a/src/client/datascience/jupyter/jupyterConnection.ts b/src/client/datascience/jupyter/jupyterConnection.ts index ff3fc5f4b587..254288202279 100644 --- a/src/client/datascience/jupyter/jupyterConnection.ts +++ b/src/client/datascience/jupyter/jupyterConnection.ts @@ -60,7 +60,7 @@ export class JupyterConnectionWaiter implements IDisposable { this.startPromise = createDeferred(); // We want to reject our Jupyter connection after a specific timeout - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(undefined); const jupyterLaunchTimeout = settings.datascience.jupyterLaunchTimeout; this.launchTimeout = setTimeout(() => { diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts index 3ca07ace41b1..53e98391d9e0 100644 --- a/src/client/datascience/jupyter/jupyterExecution.ts +++ b/src/client/datascience/jupyter/jupyterExecution.ts @@ -151,7 +151,7 @@ export class JupyterExecutionBase implements IJupyterExecution { // Try to connect to our jupyter process. Check our setting for the number of tries let tryCount = 0; - const maxTries = this.configuration.getSettings().datascience.jupyterLaunchRetries; + const maxTries = this.configuration.getSettings(undefined).datascience.jupyterLaunchRetries; const stopWatch = new StopWatch(); while (tryCount < maxTries) { try { @@ -333,7 +333,7 @@ export class JupyterExecutionBase implements IJupyterExecution { const useDefaultConfig = options && options.useDefaultConfig ? true : false; const connection = await this.startNotebookServer( useDefaultConfig, - this.configuration.getSettings().datascience.jupyterCommandLineArguments, + this.configuration.getSettings(undefined).datascience.jupyterCommandLineArguments, cancelToken ); if (connection) { @@ -347,7 +347,7 @@ export class JupyterExecutionBase implements IJupyterExecution { } } else { // If we have a URI spec up a connection info for it - return createRemoteConnectionInfo(options.uri, this.configuration.getSettings().datascience); + return createRemoteConnectionInfo(options.uri, this.configuration.getSettings(undefined).datascience); } } diff --git a/src/client/datascience/jupyter/kernels/kernelSwitcher.ts b/src/client/datascience/jupyter/kernels/kernelSwitcher.ts index 817b79de876d..7cd6297b48b7 100644 --- a/src/client/datascience/jupyter/kernels/kernelSwitcher.ts +++ b/src/client/datascience/jupyter/kernels/kernelSwitcher.ts @@ -122,7 +122,7 @@ export class KernelSwitcher { // Change the kernel. A status update should fire that changes our display await notebook.setKernelSpec( newKernel.kernelSpec || newKernel.kernelModel!, - this.configService.getSettings().datascience.jupyterLaunchTimeout + this.configService.getSettings(notebook.resource).datascience.jupyterLaunchTimeout ); if (newKernel.interpreter) { diff --git a/src/client/datascience/jupyter/liveshare/serverCache.ts b/src/client/datascience/jupyter/liveshare/serverCache.ts index a608569a0175..c3b6fb8a70a6 100644 --- a/src/client/datascience/jupyter/liveshare/serverCache.ts +++ b/src/client/datascience/jupyter/liveshare/serverCache.ts @@ -143,7 +143,7 @@ export class ServerCache implements IAsyncDisposable { private async calculateWorkingDirectory(): Promise { let workingDir: string | undefined; // For a local launch calculate the working directory that we should switch into - const settings = this.configService.getSettings(); + const settings = this.configService.getSettings(undefined); const fileRoot = settings.datascience.notebookFileRoot; // If we don't have a workspace open the notebookFileRoot seems to often have a random location in it (we use ${workspaceRoot} as default) diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index 0aab2cc65b44..03239302bb9f 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -735,13 +735,13 @@ suite('Jupyter Execution', async () => { skipSearch?: boolean, runInDocker?: boolean ): { - workingPythonExecutionService: TypeMoq.IMock; + executionService: IPythonExecutionService; jupyterExecutionFactory: JupyterExecutionFactory; } { // Setup defaults when(interpreterService.onDidChangeInterpreter).thenReturn(dummyEvent.event); - when(interpreterService.getActiveInterpreter()).thenResolve(activeInterpreter); - when(interpreterService.getInterpreters()).thenResolve([ + when(interpreterService.getActiveInterpreter(anything())).thenResolve(activeInterpreter); + when(interpreterService.getInterpreters(anything())).thenResolve([ workingPython, missingKernelPython, missingNotebookPython @@ -781,19 +781,52 @@ suite('Jupyter Execution', async () => { executionFactory.create(argThat(o => o.pythonPath && o.pythonPath === missingNotebookPython2.path)) ).thenResolve(missingNotebookService2.object); - // Special case, nothing passed in. Match the active - let activeService = workingService.object; + when( + executionFactory.createDaemon(argThat(o => o.pythonPath && o.pythonPath === workingPython.path)) + ).thenResolve(workingService.object); + + when( + executionFactory.createDaemon(argThat(o => o.pythonPath && o.pythonPath === missingKernelPython.path)) + ).thenResolve(missingKernelService.object); + + when( + executionFactory.createDaemon(argThat(o => o.pythonPath && o.pythonPath === missingNotebookPython.path)) + ).thenResolve(missingNotebookService.object); + + when( + executionFactory.createDaemon(argThat(o => o.pythonPath && o.pythonPath === missingNotebookPython2.path)) + ).thenResolve(missingNotebookService2.object); + + let activeService = workingService; if (activeInterpreter === missingKernelPython) { - activeService = missingKernelService.object; + activeService = missingKernelService; } else if (activeInterpreter === missingNotebookPython) { - activeService = missingNotebookService.object; + activeService = missingNotebookService; } else if (activeInterpreter === missingNotebookPython2) { - activeService = missingNotebookService2.object; + activeService = missingNotebookService2; } - when(executionFactory.create(argThat(o => !o || !o.pythonPath))).thenResolve(activeService); + when(executionFactory.create(argThat(o => !o || !o.pythonPath))).thenResolve(activeService.object); when( executionFactory.createActivatedEnvironment(argThat(o => !o || o.interpreter === activeInterpreter)) - ).thenResolve(activeService); + ).thenResolve(activeService.object); + when( + executionFactory.createActivatedEnvironment(argThat(o => o && o.interpreter.path === workingPython.path)) + ).thenResolve(workingService.object); + when( + executionFactory.createActivatedEnvironment( + argThat(o => o && o.interpreter.path === missingKernelPython.path) + ) + ).thenResolve(missingKernelService.object); + when( + executionFactory.createActivatedEnvironment( + argThat(o => o && o.interpreter.path === missingNotebookPython.path) + ) + ).thenResolve(missingNotebookService.object); + when( + executionFactory.createActivatedEnvironment( + argThat(o => o && o.interpreter.path === missingNotebookPython2.path) + ) + ).thenResolve(missingNotebookService2.object); when(processServiceFactory.create()).thenResolve(processService.object); when(liveShare.getApi()).thenResolve(null); @@ -938,7 +971,7 @@ suite('Jupyter Execution', async () => { when(serviceContainer.get(KernelSelector)).thenReturn(instance(kernelSelector)); when(serviceContainer.get(NotebookStarter)).thenReturn(notebookStarter); return { - workingPythonExecutionService: workingService, + executionService: activeService.object, jupyterExecutionFactory: new JupyterExecutionFactory( instance(liveShare), instance(interpreterService), @@ -957,14 +990,7 @@ suite('Jupyter Execution', async () => { } test('Working notebook and commands found', async () => { - const { workingPythonExecutionService, jupyterExecutionFactory } = createExecutionAndReturnProcessService( - workingPython - ); - when( - executionFactory.createDaemon( - deepEqual({ daemonModule: PythonDaemonModule, pythonPath: workingPython.path }) - ) - ).thenResolve(workingPythonExecutionService.object); + const jupyterExecutionFactory = createExecution(workingPython); await assert.eventually.equal(jupyterExecutionFactory.isNotebookSupported(), true, 'Notebook not supported'); await assert.eventually.equal(jupyterExecutionFactory.isImportSupported(), true, 'Import not supported'); @@ -974,17 +1000,12 @@ suite('Jupyter Execution', async () => { }).timeout(10000); test('Includes correct args for running in docker', async () => { - const { workingPythonExecutionService, jupyterExecutionFactory } = createExecutionAndReturnProcessService( + const { jupyterExecutionFactory } = createExecutionAndReturnProcessService( workingPython, undefined, undefined, true ); - when( - executionFactory.createDaemon( - deepEqual({ daemonModule: PythonDaemonModule, pythonPath: workingPython.path }) - ) - ).thenResolve(workingPythonExecutionService.object); await assert.eventually.equal(jupyterExecutionFactory.isNotebookSupported(), true, 'Notebook not supported'); await assert.eventually.equal(jupyterExecutionFactory.isImportSupported(), true, 'Import not supported'); @@ -995,13 +1016,16 @@ suite('Jupyter Execution', async () => { test('Failing notebook throws exception', async () => { const execution = createExecution(missingNotebookPython); - when(interpreterService.getInterpreters()).thenResolve([missingNotebookPython]); + when(interpreterService.getInterpreters(anything())).thenResolve([missingNotebookPython]); await assert.isRejected(execution.connectToNotebookServer(), 'cant exec'); }).timeout(10000); test('Failing others throws exception', async () => { const execution = createExecution(missingNotebookPython); - when(interpreterService.getInterpreters()).thenResolve([missingNotebookPython, missingNotebookPython2]); + when(interpreterService.getInterpreters(anything())).thenResolve([ + missingNotebookPython, + missingNotebookPython2 + ]); await assert.isRejected(execution.connectToNotebookServer(), 'cant exec'); }).timeout(10000); @@ -1018,7 +1042,7 @@ suite('Jupyter Execution', async () => { test('Missing kernel python still finds interpreter', async () => { const execution = createExecution(missingKernelPython); - when(interpreterService.getActiveInterpreter()).thenResolve(missingKernelPython); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(missingKernelPython); await assert.eventually.equal(execution.isNotebookSupported(), true, 'Notebook not supported'); const usableInterpreter = await execution.getUsableJupyterPython(); assert.isOk(usableInterpreter, 'Usable interpreter not found'); @@ -1040,7 +1064,7 @@ suite('Jupyter Execution', async () => { test('Other than active finds closest match', async () => { const execution = createExecution(missingNotebookPython); - when(interpreterService.getActiveInterpreter()).thenResolve(missingNotebookPython); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(missingNotebookPython); await assert.eventually.equal(execution.isNotebookSupported(), true, 'Notebook not supported'); const usableInterpreter = await execution.getUsableJupyterPython(); assert.isOk(usableInterpreter, 'Usable interpreter not found'); @@ -1117,7 +1141,7 @@ suite('Jupyter Execution', async () => { ); // Now interpreters = fast discovery (less time for display of progress). - when(interpreterService.getInterpreters()).thenReturn(Promise.resolve([])); + when(interpreterService.getInterpreters(anything())).thenReturn(Promise.resolve([])); // The call to isNotebookSupported should not timeout in 1 seconds. const isNotebookSupported = execution.isNotebookSupported(); @@ -1143,7 +1167,7 @@ suite('Jupyter Execution', async () => { ); const slowInterpreterDiscovery = createDeferred(); - when(interpreterService.getInterpreters()).thenReturn(slowInterpreterDiscovery.promise); + when(interpreterService.getInterpreters(anything())).thenReturn(slowInterpreterDiscovery.promise); // The call to interpreterService.getInterpreters shoud not complete, it is very slow. const isNotebookSupported = execution.isNotebookSupported(); @@ -1165,7 +1189,7 @@ suite('Jupyter Execution', async () => { // Make sure we can find jupyter on the path if we // can't find it in a python module. const execution = createExecution(missingNotebookPython); - when(interpreterService.getInterpreters()).thenResolve([missingNotebookPython]); + when(interpreterService.getInterpreters(anything())).thenResolve([missingNotebookPython]); when(fileSystem.getFiles(anyString())).thenResolve([jupyterOnPath]); await assert.isFulfilled(execution.connectToNotebookServer(), 'Should be able to start a server'); }).timeout(10000); @@ -1174,7 +1198,7 @@ suite('Jupyter Execution', async () => { // Make sure we can find jupyter on the path if we // can't find it in a python module. const execution = createExecution(missingNotebookPython, undefined, true); - when(interpreterService.getInterpreters()).thenResolve([missingNotebookPython]); + when(interpreterService.getInterpreters(anything())).thenResolve([missingNotebookPython]); when(fileSystem.getFiles(anyString())).thenResolve([jupyterOnPath]); await assert.isRejected(execution.connectToNotebookServer(), 'cant exec'); }).timeout(10000); From 64b5831792d6afa5e678762608a6a679e7397a2a Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 19 Feb 2020 10:14:57 -0800 Subject: [PATCH 12/14] Fix linter errors --- src/test/datascience/execution.unit.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index 03239302bb9f..4ecbe743fb4e 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -7,7 +7,7 @@ import * as os from 'os'; import * as path from 'path'; import { Observable } from 'rxjs/Observable'; import { SemVer } from 'semver'; -import { anyString, anything, deepEqual, instance, match, mock, reset, verify, when } from 'ts-mockito'; +import { anyString, anything, instance, match, mock, reset, verify, when } from 'ts-mockito'; import { Matcher } from 'ts-mockito/lib/matcher/type/Matcher'; import * as TypeMoq from 'typemoq'; import * as uuid from 'uuid/v4'; @@ -36,7 +36,6 @@ import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } import { createDeferred } from '../../client/common/utils/async'; import { Architecture } from '../../client/common/utils/platform'; import { EXTENSION_ROOT_DIR } from '../../client/constants'; -import { PythonDaemonModule } from '../../client/datascience/constants'; import { JupyterCommandFactory } from '../../client/datascience/jupyter/interpreter/jupyterCommand'; import { JupyterCommandFinder } from '../../client/datascience/jupyter/interpreter/jupyterCommandFinder'; import { JupyterCommandFinderInterpreterExecutionService } from '../../client/datascience/jupyter/interpreter/jupyterCommandInterpreterExecutionService'; From 389d17d1ae9dc81f126bf393c353b220be3552c3 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 19 Feb 2020 11:07:31 -0800 Subject: [PATCH 13/14] Fix other functional tests and unit test --- src/client/datascience/webViewHost.ts | 4 ++-- src/test/datascience/dataScienceIocContainer.ts | 2 +- src/test/datascience/datascience.unit.test.ts | 4 ++-- src/test/datascience/intellisense.functional.test.tsx | 6 +++--- src/test/datascience/mockJupyterManager.ts | 2 +- src/test/datascience/testHelpers.tsx | 1 + 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index 1434923bf690..8cf94d263af3 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -327,7 +327,7 @@ export abstract class WebViewHost implements IDisposable { } // Post a message to our webpanel and update our new datascience settings - private onPossibleSettingsChange = (event: ConfigurationChangeEvent) => { + private onPossibleSettingsChange = async (event: ConfigurationChangeEvent) => { if ( event.affectsConfiguration('workbench.colorTheme') || event.affectsConfiguration('editor.fontSize') || @@ -345,7 +345,7 @@ export abstract class WebViewHost implements IDisposable { event.affectsConfiguration('python.dataScience.enableGather') ) { // See if the theme changed - const newSettings = this.generateDataScienceExtraSettings(); + const newSettings = await this.generateDataScienceExtraSettings(); if (newSettings) { const dsSettings = JSON.stringify(newSettings); this.postMessageInternal(SharedMessages.UpdateSettings, dsSettings).ignoreErrors(); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 93d3154bab09..f214ba127a3f 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -1154,7 +1154,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { // Force a new config setting to appear. if (!pythonPath) { const active = await this.get(IInterpreterService).getActiveInterpreter(undefined); - const list = await this.get(IInterpreterService).getInterpreters(); + const list = await this.get(IInterpreterService).getInterpreters(undefined); // Should support jupyter? How to enforce this const supportsJupyter = list.filter(l => l.path !== active?.path).filter(f => this.hasJupyter(f.path)); diff --git a/src/test/datascience/datascience.unit.test.ts b/src/test/datascience/datascience.unit.test.ts index d9f96c633713..486b1d48fc1b 100644 --- a/src/test/datascience/datascience.unit.test.ts +++ b/src/test/datascience/datascience.unit.test.ts @@ -4,7 +4,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { instance, mock, verify, when } from 'ts-mockito'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; import { CommandManager } from '../../client/common/application/commandManager'; import { DocumentManager } from '../../client/common/application/documentManager'; import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; @@ -53,7 +53,7 @@ suite('Data Science Tests', () => { onDidChangeSettings = sinon.stub(); onDidChangeActiveTextEditor = sinon.stub(); - when(configService.getSettings()).thenReturn(instance(settings)); + when(configService.getSettings(anything())).thenReturn(instance(settings)); when(settings.onDidChange).thenReturn(onDidChangeSettings); // tslint:disable-next-line: no-any when(settings.datascience).thenReturn({} as any); diff --git a/src/test/datascience/intellisense.functional.test.tsx b/src/test/datascience/intellisense.functional.test.tsx index a0ab269bdf7e..bf4a8843ff4f 100644 --- a/src/test/datascience/intellisense.functional.test.tsx +++ b/src/test/datascience/intellisense.functional.test.tsx @@ -149,12 +149,12 @@ suite('DataScience Intellisense tests', () => { // Then change our current interpreter const interpreterService = ioc.get(IInterpreterService); - const oldActive = await interpreterService.getActiveInterpreter(); - const interpreters = await interpreterService.getInterpreters(); + const oldActive = await interpreterService.getActiveInterpreter(undefined); + const interpreters = await interpreterService.getInterpreters(undefined); if (interpreters.length > 1 && oldActive) { const firstOther = interpreters.filter(i => i.path !== oldActive.path); ioc.forceSettingsChanged(firstOther[0].path); - const active = await interpreterService.getActiveInterpreter(); + const active = await interpreterService.getActiveInterpreter(undefined); assert.notDeepEqual(active, oldActive, 'Should have changed interpreter'); } diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts index ab849cc9fe7d..5464b20056a0 100644 --- a/src/test/datascience/mockJupyterManager.ts +++ b/src/test/datascience/mockJupyterManager.ts @@ -102,7 +102,7 @@ export class MockJupyterManager implements IJupyterSessionManager { .setup(i => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(this.activeInterpreter)); this.interpreterService - .setup(i => i.getInterpreters()) + .setup(i => i.getInterpreters(TypeMoq.It.isAny())) .returns(() => Promise.resolve(this.installedInterpreters)); this.interpreterService .setup(i => i.getInterpreterDetails(TypeMoq.It.isAnyString())) diff --git a/src/test/datascience/testHelpers.tsx b/src/test/datascience/testHelpers.tsx index b0ffad941f8e..ac60386edacd 100644 --- a/src/test/datascience/testHelpers.tsx +++ b/src/test/datascience/testHelpers.tsx @@ -571,6 +571,7 @@ function enterKey( export function getInteractiveEditor( wrapper: ReactWrapper, React.Component> ): ReactWrapper, React.Component> { + wrapper.update(); // Find the last cell. It should have a monacoEditor object const cells = wrapper.find('InteractiveCell'); const lastCell = cells.last(); From 3fd4a21c0c104c0e7b69cc2b0e3ccef3cc04f0d4 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 19 Feb 2020 12:37:17 -0800 Subject: [PATCH 14/14] Fix last failures --- src/client/datascience/datascience.ts | 2 +- src/client/datascience/interactive-common/interactiveBase.ts | 2 +- src/test/datascience/interactiveWindow.functional.test.tsx | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts index 5ebec44f9e29..eab7315fe283 100644 --- a/src/client/datascience/datascience.ts +++ b/src/client/datascience/datascience.ts @@ -68,7 +68,7 @@ export class DataScience implements IDataScience { } private onSettingsChanged = () => { - const settings = this.configuration.getSettings(); + const settings = this.configuration.getSettings(undefined); const enabled = settings.datascience.enabled; let editorContext = new ContextKey(EditorContexts.DataScienceEnabled, this.commandManager); editorContext.set(enabled).catch(); diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index ac6efe04bd32..630ea9af76c6 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -420,7 +420,7 @@ export abstract class InteractiveBase extends WebViewHost { + return this.copyCodeInternal(args.source).catch(err => { this.applicationShell.showErrorMessage(err); }); } diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 98f83fc23526..7fdb88007e0b 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -968,7 +968,8 @@ Type: builtin_function_or_method`, const docManager = ioc.get(IDocumentManager); docManager.showTextDocument(docManager.textDocuments[0]); const window = (await getOrCreateInteractiveWindow(ioc)) as InteractiveWindow; - window.copyCode({ source: 'print("baz")' }); + await window.show(); + await window.copyCode({ source: 'print("baz")' }); assert.equal( docManager.textDocuments[0].getText(), `${defaultCellMarker}${os.EOL}print("baz")${os.EOL}${defaultCellMarker}${os.EOL}print("bar")`, @@ -976,7 +977,7 @@ Type: builtin_function_or_method`, ); const activeEditor = docManager.activeTextEditor as MockEditor; activeEditor.selection = new Selection(1, 2, 1, 2); - window.copyCode({ source: 'print("baz")' }); + await window.copyCode({ source: 'print("baz")' }); assert.equal( docManager.textDocuments[0].getText(), `${defaultCellMarker}${os.EOL}${defaultCellMarker}${os.EOL}print("baz")${os.EOL}${defaultCellMarker}${os.EOL}print("baz")${os.EOL}${defaultCellMarker}${os.EOL}print("bar")`,