Skip to content

Commit 9df5ef4

Browse files
rchiodorchiodo
authored
Support launching with the same directory as a notebook (#13324)
* Fix working directory to come correctly on raw and local jupyter * Add a functional test for current directory and fix some issues with starting raw kernel * Fix deleting * Add news entry * Fix unit tests * Fix functional test on windows
1 parent b7dc679 commit 9df5ef4

33 files changed

+202
-79
lines changed

‎news/2 Fixes/12760.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Support starting kernels with the same directory as the notebook.

‎src/client/datascience/baseJupyterSession.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
7777
private _kernelSocket = new ReplaySubject<KernelSocketInformation | undefined>();
7878
private _jupyterLab?: typeof import('@jupyterlab/services');
7979

80-
constructor(private restartSessionUsed: (id: Kernel.IKernelConnection) => void) {
80+
constructor(private restartSessionUsed: (id: Kernel.IKernelConnection) => void, public workingDirectory: string) {
8181
this.statusHandler = this.onStatusChanged.bind(this);
8282
}
8383
public dispose(): Promise<void> {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export class JupyterConnectionWaiter implements IDisposable {
4646
constructor(
4747
private readonly launchResult: ObservableExecutionResult<string>,
4848
private readonly notebookDir: string,
49+
private readonly rootDir: string,
4950
private readonly getServerInfo: (cancelToken?: CancellationToken) => Promise<JupyterServerInfo[] | undefined>,
5051
serviceContainer: IServiceContainer,
5152
private cancelToken?: CancellationToken
@@ -105,7 +106,7 @@ export class JupyterConnectionWaiter implements IDisposable {
105106

106107
private createConnection(baseUrl: string, token: string, hostName: string, processDisposable: Disposable) {
107108
// tslint:disable-next-line: no-use-before-declare
108-
return new JupyterConnection(baseUrl, token, hostName, processDisposable, this.launchResult.proc);
109+
return new JupyterConnection(baseUrl, token, hostName, this.rootDir, processDisposable, this.launchResult.proc);
109110
}
110111

111112
// tslint:disable-next-line:no-any
@@ -230,6 +231,7 @@ class JupyterConnection implements IJupyterConnection {
230231
public readonly baseUrl: string,
231232
public readonly token: string,
232233
public readonly hostName: string,
234+
public readonly rootDirectory: string,
233235
private readonly disposable: Disposable,
234236
childProc: ChildProcess | undefined
235237
) {

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
'use strict';
4+
import * as path from 'path';
45
import * as uuid from 'uuid/v4';
56
import { CancellationToken, CancellationTokenSource, Event, EventEmitter, Uri } from 'vscode';
67

@@ -33,7 +34,7 @@ import {
3334
JupyterServerUriHandle
3435
} from '../types';
3536
import { JupyterSelfCertsError } from './jupyterSelfCertsError';
36-
import { createRemoteConnectionInfo } from './jupyterUtils';
37+
import { createRemoteConnectionInfo, expandWorkingDir } from './jupyterUtils';
3738
import { JupyterWaitForIdleError } from './jupyterWaitForIdleError';
3839
import { KernelSelector, KernelSpecInterpreter } from './kernels/kernelSelector';
3940
import { NotebookStarter } from './notebookStarter';
@@ -53,7 +54,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
5354
_liveShare: ILiveShareApi,
5455
private readonly interpreterService: IInterpreterService,
5556
private readonly disposableRegistry: IDisposableRegistry,
56-
workspace: IWorkspaceService,
57+
private readonly workspace: IWorkspaceService,
5758
private readonly configuration: IConfigurationService,
5859
private readonly kernelSelector: KernelSelector,
5960
private readonly notebookStarter: NotebookStarter,
@@ -361,9 +362,18 @@ export class JupyterExecutionBase implements IJupyterExecution {
361362
// If that works, then attempt to start the server
362363
traceInfo(`Launching ${options ? options.purpose : 'unknown type of'} server`);
363364
const useDefaultConfig = !options || options.skipUsingDefaultConfig ? false : true;
365+
366+
// Expand the working directory. Create a dummy launching file in the root path (so we expand correctly)
367+
const workingDirectory = expandWorkingDir(
368+
options?.workingDir,
369+
this.workspace.rootPath ? path.join(this.workspace.rootPath, `${uuid()}.txt`) : undefined,
370+
this.workspace
371+
);
372+
364373
const connection = await this.startNotebookServer(
365374
useDefaultConfig,
366375
this.configuration.getSettings(undefined).datascience.jupyterCommandLineArguments,
376+
workingDirectory,
367377
cancelToken
368378
);
369379
if (connection) {
@@ -389,9 +399,10 @@ export class JupyterExecutionBase implements IJupyterExecution {
389399
private async startNotebookServer(
390400
useDefaultConfig: boolean,
391401
customCommandLine: string[],
402+
workingDirectory: string,
392403
cancelToken?: CancellationToken
393404
): Promise<IJupyterConnection> {
394-
return this.notebookStarter.start(useDefaultConfig, customCommandLine, cancelToken);
405+
return this.notebookStarter.start(useDefaultConfig, customCommandLine, workingDirectory, cancelToken);
395406
}
396407
private onSettingsChanged() {
397408
// Clear our usableJupyterInterpreter so that we recompute our values

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,15 @@ export class JupyterServerBase implements INotebookServer {
8686

8787
// Create our session manager
8888
this.sessionManager = await this.sessionManagerFactory.create(launchInfo.connectionInfo);
89+
8990
// Try creating a session just to ensure we're connected. Callers of this function check to make sure jupyter
9091
// is running and connectable.
9192
let session: IJupyterSession | undefined;
92-
session = await this.sessionManager.startNew(launchInfo.kernelSpec, cancelToken);
93+
session = await this.sessionManager.startNew(
94+
launchInfo.kernelSpec,
95+
launchInfo.connectionInfo.rootDirectory,
96+
cancelToken
97+
);
9398
const idleTimeout = this.configService.getSettings().datascience.jupyterLaunchTimeout;
9499
// The wait for idle should throw if we can't connect.
95100
await session.waitForIdle(idleTimeout);

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
'use strict';
4-
import type { ContentsManager, Kernel, ServerConnection, Session, SessionManager } from '@jupyterlab/services';
4+
import type {
5+
Contents,
6+
ContentsManager,
7+
Kernel,
8+
ServerConnection,
9+
Session,
10+
SessionManager
11+
} from '@jupyterlab/services';
512
import * as path from 'path';
613
import * as uuid from 'uuid/v4';
714
import { CancellationToken } from 'vscode-jsonrpc';
@@ -29,9 +36,10 @@ export class JupyterSession extends BaseJupyterSession {
2936
private contentsManager: ContentsManager,
3037
private readonly outputChannel: IOutputChannel,
3138
private readonly restartSessionCreated: (id: Kernel.IKernelConnection) => void,
32-
restartSessionUsed: (id: Kernel.IKernelConnection) => void
39+
restartSessionUsed: (id: Kernel.IKernelConnection) => void,
40+
readonly workingDirectory: string
3341
) {
34-
super(restartSessionUsed);
42+
super(restartSessionUsed, workingDirectory);
3543
this.kernelSpec = kernelSpec;
3644
}
3745

@@ -137,11 +145,23 @@ export class JupyterSession extends BaseJupyterSession {
137145
contentsManager: ContentsManager,
138146
cancelToken?: CancellationToken
139147
): Promise<ISessionWithSocket> {
148+
// First make sure the notebook is in the right relative path (jupyter expects a relative path with unix delimiters)
149+
const relativeDirectory = path.relative(this.connInfo.rootDirectory, this.workingDirectory).replace(/\\/g, '/');
150+
151+
// However jupyter does not support relative paths outside of the original root.
152+
const backingFileOptions: Contents.ICreateOptions =
153+
this.connInfo.localLaunch && !relativeDirectory.startsWith('..')
154+
? { type: 'notebook', path: relativeDirectory }
155+
: { type: 'notebook' };
156+
140157
// Create a temporary notebook for this session. Each needs a unique name (otherwise we get the same session every time)
141-
let backingFile = await contentsManager.newUntitled({ type: 'notebook' });
158+
let backingFile = await contentsManager.newUntitled(backingFileOptions);
159+
const backingFileDir = path.dirname(backingFile.path);
142160
backingFile = await contentsManager.rename(
143161
backingFile.path,
144-
`${path.dirname(backingFile.path)}/t-${uuid()}.ipynb` // Note, the docs say the path uses UNIX delimiters.
162+
backingFileDir.length && backingFileDir !== '.'
163+
? `${backingFileDir}/t-${uuid()}.ipynb`
164+
: `t-${uuid()}.ipynb` // Note, the docs say the path uses UNIX delimiters.
145165
);
146166

147167
// Create our session options using this temporary notebook and our connection info
@@ -176,7 +196,7 @@ export class JupyterSession extends BaseJupyterSession {
176196
})
177197
.catch((ex) => Promise.reject(new JupyterSessionStartError(ex)))
178198
.finally(() => {
179-
if (this.connInfo && !this.connInfo.localLaunch) {
199+
if (this.connInfo) {
180200
this.contentsManager.delete(backingFile.path).ignoreErrors();
181201
}
182202
}),

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ export class JupyterSessionManager implements IJupyterSessionManager {
166166

167167
public async startNew(
168168
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined,
169+
workingDirectory: string,
169170
cancelToken?: CancellationToken
170171
): Promise<IJupyterSession> {
171172
if (!this.connInfo || !this.sessionManager || !this.contentsManager || !this.serverSettings) {
@@ -180,7 +181,8 @@ export class JupyterSessionManager implements IJupyterSessionManager {
180181
this.contentsManager,
181182
this.outputChannel,
182183
this.restartSessionCreatedEvent.fire.bind(this.restartSessionCreatedEvent),
183-
this.restartSessionUsedEvent.fire.bind(this.restartSessionUsedEvent)
184+
this.restartSessionUsedEvent.fire.bind(this.restartSessionUsedEvent),
185+
workingDirectory
184186
);
185187
try {
186188
await session.connect(this.configService.getSettings().datascience.jupyterLaunchTimeout, cancelToken);

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,30 @@ import { IJupyterConnection, IJupyterServerUri } from '../types';
1414

1515
export function expandWorkingDir(
1616
workingDir: string | undefined,
17-
launchingFile: string,
17+
launchingFile: string | undefined,
1818
workspace: IWorkspaceService
1919
): string {
2020
if (workingDir) {
21-
const variables = new SystemVariables(Uri.file(launchingFile), undefined, workspace);
21+
const variables = new SystemVariables(
22+
launchingFile ? Uri.file(launchingFile) : undefined,
23+
workspace.rootPath,
24+
workspace
25+
);
2226
return variables.resolve(workingDir);
2327
}
2428

2529
// No working dir, just use the path of the launching file.
26-
return path.dirname(launchingFile);
30+
if (launchingFile) {
31+
return path.dirname(launchingFile);
32+
}
33+
34+
// No launching file or working dir. Just use the default workspace folder
35+
const workspaceFolder = workspace.getWorkspaceFolder(undefined);
36+
if (workspaceFolder) {
37+
return workspaceFolder.uri.fsPath;
38+
}
39+
40+
return process.cwd();
2741
}
2842

2943
export function createRemoteConnectionInfo(
@@ -59,6 +73,7 @@ export function createRemoteConnectionInfo(
5973
return { dispose: noop };
6074
},
6175
dispose: noop,
76+
rootDirectory: '',
6277
getAuthHeader: serverUri ? () => getJupyterServerUri(uri)?.authorizationHeader : undefined
6378
};
6479
}

‎src/client/datascience/jupyter/liveshare/guestJupyterSessionManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ export class GuestJupyterSessionManager implements IJupyterSessionManager {
3434
}
3535
public startNew(
3636
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined,
37+
workingDirectory: string,
3738
cancelToken?: CancellationToken
3839
): Promise<IJupyterSession> {
39-
return this.realSessionManager.startNew(kernelSpec, cancelToken);
40+
return this.realSessionManager.startNew(kernelSpec, workingDirectory, cancelToken);
4041
}
4142

4243
public async getKernelSpecs(): Promise<IJupyterKernelSpec[]> {

‎src/client/datascience/jupyter/liveshare/hostJupyterExecution.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ export class HostJupyterExecution
186186
disconnected: (_l) => {
187187
return { dispose: noop };
188188
},
189-
dispose: noop
189+
dispose: noop,
190+
rootDirectory: connectionInfo.rootDirectory
190191
};
191192
}
192193
}

‎src/client/datascience/jupyter/liveshare/hostJupyterServer.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import '../../../common/extensions';
66
// tslint:disable-next-line: no-require-imports
77
import cloneDeep = require('lodash/cloneDeep');
88
import * as os from 'os';
9+
import * as path from 'path';
910
import * as vscode from 'vscode';
1011
import { CancellationToken } from 'vscode-jsonrpc';
1112
import * as vsls from 'vsls/vscode';
@@ -215,8 +216,17 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas
215216
);
216217
}
217218

218-
// Start a session (or use the existing one)
219-
const session = possibleSession || (await sessionManager.startNew(info.kernelSpec, cancelToken));
219+
// Figure out the working directory we need for our new notebook.
220+
const workingDirectory =
221+
resource && resource.scheme === 'file'
222+
? path.dirname(resource.fsPath)
223+
: this.workspaceService.getWorkspaceFolder(resource)?.uri.fsPath || process.cwd();
224+
225+
// Start a session (or use the existing one if allowed)
226+
const session =
227+
possibleSession && this.fs.areLocalPathsSame(possibleSession.workingDirectory, workingDirectory)
228+
? possibleSession
229+
: await sessionManager.startNew(info.kernelSpec, workingDirectory, cancelToken);
220230
traceInfo(`Started session ${this.id}`);
221231
return { info, session };
222232
};

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export class NotebookStarter implements Disposable {
6060
public async start(
6161
useDefaultConfig: boolean,
6262
customCommandLine: string[],
63+
workingDirectory: string,
6364
cancelToken?: CancellationToken
6465
): Promise<IJupyterConnection> {
6566
traceInfo('Starting Notebook');
@@ -71,7 +72,12 @@ export class NotebookStarter implements Disposable {
7172
const tempDirPromise = this.generateTempDir();
7273
tempDirPromise.then((dir) => this.disposables.push(dir)).ignoreErrors();
7374
// Before starting the notebook process, make sure we generate a kernel spec
74-
const args = await this.generateArguments(useDefaultConfig, customCommandLine, tempDirPromise);
75+
const args = await this.generateArguments(
76+
useDefaultConfig,
77+
customCommandLine,
78+
tempDirPromise,
79+
workingDirectory
80+
);
7581

7682
// Make sure we haven't canceled already.
7783
if (cancelToken && cancelToken.isCancellationRequested) {
@@ -108,6 +114,7 @@ export class NotebookStarter implements Disposable {
108114
starter = new JupyterConnectionWaiter(
109115
launchResult,
110116
tempDir.path,
117+
workingDirectory,
111118
this.jupyterInterpreterService.getRunningJupyterServers.bind(this.jupyterInterpreterService),
112119
this.serviceContainer,
113120
cancelToken
@@ -159,12 +166,13 @@ export class NotebookStarter implements Disposable {
159166

160167
private async generateDefaultArguments(
161168
useDefaultConfig: boolean,
162-
tempDirPromise: Promise<TemporaryDirectory>
169+
tempDirPromise: Promise<TemporaryDirectory>,
170+
workingDirectory: string
163171
): Promise<string[]> {
164172
// Parallelize as much as possible.
165173
const promisedArgs: Promise<string>[] = [];
166174
promisedArgs.push(Promise.resolve('--no-browser'));
167-
promisedArgs.push(this.getNotebookDirArgument(tempDirPromise));
175+
promisedArgs.push(Promise.resolve(this.getNotebookDirArgument(workingDirectory)));
168176
if (useDefaultConfig) {
169177
promisedArgs.push(this.getConfigArgument(tempDirPromise));
170178
}
@@ -192,10 +200,11 @@ export class NotebookStarter implements Disposable {
192200
private async generateArguments(
193201
useDefaultConfig: boolean,
194202
customCommandLine: string[],
195-
tempDirPromise: Promise<TemporaryDirectory>
203+
tempDirPromise: Promise<TemporaryDirectory>,
204+
workingDirectory: string
196205
): Promise<string[]> {
197206
if (!customCommandLine || customCommandLine.length === 0) {
198-
return this.generateDefaultArguments(useDefaultConfig, tempDirPromise);
207+
return this.generateDefaultArguments(useDefaultConfig, tempDirPromise, workingDirectory);
199208
}
200209
return this.generateCustomArguments(customCommandLine);
201210
}
@@ -208,9 +217,9 @@ export class NotebookStarter implements Disposable {
208217
* @returns {Promise<void>}
209218
* @memberof NotebookStarter
210219
*/
211-
private async getNotebookDirArgument(tempDirectory: Promise<TemporaryDirectory>): Promise<string> {
212-
const tempDir = await tempDirectory;
213-
return `--notebook-dir=${tempDir.path}`;
220+
private getNotebookDirArgument(workingDirectory: string): string {
221+
// Escape the result so Jupyter can actually read it.
222+
return `--notebook-dir="${workingDirectory.replace(/\\/g, '\\\\')}"`;
214223
}
215224

216225
/**

‎src/client/datascience/kernel-launcher/kernelDaemon.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ export class PythonKernelDaemon extends BasePythonDaemon implements IPythonKerne
6969
if (options.mergeStdOutErr) {
7070
throw new Error("'mergeStdOutErr' not supported in spawnOptions for KernelDaemon.start");
7171
}
72-
if (options.cwd) {
73-
throw new Error("'cwd' not supported in spawnOptions for KernelDaemon.start");
74-
}
7572
if (this.started) {
7673
throw new Error('Kernel has already been started in daemon');
7774
}

‎src/client/datascience/kernel-launcher/kernelLauncher.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export class KernelLauncher implements IKernelLauncher {
3535
public async launch(
3636
kernelSpec: IJupyterKernelSpec,
3737
resource: Resource,
38+
workingDirectory: string,
3839
interpreter?: PythonInterpreter
3940
): Promise<IKernelProcess> {
4041
const connection = await this.getKernelConnection();
@@ -47,7 +48,7 @@ export class KernelLauncher implements IKernelLauncher {
4748
resource,
4849
interpreter
4950
);
50-
await kernelProcess.launch();
51+
await kernelProcess.launch(workingDirectory);
5152
return kernelProcess;
5253
}
5354

0 commit comments

Comments
 (0)