Skip to content

Commit c7ed5b8

Browse files
IanMatthewHuffIanMatthewHuff
authored
Separate Jupyter Server (microsoft#4351)
Separate out JupyterServer creation from the history window
1 parent 30637da commit c7ed5b8

12 files changed

Lines changed: 423 additions & 123 deletions

File tree

‎news/1 Enhancements/4348.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Don't shut down the notebook server on window close

‎src/client/datascience/history.ts‎

Lines changed: 5 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { createDeferred } from '../common/utils/async';
2727
import * as localize from '../common/utils/localize';
2828
import { IInterpreterService } from '../interpreter/contracts';
2929
import { captureTelemetry, sendTelemetryEvent } from '../telemetry';
30-
import { EditorContexts, HistoryMessages, Identifiers, Settings, Telemetry } from './constants';
30+
import { EditorContexts, HistoryMessages, Identifiers, Telemetry } from './constants';
3131
import { HistoryMessageListener } from './historyMessageListener';
3232
import { JupyterInstallError } from './jupyter/jupyterInstallError';
3333
import {
@@ -41,6 +41,7 @@ import {
4141
IJupyterExecution,
4242
INotebookExporter,
4343
INotebookServer,
44+
INotebookServerManager,
4445
InterruptResult,
4546
IStatusProvider
4647
} from './types';
@@ -82,6 +83,7 @@ export class History implements IHistory {
8283
@inject(IConfigurationService) private configuration: IConfigurationService,
8384
@inject(ICommandManager) private commandManager: ICommandManager,
8485
@inject(INotebookExporter) private jupyterExporter: INotebookExporter,
86+
@inject(INotebookServerManager) private jupyterServerManager: INotebookServerManager,
8587
@inject(IWorkspaceService) private workspaceService: IWorkspaceService) {
8688

8789
// Sign up for configuration changes
@@ -199,9 +201,6 @@ export class History implements IHistory {
199201
if (this.closedEvent) {
200202
this.closedEvent.fire(this);
201203
}
202-
if (this.jupyterServer) {
203-
await this.jupyterServer.dispose();
204-
}
205204
this.updateContexts();
206205
}
207206
if (this.changeHandler) {
@@ -602,75 +601,8 @@ export class History implements IHistory {
602601
}
603602
}
604603

605-
private loadJupyterServer = async (restart?: boolean): Promise<void> => {
606-
// Startup our jupyter server
607-
const settings = this.configuration.getSettings();
608-
let serverURI: string | undefined = settings.datascience.jupyterServerURI;
609-
let workingDir: string | undefined;
610-
const useDefaultConfig: boolean | undefined = settings.datascience.useDefaultConfigForJupyter;
611-
const status = this.setStatus(localize.DataScience.connectingToJupyter());
612-
// Check for dark theme, if so set matplot lib to use dark_background settings
613-
let darkTheme: boolean = false;
614-
const workbench = this.workspaceService.getConfiguration('workbench');
615-
if (workbench) {
616-
const theme = workbench.get<string>('colorTheme');
617-
if (theme) {
618-
darkTheme = /dark/i.test(theme);
619-
}
620-
}
621-
622-
try {
623-
// For the local case pass in our URI as undefined, that way connect doesn't have to check the setting
624-
if (serverURI === Settings.JupyterServerLocalLaunch) {
625-
serverURI = undefined;
626-
627-
workingDir = await this.calculateWorkingDirectory();
628-
}
629-
this.jupyterServer = await this.jupyterExecution.connectToNotebookServer(serverURI, darkTheme, useDefaultConfig, undefined, workingDir);
630-
631-
// If this is a restart, show our restart info
632-
if (restart) {
633-
await this.addSysInfo(SysInfoReason.Restart);
634-
}
635-
} finally {
636-
if (status) {
637-
status.dispose();
638-
}
639-
}
640-
}
641-
642-
// Calculate the working directory that we should move into when starting up our Jupyter server locally
643-
private calculateWorkingDirectory = async (): Promise<string | undefined> => {
644-
let workingDir: string | undefined;
645-
// For a local launch calculate the working directory that we should switch into
646-
const settings = this.configuration.getSettings();
647-
const fileRoot = settings.datascience.notebookFileRoot;
648-
649-
// If we don't have a workspace open the notebookFileRoot seems to often have a random location in it (we use ${workspaceRoot} as default)
650-
// so only do this setting if we actually have a valid workspace open
651-
if (fileRoot && this.workspaceService.hasWorkspaceFolders) {
652-
const workspaceFolderPath = this.workspaceService.workspaceFolders![0].uri.fsPath;
653-
if (path.isAbsolute(fileRoot)) {
654-
if (await this.fileSystem.directoryExists(fileRoot)) {
655-
// User setting is absolute and exists, use it
656-
workingDir = fileRoot;
657-
} else {
658-
// User setting is absolute and doesn't exist, use workspace
659-
workingDir = workspaceFolderPath;
660-
}
661-
} else {
662-
// fileRoot is a relative path, combine it with the workspace folder
663-
const combinedPath = path.join(workspaceFolderPath, fileRoot);
664-
if (await this.fileSystem.directoryExists(combinedPath)) {
665-
// combined path exists, use it
666-
workingDir = combinedPath;
667-
} else {
668-
// Combined path doesn't exist, use workspace
669-
workingDir = workspaceFolderPath;
670-
}
671-
}
672-
}
673-
return workingDir;
604+
private async loadJupyterServer(restart?: boolean): Promise<void> {
605+
this.jupyterServer = await this.jupyterServerManager.getOrCreateServer();
674606
}
675607

676608
private generateSysInfoCell = async (reason: SysInfoReason): Promise<ICell | undefined> => {

‎src/client/datascience/jupyter/jupyterExecution.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import {
2828
IJupyterExecution,
2929
IJupyterKernelSpec,
3030
IJupyterSessionManager,
31-
INotebookServer
31+
INotebookServer,
32+
INotebookServerLaunchInfo
3233
} from '../types';
3334
import { JupyterConnection, JupyterServerInfo } from './jupyterConnection';
3435
import { JupyterKernelSpec } from './jupyterKernelSpec';
@@ -141,7 +142,17 @@ export class JupyterExecutionBase implements IJupyterExecution {
141142

142143
// Try to connect to our jupyter process
143144
const result = this.serviceContainer.get<INotebookServer>(INotebookServer);
144-
await result.connect(connection, kernelSpec, usingDarkTheme, cancelToken, workingDir);
145+
const info = await this.interpreterService.getActiveInterpreter();
146+
// Populate the launch info that we are starting our server with
147+
const launchInfo: INotebookServerLaunchInfo = {
148+
connectionInfo: connection,
149+
currentInterpreter: info,
150+
kernelSpec: kernelSpec,
151+
usingDarkTheme: usingDarkTheme,
152+
workingDir: workingDir,
153+
uri: uri
154+
};
155+
await result.connect(launchInfo, cancelToken);
145156
sendTelemetryEvent(uri ? Telemetry.ConnectRemoteJupyter : Telemetry.ConnectLocalJupyter);
146157
return result;
147158
} catch (err) {

‎src/client/datascience/jupyter/jupyterServer.ts‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ import {
2626
ICell,
2727
IConnection,
2828
IDataScience,
29-
IJupyterKernelSpec,
3029
IJupyterSession,
3130
IJupyterSessionManager,
3231
INotebookServer,
32+
INotebookServerLaunchInfo,
3333
InterruptResult
3434
} from '../types';
3535

@@ -109,13 +109,11 @@ class CellSubscriber {
109109
// https://www.npmjs.com/package/@jupyterlab/services
110110

111111
export class JupyterServerBase implements INotebookServer {
112+
private launchInfo: INotebookServerLaunchInfo | undefined;
112113
private session: IJupyterSession | undefined;
113-
private connInfo: IConnection | undefined;
114-
private workingDir: string | undefined;
115114
private sessionStartTime: number | undefined;
116115
private pendingCellSubscriptions: CellSubscriber[] = [];
117116
private ranInitialSetup = false;
118-
private usingDarkTheme: boolean | undefined;
119117

120118
constructor(
121119
liveShare: ILiveShareApi,
@@ -128,23 +126,23 @@ export class JupyterServerBase implements INotebookServer {
128126
this.asyncRegistry.push(this);
129127
}
130128

131-
public async connect(connInfo: IConnection, kernelSpec: IJupyterKernelSpec | undefined, usingDarkTheme: boolean, cancelToken?: CancellationToken, workingDir?: string): Promise<void> {
132-
// Save connection info. Determines if we need to change directory or not
133-
this.connInfo = connInfo;
134-
this.workingDir = workingDir;
135-
this.usingDarkTheme = usingDarkTheme;
129+
public async connect(launchInfo: INotebookServerLaunchInfo, cancelToken?: CancellationToken) {
130+
// Save our launch info
131+
this.launchInfo = launchInfo;
136132

137133
// Start our session
138-
this.session = await this.sessionManager.startNew(connInfo, kernelSpec, cancelToken);
134+
this.session = await this.sessionManager.startNew(launchInfo.connectionInfo, launchInfo.kernelSpec, cancelToken);
139135

140-
// Setup our start time. We reject anything that comes in before this time during execute
141-
this.sessionStartTime = Date.now();
136+
if (this.session) {
137+
// Setup our start time. We reject anything that comes in before this time during execute
138+
this.sessionStartTime = Date.now();
142139

143-
// Wait for it to be ready
144-
await this.session.waitForIdle();
140+
// Wait for it to be ready
141+
await this.session.waitForIdle();
145142

146-
// Run our initial setup and plot magics
147-
this.initialNotebookSetup(cancelToken);
143+
// Run our initial setup and plot magics
144+
this.initialNotebookSetup(cancelToken);
145+
}
148146
}
149147

150148
public shutdown(): Promise<void> {
@@ -192,9 +190,9 @@ export class JupyterServerBase implements INotebookServer {
192190

193191
public async setInitialDirectory(directory: string): Promise<void> {
194192
// If we launched local and have no working directory call this on add code to change directory
195-
if (!this.workingDir && this.connInfo && this.connInfo.localLaunch) {
193+
if (this.launchInfo && !this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) {
196194
await this.changeDirectoryIfPossible(directory);
197-
this.workingDir = directory;
195+
this.launchInfo.workingDir = directory;
198196
}
199197
}
200198

@@ -365,15 +363,23 @@ export class JupyterServerBase implements INotebookServer {
365363
throw new Error(localize.DataScience.sessionDisposed());
366364
}
367365

366+
public getLaunchInfo(): INotebookServerLaunchInfo | undefined {
367+
if (!this.launchInfo) {
368+
return undefined;
369+
}
370+
371+
return this.launchInfo;
372+
}
373+
368374
// Return a copy of the connection information that this server used to connect with
369375
public getConnectionInfo(): IConnection | undefined {
370-
if (!this.connInfo) {
376+
if (!this.launchInfo) {
371377
return undefined;
372378
}
373379

374380
// Return a copy with a no-op for dispose
375381
return {
376-
...this.connInfo,
382+
...this.launchInfo.connectionInfo,
377383
dispose: noop
378384
};
379385
}
@@ -458,12 +464,12 @@ export class JupyterServerBase implements INotebookServer {
458464
this.ranInitialSetup = true;
459465

460466
// When we start our notebook initial, change to our workspace or user specified root directory
461-
if (this.connInfo && this.connInfo.localLaunch && this.workingDir) {
462-
this.changeDirectoryIfPossible(this.workingDir).ignoreErrors();
467+
if (this.launchInfo && this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) {
468+
this.changeDirectoryIfPossible(this.launchInfo.workingDir).ignoreErrors();
463469
}
464470

465471
this.executeSilently(
466-
`%matplotlib inline${os.EOL}import matplotlib.pyplot as plt${this.usingDarkTheme ? `${os.EOL}from matplotlib import style${os.EOL}style.use(\'dark_background\')` : ''}`,
472+
`%matplotlib inline${os.EOL}import matplotlib.pyplot as plt${(this.launchInfo && this.launchInfo.usingDarkTheme) ? `${os.EOL}from matplotlib import style${os.EOL}style.use(\'dark_background\')` : ''}`,
467473
cancelToken
468474
).ignoreErrors();
469475
}
@@ -508,7 +514,7 @@ export class JupyterServerBase implements INotebookServer {
508514
}
509515

510516
private changeDirectoryIfPossible = async (directory: string): Promise<void> => {
511-
if (this.connInfo && this.connInfo.localLaunch && await fs.pathExists(directory)) {
517+
if (this.launchInfo && this.launchInfo.connectionInfo.localLaunch && await fs.pathExists(directory)) {
512518
await this.executeSilently(`%cd "${directory}"`);
513519
}
514520
}

‎src/client/datascience/jupyter/jupyterServerFactory.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ import {
1313
ICell,
1414
IConnection,
1515
IDataScience,
16-
IJupyterKernelSpec,
1716
IJupyterSessionManager,
1817
INotebookServer,
18+
INotebookServerLaunchInfo,
1919
InterruptResult
2020
} from '../types';
2121
import { JupyterServerBase } from './jupyterServer';
@@ -37,7 +37,7 @@ type JupyterServerClassType = {
3737
export class JupyterServer implements INotebookServer {
3838
private serverFactory: RoleBasedFactory<INotebookServer, JupyterServerClassType>;
3939

40-
private connInfo : IConnection | undefined;
40+
private launchInfo: INotebookServerLaunchInfo | undefined;
4141

4242
constructor(
4343
@inject(ILiveShareApi) liveShare: ILiveShareApi,
@@ -62,10 +62,10 @@ export class JupyterServer implements INotebookServer {
6262
);
6363
}
6464

65-
public async connect(connInfo: IConnection, kernelSpec: IJupyterKernelSpec | undefined, usingDarkTheme: boolean, cancelToken?: CancellationToken, workingDir?: string): Promise<void> {
66-
this.connInfo = connInfo;
65+
public async connect(launchInfo: INotebookServerLaunchInfo, cancelToken?: CancellationToken): Promise<void> {
66+
this.launchInfo = launchInfo;
6767
const server = await this.serverFactory.get();
68-
return server.connect(connInfo, kernelSpec, usingDarkTheme, cancelToken, workingDir);
68+
return server.connect(launchInfo, cancelToken);
6969
}
7070

7171
public async shutdown(): Promise<void> {
@@ -126,7 +126,15 @@ export class JupyterServer implements INotebookServer {
126126

127127
// Return a copy of the connection information that this server used to connect with
128128
public getConnectionInfo(): IConnection | undefined {
129-
return this.connInfo;
129+
if (this.launchInfo) {
130+
return this.launchInfo.connectionInfo;
131+
}
132+
133+
return undefined;
134+
}
135+
136+
public getLaunchInfo(): INotebookServerLaunchInfo | undefined {
137+
return this.launchInfo;
130138
}
131139

132140
public async getSysInfo() : Promise<ICell | undefined> {

0 commit comments

Comments
 (0)