Skip to content

Commit a8464f4

Browse files
hc2phc2p
committed
feat: add configuration-based APIs to DeepnoteToolkitInstaller and DeepnoteServerStarter
Refactor DeepnoteToolkitInstaller and DeepnoteServerStarter to support configuration-based kernel management alongside existing file-based workflow. DeepnoteToolkitInstaller changes: - Add ensureVenvAndToolkit() method for direct venv path management - Add installAdditionalPackages() for installing packages in existing venvs - Add getVenvInterpreterByPath() helper for venv path-based interpreter lookup - Add getKernelSpecName() and getKernelDisplayName() for venv-based naming - Refactor installImpl() to installVenvAndToolkit() using venv paths - Update kernel spec installation to use venv directory names instead of file hashes - Mark ensureInstalled() as deprecated, now delegates to ensureVenvAndToolkit() DeepnoteServerStarter changes: - Add startServer() method accepting venv path and configuration ID - Add stopServer() method accepting configuration ID instead of file URI - Add startServerForConfiguration() implementation for config-based lifecycle - Add stopServerForConfiguration() implementation for config-based cleanup - Mark getOrStartServer() as deprecated for backward compatibility - Update server process tracking to work with configuration IDs as keys These changes enable the kernel configuration manager to create and manage isolated Python environments without requiring .deepnote file associations. Legacy file-based methods are preserved for backward compatibility. Part of Phase 2: Refactoring Existing Services
1 parent 9eaeb9c commit a8464f4

File tree

2 files changed

+325
-42
lines changed

2 files changed

+325
-42
lines changed

‎src/kernels/deepnote/deepnoteServerStarter.node.ts‎

Lines changed: 211 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,89 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
6565
});
6666
}
6767

68+
/**
69+
* Configuration-based method: Start a server for a configuration.
70+
* @param interpreter The Python interpreter to use
71+
* @param venvPath The path to the venv
72+
* @param configurationId The configuration ID (used as key for server management)
73+
* @param token Cancellation token
74+
* @returns Server connection information
75+
*/
76+
public async startServer(
77+
interpreter: PythonEnvironment,
78+
venvPath: Uri,
79+
configurationId: string,
80+
token?: CancellationToken
81+
): Promise<DeepnoteServerInfo> {
82+
// Wait for any pending operations on this configuration to complete
83+
const pendingOp = this.pendingOperations.get(configurationId);
84+
if (pendingOp) {
85+
logger.info(`Waiting for pending operation on configuration ${configurationId} to complete...`);
86+
try {
87+
await pendingOp;
88+
} catch {
89+
// Ignore errors from previous operations
90+
}
91+
}
92+
93+
// If server is already running for this configuration, return existing info
94+
const existingServerInfo = this.serverInfos.get(configurationId);
95+
if (existingServerInfo && (await this.isServerRunning(existingServerInfo))) {
96+
logger.info(
97+
`Deepnote server already running at ${existingServerInfo.url} for configuration ${configurationId}`
98+
);
99+
return existingServerInfo;
100+
}
101+
102+
// Start the operation and track it
103+
const operation = this.startServerForConfiguration(interpreter, venvPath, configurationId, token);
104+
this.pendingOperations.set(configurationId, operation);
105+
106+
try {
107+
const result = await operation;
108+
return result;
109+
} finally {
110+
// Remove from pending operations when done
111+
if (this.pendingOperations.get(configurationId) === operation) {
112+
this.pendingOperations.delete(configurationId);
113+
}
114+
}
115+
}
116+
117+
/**
118+
* Configuration-based method: Stop the server for a configuration.
119+
* @param configurationId The configuration ID
120+
*/
121+
public async stopServer(configurationId: string): Promise<void> {
122+
// Wait for any pending operations on this configuration to complete
123+
const pendingOp = this.pendingOperations.get(configurationId);
124+
if (pendingOp) {
125+
logger.info(`Waiting for pending operation on configuration ${configurationId} before stopping...`);
126+
try {
127+
await pendingOp;
128+
} catch {
129+
// Ignore errors from previous operations
130+
}
131+
}
132+
133+
// Start the stop operation and track it
134+
const operation = this.stopServerForConfiguration(configurationId);
135+
this.pendingOperations.set(configurationId, operation);
136+
137+
try {
138+
await operation;
139+
} finally {
140+
// Remove from pending operations when done
141+
if (this.pendingOperations.get(configurationId) === operation) {
142+
this.pendingOperations.delete(configurationId);
143+
}
144+
}
145+
}
146+
147+
/**
148+
* Legacy file-based method (for backward compatibility).
149+
* @deprecated Use startServer instead
150+
*/
68151
public async getOrStartServer(
69152
interpreter: PythonEnvironment,
70153
deepnoteFileUri: Uri,
@@ -221,32 +304,141 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
221304
return serverInfo;
222305
}
223306

224-
public async stopServer(deepnoteFileUri: Uri): Promise<void> {
225-
const fileKey = deepnoteFileUri.fsPath;
307+
/**
308+
* Configuration-based server start implementation.
309+
*/
310+
private async startServerForConfiguration(
311+
interpreter: PythonEnvironment,
312+
venvPath: Uri,
313+
configurationId: string,
314+
token?: CancellationToken
315+
): Promise<DeepnoteServerInfo> {
316+
Cancellation.throwIfCanceled(token);
226317

227-
// Wait for any pending operations on this file to complete
228-
const pendingOp = this.pendingOperations.get(fileKey);
229-
if (pendingOp) {
230-
logger.info(`Waiting for pending operation on ${fileKey} before stopping...`);
231-
try {
232-
await pendingOp;
233-
} catch {
234-
// Ignore errors from previous operations
235-
}
318+
// Ensure toolkit is installed in venv
319+
logger.info(`Ensuring deepnote-toolkit is installed in venv for configuration ${configurationId}...`);
320+
const installed = await this.toolkitInstaller.ensureVenvAndToolkit(interpreter, venvPath, token);
321+
if (!installed) {
322+
throw new Error('Failed to install deepnote-toolkit. Please check the output for details.');
236323
}
237324

238-
// Start the stop operation and track it
239-
const operation = this.stopServerImpl(deepnoteFileUri);
240-
this.pendingOperations.set(fileKey, operation);
325+
Cancellation.throwIfCanceled(token);
326+
327+
// Find available port
328+
const port = await getPort({ host: 'localhost', port: DEEPNOTE_DEFAULT_PORT });
329+
logger.info(`Starting deepnote-toolkit server on port ${port} for configuration ${configurationId}`);
330+
this.outputChannel.appendLine(`Starting Deepnote server on port ${port}...`);
331+
332+
// Start the server with venv's Python in PATH
333+
const processService = await this.processServiceFactory.create(undefined);
334+
335+
// Set up environment to ensure the venv's Python is used for shell commands
336+
const venvBinDir = interpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, '');
337+
const env = { ...process.env };
338+
339+
// Prepend venv bin directory to PATH so shell commands use venv's Python
340+
env.PATH = `${venvBinDir}${process.platform === 'win32' ? ';' : ':'}${env.PATH || ''}`;
341+
342+
// Also set VIRTUAL_ENV to indicate we're in a venv
343+
env.VIRTUAL_ENV = venvPath.fsPath;
344+
345+
// Enforce published pip constraints to prevent breaking Deepnote Toolkit's dependencies
346+
env.DEEPNOTE_ENFORCE_PIP_CONSTRAINTS = 'true';
347+
348+
// Detached mode
349+
env.DEEPNOTE_RUNTIME__RUNNING_IN_DETACHED_MODE = 'true';
350+
351+
// Remove PYTHONHOME if it exists (can interfere with venv)
352+
delete env.PYTHONHOME;
353+
354+
const serverProcess = processService.execObservable(
355+
interpreter.uri.fsPath,
356+
['-m', 'deepnote_toolkit', 'server', '--jupyter-port', port.toString()],
357+
{ env }
358+
);
359+
360+
this.serverProcesses.set(configurationId, serverProcess);
361+
362+
// Track disposables for this configuration
363+
const disposables: IDisposable[] = [];
364+
this.disposablesByFile.set(configurationId, disposables);
365+
366+
// Monitor server output
367+
serverProcess.out.onDidChange(
368+
(output) => {
369+
if (output.source === 'stdout') {
370+
logger.trace(`Deepnote server (${configurationId}): ${output.out}`);
371+
this.outputChannel.appendLine(output.out);
372+
} else if (output.source === 'stderr') {
373+
logger.warn(`Deepnote server stderr (${configurationId}): ${output.out}`);
374+
this.outputChannel.appendLine(output.out);
375+
}
376+
},
377+
this,
378+
disposables
379+
);
380+
381+
// Wait for server to be ready
382+
const url = `http://localhost:${port}`;
383+
const serverInfo = { url, port };
384+
this.serverInfos.set(configurationId, serverInfo);
385+
386+
// Write lock file for the server process
387+
const serverPid = serverProcess.proc?.pid;
388+
if (serverPid) {
389+
await this.writeLockFile(serverPid);
390+
} else {
391+
logger.warn(`Could not get PID for server process for configuration ${configurationId}`);
392+
}
241393

242394
try {
243-
await operation;
244-
} finally {
245-
// Remove from pending operations when done
246-
if (this.pendingOperations.get(fileKey) === operation) {
247-
this.pendingOperations.delete(fileKey);
395+
const serverReady = await this.waitForServer(serverInfo, 120000, token);
396+
if (!serverReady) {
397+
await this.stopServerForConfiguration(configurationId);
398+
throw new Error('Deepnote server failed to start within timeout period');
399+
}
400+
} catch (error) {
401+
// Clean up leaked server before rethrowing
402+
await this.stopServerForConfiguration(configurationId);
403+
throw error;
404+
}
405+
406+
logger.info(`Deepnote server started successfully at ${url} for configuration ${configurationId}`);
407+
this.outputChannel.appendLine(`✓ Deepnote server running at ${url}`);
408+
409+
return serverInfo;
410+
}
411+
412+
/**
413+
* Configuration-based server stop implementation.
414+
*/
415+
private async stopServerForConfiguration(configurationId: string): Promise<void> {
416+
const serverProcess = this.serverProcesses.get(configurationId);
417+
418+
if (serverProcess) {
419+
const serverPid = serverProcess.proc?.pid;
420+
421+
try {
422+
logger.info(`Stopping Deepnote server for configuration ${configurationId}...`);
423+
serverProcess.proc?.kill();
424+
this.serverProcesses.delete(configurationId);
425+
this.serverInfos.delete(configurationId);
426+
this.outputChannel.appendLine(`Deepnote server stopped for configuration ${configurationId}`);
427+
428+
// Clean up lock file after stopping the server
429+
if (serverPid) {
430+
await this.deleteLockFile(serverPid);
431+
}
432+
} catch (ex) {
433+
logger.error(`Error stopping Deepnote server: ${ex}`);
248434
}
249435
}
436+
437+
const disposables = this.disposablesByFile.get(configurationId);
438+
if (disposables) {
439+
disposables.forEach((d) => d.dispose());
440+
this.disposablesByFile.delete(configurationId);
441+
}
250442
}
251443

252444
private async stopServerImpl(deepnoteFileUri: Uri): Promise<void> {

0 commit comments

Comments
 (0)