Skip to content

Commit ae40119

Browse files
IanMatthewHuffIanMatthewHuff
authored
Fix URL Jupyter server connection issue and add launch timeout (microsoft#3386)
* Get connection info to server via the notebook api * Fallback to more relaxed string parsing * Timeout if we can't connect
1 parent 092a37e commit ae40119

File tree

11 files changed

+167
-16
lines changed

11 files changed

+167
-16
lines changed

‎package.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,12 @@
866866
"description": "Enable the experimental data science features in the python extension.",
867867
"scope": "resource"
868868
},
869+
"python.dataScience.jupyterLaunchTimeout": {
870+
"type": "number",
871+
"default": 60000,
872+
"description": "Amount of time (in ms) to wait for the Jupyter Notebook server to start.",
873+
"scope": "resource"
874+
},
869875
"python.disableInstallationCheck": {
870876
"type": "boolean",
871877
"default": false,

‎package.nls.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@
7777
"DataScience.notebookCheckForImportTitle": "Do you want to import the Jupyter Notebook into Python code?",
7878
"DataScience.jupyterNotSupported": "Running cells requires Jupyter notebooks to be installed.",
7979
"DataScience.jupyterNbConvertNotSupported": "Importing notebooks requires Jupyter nbconvert to be installed.",
80+
"DataScience.jupyterLaunchTimedOut": "The Jupyter notebook server failed to launch in time",
81+
"DataScience.jupyterLaunchNoURL": "Failed to find the URL of the launched Jupyter notebook server",
8082
"DataScience.pythonInteractiveHelpLink" : "Get more help",
8183
"DataScience.importingFormat": "Importing {0}",
8284
"DataScience.startingJupyter": "Starting Jupyter Server",

‎pythonFiles/datascience/getServerInfo.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
from notebook.notebookapp import list_running_servers
5+
import json
6+
7+
server_list = list_running_servers()
8+
9+
server_info_list = []
10+
11+
for si in server_list:
12+
server_info_object = {}
13+
server_info_object["base_url"] = si['base_url']
14+
server_info_object["notebook_dir"] = si['notebook_dir']
15+
server_info_object["hostname"] = si['hostname']
16+
server_info_object["password"] = si['password']
17+
server_info_object["pid"] = si['pid']
18+
server_info_object["port"] = si['port']
19+
server_info_object["secure"] = si['secure']
20+
server_info_object["token"] = si['token']
21+
server_info_object["url"] = si['url']
22+
server_info_list.append(server_info_object)
23+
24+
print(json.dumps(server_info_list))

‎src/client/common/platform/fileSystem.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ export class FileSystem implements IFileSystem {
5959
return fs.mkdirp(directoryPath);
6060
}
6161

62+
public deleteDirectory(directoryPath: string): Promise<void> {
63+
const deferred = createDeferred<void>();
64+
fs.rmdir(directoryPath, err => err ? deferred.reject(err) : deferred.resolve());
65+
return deferred.promise;
66+
}
67+
6268
public getSubDirectories(rootDir: string): Promise<string[]> {
6369
return new Promise<string[]>(resolve => {
6470
fs.readdir(rootDir, (error, files) => {

‎src/client/common/platform/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export interface IFileSystem {
4949
fileExistsSync(path: string): boolean;
5050
directoryExists(path: string): Promise<boolean>;
5151
createDirectory(path: string): Promise<void>;
52+
deleteDirectory(path: string): Promise<void>;
5253
getSubDirectories(rootDir: string): Promise<string[]>;
5354
arePathsSame(path1: string, path2: string): boolean;
5455
readFile(filePath: string): Promise<string>;

‎src/client/common/types.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,9 @@ export interface IAnalysisSettings {
264264
}
265265

266266
export interface IDataScienceSettings {
267-
allowImportFromNotebook : boolean;
268-
enabled : boolean;
267+
allowImportFromNotebook: boolean;
268+
enabled: boolean;
269+
jupyterLaunchTimeout: number;
269270
}
270271

271272
export const IConfigurationService = Symbol('IConfigurationService');

‎src/client/common/utils/localize.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ export namespace DataScience {
4444
export const notebookCheckForImportDontAskAgain = localize('DataScience.notebookCheckForImportDontAskAgain', 'Don\'t Ask Again');
4545
export const jupyterNotSupported = localize('DataScience.jupyterNotSupported', 'Jupyter is not installed');
4646
export const jupyterNbConvertNotSupported = localize('DataScience.jupyterNbConvertNotSupported', 'Jupyter nbconvert is not installed');
47+
export const jupyterLaunchTimedOut = localize('DataScience.jupyterLaunchTimedOut', 'The Jupyter notebook server failed to launch in time');
48+
export const jupyterLaunchNoURL = localize('DataScience.jupyterLaunchNoURL', 'Failed to find the URL of the launched Jupyter notebook server');
4749
export const pythonInteractiveHelpLink = localize('DataScience.pythonInteractiveHelpLink', 'See [https://aka.ms/pyaiinstall] for help on installing jupyter.');
4850
export const importingFormat = localize('DataScience.importingFormat', 'Importing {0}');
4951
export const startingJupyter = localize('DataScience.startingJupyter', 'Starting Jupyter Server');

‎src/client/datascience/jupyterExecution.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import * as uuid from 'uuid/v4';
1010
import { ExecutionResult, IPythonExecutionFactory, ObservableExecutionResult, SpawnOptions } from '../common/process/types';
1111
import { ILogger } from '../common/types';
1212
import * as localize from '../common/utils/localize';
13+
import { EXTENSION_ROOT_DIR } from '../constants';
1314
import { ICondaService, IInterpreterService, InterpreterType } from '../interpreter/contracts';
14-
import { IJupyterExecution, IJupyterKernelSpec } from './types';
15+
import { IJupyterExecution, IJupyterKernelSpec, JupyterServerInfo } from './types';
1516

1617
class JupyterKernelSpec implements IJupyterKernelSpec {
1718
public name: string;
@@ -32,6 +33,25 @@ export class JupyterExecution implements IJupyterExecution {
3233
@inject(ILogger) private logger: ILogger) {
3334
}
3435

36+
public getJupyterServerInfo = async () : Promise<JupyterServerInfo[]> => {
37+
const newOptions: SpawnOptions = {mergeStdOutErr: true};
38+
newOptions.env = await this.fixupCondaEnv(newOptions.env);
39+
40+
// We have a small python file here that we will execute to get the server info from all running Jupyter instances
41+
const pythonService = await this.executionFactory.create({});
42+
const file = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getServerInfo.py');
43+
const serverInfoString = await pythonService.exec([file], newOptions);
44+
45+
let serverInfos: JupyterServerInfo[];
46+
try {
47+
// Parse out our results, return undefined if we can't suss it out
48+
serverInfos = JSON.parse(serverInfoString.stdout.trim()) as JupyterServerInfo[];
49+
} catch (err) {
50+
return undefined;
51+
}
52+
return serverInfos;
53+
}
54+
3555
public execModuleObservable = async (module: string, args: string[], options: SpawnOptions): Promise<ObservableExecutionResult<string>> => {
3656
const newOptions = {...options};
3757
newOptions.env = await this.fixupCondaEnv(newOptions.env);

‎src/client/datascience/jupyterProcess.ts‎

Lines changed: 89 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
'use strict';
4+
import '../common/extensions';
45

56
import { inject, injectable } from 'inversify';
67
import * as tk from 'tree-kill';
78
import { URL } from 'url';
89

10+
import { IFileSystem } from '../common/platform/types';
911
import { ExecutionResult, IPythonExecutionFactory, ObservableExecutionResult, Output, PythonVersionInfo } from '../common/process/types';
10-
import { ILogger } from '../common/types';
12+
import { IConfigurationService, ILogger} from '../common/types';
1113
import { createDeferred, Deferred } from '../common/utils/async';
12-
import { IJupyterExecution, INotebookProcess } from './types';
14+
import * as localize from '../common/utils/localize';
15+
import { IJupyterExecution, INotebookProcess, JupyterServerInfo } from './types';
1316

1417
export interface IConnectionInfo {
1518
baseUrl: string;
@@ -19,20 +22,25 @@ export interface IConnectionInfo {
1922
// This class communicates with an instance of jupyter that's running in the background
2023
@injectable()
2124
export class JupyterProcess implements INotebookProcess {
22-
private static urlPattern = /http:\/\/localhost:[0-9]+\/\?token=[a-z0-9]+/;
25+
private static urlPattern = /(https?:\/\/[^\s]+)/;
2326
private static forbiddenPattern = /Forbidden/;
27+
private static httpPattern = /https?:\/\//;
2428
public isDisposed: boolean = false;
2529
private startPromise: Deferred<IConnectionInfo> | undefined;
2630
private startObservable: ObservableExecutionResult<string> | undefined;
31+
private launchTimeout: NodeJS.Timer;
32+
33+
private notebook_dir: string;
2734

2835
constructor(
2936
@inject(IPythonExecutionFactory) private executionFactory: IPythonExecutionFactory,
37+
@inject(IConfigurationService) private configService: IConfigurationService,
3038
@inject(IJupyterExecution) private jupyterExecution : IJupyterExecution,
39+
@inject(IFileSystem) private fileSystem: IFileSystem,
3140
@inject(ILogger) private logger: ILogger) {
3241
}
3342

3443
public start = async (notebookdir: string) : Promise<void> => {
35-
3644
// Compute args based on if inside a workspace or not
3745
const args: string [] = ['notebook', '--no-browser', `--notebook-dir=${notebookdir}`];
3846

@@ -41,6 +49,19 @@ export class JupyterProcess implements INotebookProcess {
4149

4250
// Use the IPythonExecutionService to find Jupyter
4351
this.startObservable = await this.jupyterExecution.execModuleObservable('jupyter', args, { throwOnStdErr: false, encoding: 'utf8'});
52+
// Save our notebook dir as we will use this to verify when we started up
53+
this.notebook_dir = notebookdir;
54+
55+
// We want to reject our Jupyter connection after a specific timeout
56+
const settings = this.configService.getSettings();
57+
const jupyterLaunchTimeout = settings.datascience.jupyterLaunchTimeout;
58+
59+
if (this.launchTimeout) {
60+
clearTimeout(this.launchTimeout);
61+
}
62+
this.launchTimeout = setTimeout(() => {
63+
this.launchTimedOut();
64+
}, jupyterLaunchTimeout);
4465

4566
// Listen on stderr for its connection information
4667
this.startObservable.out.subscribe((output : Output<string>) => {
@@ -53,6 +74,7 @@ export class JupyterProcess implements INotebookProcess {
5374
}
5475

5576
public shutdown = async () : Promise<void> => {
77+
clearTimeout(this.launchTimeout);
5678
if (this.startObservable && this.startObservable.proc) {
5779
if (!this.startObservable.proc.killed) {
5880
tk(this.startObservable.proc.pid);
@@ -62,7 +84,6 @@ export class JupyterProcess implements INotebookProcess {
6284
}
6385

6486
public spawn = async (notebookFile: string) : Promise<ExecutionResult<string>> => {
65-
6687
// Compute args for the file
6788
const args: string [] = ['notebook', `--NotebookApp.file_to_run=${notebookFile}`];
6889

@@ -94,6 +115,7 @@ export class JupyterProcess implements INotebookProcess {
94115
return this.startPromise!.promise;
95116
}
96117

118+
clearTimeout(this.launchTimeout);
97119
return Promise.resolve({ baseUrl: '', token: ''});
98120
}
99121

@@ -111,25 +133,79 @@ export class JupyterProcess implements INotebookProcess {
111133
}
112134
}
113135

136+
// From a list of jupyter server infos try to find the matching jupyter that we launched
114137
// tslint:disable-next-line:no-any
115-
private extractConnectionInformation = (data: any) => {
116-
this.output(data);
138+
private getJupyterURL(serverInfos: JupyterServerInfo[], data: any) {
139+
if (serverInfos && !this.startPromise.completed) {
140+
const matchInfo = serverInfos.find(info => this.fileSystem.arePathsSame(this.notebook_dir, info['notebook_dir']));
141+
if (matchInfo) {
142+
const url = matchInfo['url'];
143+
const token = matchInfo['token'];
144+
145+
this.resolveStartPromise({ baseUrl: url, token: token });
146+
}
147+
}
117148

118-
// Look for a Jupyter Notebook url in the string received.
149+
// At this point we failed to get the server info or a matching server via the python code, so fall back to
150+
// our URL parse
151+
if (!this.startPromise.completed) {
152+
this.getJupyterURLFromString(data);
153+
}
154+
}
155+
156+
// tslint:disable-next-line:no-any
157+
private getJupyterURLFromString(data: any) {
119158
const urlMatch = JupyterProcess.urlPattern.exec(data);
159+
if (urlMatch && !this.startPromise.completed) {
160+
let url: URL;
161+
try {
162+
url = new URL(urlMatch[0]);
163+
} catch (err) {
164+
// Failed to parse the url either via server infos or the string
165+
this.rejectStartPromise(new Error(localize.DataScience.jupyterLaunchNoURL()));
166+
return;
167+
}
120168

121-
if (urlMatch && this.startPromise) {
122-
const url = new URL(urlMatch[0]);
123-
this.startPromise.resolve({ baseUrl: `${url.protocol}//${url.host}/`, token: `${url.searchParams.get('token')}` });
169+
// Here we parsed the URL correctly
170+
this.resolveStartPromise({ baseUrl: `${url.protocol}//${url.host}${url.pathname}`, token: `${url.searchParams.get('token')}`});
124171
}
172+
}
173+
174+
// tslint:disable-next-line:no-any
175+
private extractConnectionInformation = (data: any) => {
176+
this.output(data);
177+
178+
const httpMatch = JupyterProcess.httpPattern.exec(data);
125179

126-
// Do we need to worry about this not working? Timeout?
180+
if (httpMatch && this.notebook_dir && this.startPromise && !this.startPromise.completed) {
181+
// .then so that we can keep from pushing aync up to the subscribed observable function
182+
this.jupyterExecution.getJupyterServerInfo().then(serverInfos => {
183+
this.getJupyterURL(serverInfos, data);
184+
}).ignoreErrors();
185+
}
127186

128187
// Look for 'Forbidden' in the result
129188
const forbiddenMatch = JupyterProcess.forbiddenPattern.exec(data);
130189
if (forbiddenMatch && this.startPromise && !this.startPromise.resolved) {
131-
this.startPromise.reject(new Error(data.toString('utf8')));
190+
this.rejectStartPromise(new Error(data.toString('utf8')));
132191
}
192+
}
133193

194+
private launchTimedOut = () => {
195+
if (!this.startPromise.completed) {
196+
this.rejectStartPromise(new Error(localize.DataScience.jupyterLaunchTimedOut()));
197+
}
198+
}
199+
200+
private resolveStartPromise = (value?: IConnectionInfo | PromiseLike<IConnectionInfo>) => {
201+
clearTimeout(this.launchTimeout);
202+
this.startPromise.resolve(value);
134203
}
204+
205+
// tslint:disable-next-line:no-any
206+
private rejectStartPromise = (reason?: any) => {
207+
clearTimeout(this.launchTimeout);
208+
this.startPromise.reject(reason);
209+
}
210+
135211
}

‎src/client/datascience/jupyterServer.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export class JupyterServer implements INotebookServer {
3232
private sessionManager : SessionManager | undefined;
3333
private sessionStartTime: number | undefined;
3434
private tempFile: string | undefined;
35+
private tempDirList: string[] = [];
3536
private onStatusChangedEvent : vscode.EventEmitter<boolean> = new vscode.EventEmitter<boolean>();
3637

3738
constructor(
@@ -50,7 +51,12 @@ export class JupyterServer implements INotebookServer {
5051
this.isDisposed = false;
5152

5253
// First generate a temporary notebook. We need this as input to the session
54+
// Use a UUID in the path so that we can verify the instance that we have started up
5355
this.tempFile = await this.generateTempFile();
56+
const uniqueDir = uuid();
57+
this.tempFile = path.join(path.dirname(this.tempFile), uniqueDir, path.basename(this.tempFile));
58+
await this.fileSystem.createDirectory(path.dirname(this.tempFile));
59+
this.tempDirList.push(path.dirname(this.tempFile));
5460

5561
// Find our kernel spec name (this will enumerate the spec json files and
5662
// create a new spec if none match)
@@ -129,6 +135,11 @@ export class JupyterServer implements INotebookServer {
129135
if (this.process) {
130136
this.process.dispose();
131137
}
138+
139+
// Delete any temp .pynb directories that we created
140+
for (const tempDir of this.tempDirList) {
141+
await this.fileSystem.deleteDirectory(tempDir);
142+
}
132143
}
133144

134145
public waitForIdle = async () : Promise<void> => {

0 commit comments

Comments
 (0)