Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 72 additions & 43 deletions src/kernels/deepnote/deepnoteServerStarter.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import * as fs from 'fs-extra';
import { inject, injectable, named, optional } from 'inversify';
import * as os from 'os';
import { CancellationToken, l10n, Uri } from 'vscode';
import { CancellationToken, CancellationTokenSource, l10n, Uri } from 'vscode';

import { startServer, stopServer } from '@deepnote/runtime-core';

Expand All @@ -24,8 +24,10 @@ import { logger } from '../../platform/logging';
import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types';
import { PythonEnvironment } from '../../platform/pythonEnvironments/info';
import * as path from '../../platform/vscode-path/path';
import { DeepnoteServerInfo, IDeepnoteServerStarter, IDeepnoteToolkitInstaller } from './types';
import { DeepnoteServerInfo, IDeepnoteServerStarter } from './types';
import { DeepnoteAgentSkillsManager } from './deepnoteAgentSkillsManager.node';
import { getCachedEnvironment } from '../../platform/interpreter/helpers';
import { IInstaller, InstallerResponse, Product } from '../../platform/interpreter/installer/types';

const MAX_OUTPUT_TRACKING_LENGTH = 5000;
const SERVER_STARTUP_TIMEOUT_MS = 120_000;
Expand All @@ -48,7 +50,7 @@ type PendingOperation =
};

interface ProjectContext {
environmentId: string;
interpreterId: string;
serverInfo: DeepnoteServerInfo | null;
}

Expand All @@ -71,7 +73,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension

constructor(
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
@inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller,
@inject(IInstaller) private readonly installer: IInstaller,
@inject(DeepnoteAgentSkillsManager) private readonly agentSkillsManager: DeepnoteAgentSkillsManager,
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel,
@inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
Expand All @@ -98,14 +100,11 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
*/
public async startServer(
interpreter: PythonEnvironment,
venvPath: Uri,
managedVenv: boolean,
additionalPackages: string[],
environmentId: string,
deepnoteFileUri: Uri,
token?: CancellationToken
): Promise<DeepnoteServerInfo> {
const fileKey = deepnoteFileUri.fsPath;
const interpreterId = interpreter.id;

let pendingOp = this.pendingOperations.get(fileKey);
if (pendingOp) {
Expand All @@ -119,12 +118,12 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension

let existingContext = this.projectContexts.get(fileKey);
if (existingContext != null) {
const { environmentId: existingEnvironmentId, serverInfo: existingServerInfo } = existingContext;
const { interpreterId: existingInterpreterId, serverInfo: existingServerInfo } = existingContext;

if (existingEnvironmentId === environmentId) {
if (existingInterpreterId === interpreterId) {
if (existingServerInfo != null && (await this.isServerRunning(existingServerInfo))) {
logger.info(
`Deepnote server already running at ${existingServerInfo.url} for ${fileKey} (environmentId ${environmentId})`
`Deepnote server already running at ${existingServerInfo.url} for ${fileKey} (interpreter ${interpreterId})`
);
return existingServerInfo;
}
Expand All @@ -136,14 +135,15 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
}
} else {
logger.info(
`Stopping existing server for ${fileKey} with environmentId ${existingEnvironmentId} to start new one with environmentId ${environmentId}...`
`Stopping existing server for ${fileKey} with interpreter ${existingInterpreterId} to start new one with interpreter ${interpreterId}...`
);
await this.stopServerForEnvironment(existingContext, deepnoteFileUri, token);
existingContext.environmentId = environmentId;
existingContext = { interpreterId, serverInfo: null };
this.projectContexts.set(fileKey, existingContext);
}
} else {
const newContext: ProjectContext = {
environmentId,
interpreterId,
serverInfo: null
};

Expand All @@ -153,16 +153,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension

const operation = {
type: 'start' as const,
promise: this.startServerForEnvironment(
existingContext,
interpreter,
venvPath,
managedVenv,
additionalPackages,
environmentId,
deepnoteFileUri,
token
)
promise: this.startServerForEnvironment(existingContext, interpreter, deepnoteFileUri, token)
};
this.pendingOperations.set(fileKey, operation);

Expand Down Expand Up @@ -223,53 +214,68 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
* Core server start using @deepnote/runtime-core's `startServer`.
*
* Extension-specific layers:
* - Toolkit/venv installation (before start)
* - Toolkit check/install via IInstaller (before start)
* - SQL integration env var injection (via ServerOptions.env)
* - Lock file creation (after start, using returned PID)
* - Output channel logging (via process stdout/stderr streams)
*/
private async startServerForEnvironment(
projectContext: ProjectContext,
interpreter: PythonEnvironment,
venvPath: Uri,
managedVenv: boolean,
additionalPackages: string[],
environmentId: string,
deepnoteFileUri: Uri,
token?: CancellationToken
): Promise<DeepnoteServerInfo> {
const fileKey = deepnoteFileUri.fsPath;
const interpreterId = interpreter.id;

Cancellation.throwIfCanceled(token);

logger.info(`Ensuring deepnote-toolkit is installed in venv for environment ${environmentId}...`);
const { pythonInterpreter: venvInterpreter } = await this.toolkitInstaller.ensureVenvAndToolkit(
interpreter,
venvPath,
managedVenv,
token
);
// Check if deepnote-toolkit is installed, and install if needed
logger.info(`Checking deepnote-toolkit installation for interpreter ${interpreterId}...`);
const isInstalled = await this.installer.isInstalled(Product.deepnoteToolkit, interpreter);

this.agentSkillsManager.ensureSkillsUpdated(environmentId, venvInterpreter);
if (!isInstalled) {
logger.info(`deepnote-toolkit not installed, installing via IInstaller...`);
const cts = new CancellationTokenSource();
let cancellationListener: IDisposable | undefined;

Cancellation.throwIfCanceled(token);
try {
if (token) {
cancellationListener = token.onCancellationRequested(() => cts.cancel());
}

const result = await this.installer.install(Product.deepnoteToolkit, interpreter, cts);

if (result === InstallerResponse.Cancelled) {
throw new Error('deepnote-toolkit installation was cancelled by the user');
} else if (result !== InstallerResponse.Installed) {
throw new Error('Failed to install deepnote-toolkit. Check the Output panel for details.');
}
} finally {
cancellationListener?.dispose();
cts.dispose();
}
}

await this.toolkitInstaller.installAdditionalPackages(venvPath, additionalPackages, token);
this.agentSkillsManager.ensureSkillsUpdated(interpreterId, interpreter);

Cancellation.throwIfCanceled(token);

logger.info(`Starting deepnote-toolkit server for ${fileKey} (environmentId ${environmentId})`);
// Derive the environment path from the interpreter
const envPath = this.deriveEnvPath(interpreter);

logger.info(`Starting deepnote-toolkit server for ${fileKey} (interpreter ${interpreterId})`);
this.outputChannel.appendLine(l10n.t('Starting Deepnote server...'));

const extraEnv = await this.gatherSqlIntegrationEnvVars(deepnoteFileUri, environmentId, token);
const extraEnv = await this.gatherSqlIntegrationEnvVars(deepnoteFileUri, interpreterId, token);

// Initialize output tracking for error reporting
this.serverOutputByFile.set(fileKey, { stdout: '', stderr: '' });

let serverInfo: DeepnoteServerInfo | undefined;
try {
serverInfo = await startServer({
pythonEnv: venvPath.fsPath,
pythonEnv: envPath,
workingDirectory: path.dirname(deepnoteFileUri.fsPath),
startupTimeoutMs: SERVER_STARTUP_TIMEOUT_MS,
env: extraEnv
Expand All @@ -280,7 +286,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension

throw new DeepnoteServerStartupError(
interpreter.uri.fsPath,
serverInfo?.jupyterPort ?? 0,
0,
'unknown',
capturedOutput?.stdout || '',
capturedOutput?.stderr || '',
Expand All @@ -307,6 +313,29 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
return serverInfo;
}

/**
* Derive the environment path from a Python interpreter.
* Uses the cached environment info, or falls back to navigating up from the executable.
*/
private deriveEnvPath(interpreter: PythonEnvironment): string {
const cachedEnv = getCachedEnvironment(interpreter);
// eslint-disable-next-line local-rules/dont-use-fspath
const folderPath = cachedEnv?.environment?.folderUri?.fsPath;

if (folderPath) {
return folderPath;
}

const sysPrefix = cachedEnv?.executable?.sysPrefix;

if (sysPrefix) {
return sysPrefix;
}

// Fallback: go up from bin/python (or Scripts/python.exe on Windows)
return path.dirname(path.dirname(interpreter.uri.fsPath));
}

/**
* Stop the server using @deepnote/runtime-core's `stopServer` (SIGTERM -> wait -> SIGKILL).
*/
Expand Down
16 changes: 10 additions & 6 deletions src/kernels/deepnote/deepnoteServerStarter.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { assert } from 'chai';
import * as fakeTimers from '@sinonjs/fake-timers';
import { anything, instance, mock, when } from 'ts-mockito';
import { EventEmitter } from 'vscode';

import { DeepnoteAgentSkillsManager } from './deepnoteAgentSkillsManager.node';
import { DeepnoteServerStarter } from './deepnoteServerStarter.node';
import { IProcessServiceFactory } from '../../platform/common/process/types.node';
import { IAsyncDisposableRegistry, IOutputChannel } from '../../platform/common/types';
import { IDeepnoteToolkitInstaller } from './types';
import { IInstaller, InstallerResponse } from '../../platform/interpreter/installer/types';
import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types';

/**
Expand All @@ -19,7 +20,7 @@ import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnot
suite('DeepnoteServerStarter', () => {
let serverStarter: DeepnoteServerStarter;
let mockProcessServiceFactory: IProcessServiceFactory;
let mockToolkitInstaller: IDeepnoteToolkitInstaller;
let mockInstaller: IInstaller;
let mockAgentSkillsManager: DeepnoteAgentSkillsManager;
let mockOutputChannel: IOutputChannel;
let mockAsyncRegistry: IAsyncDisposableRegistry;
Expand All @@ -32,18 +33,21 @@ suite('DeepnoteServerStarter', () => {

setup(() => {
mockProcessServiceFactory = mock<IProcessServiceFactory>();
mockToolkitInstaller = mock<IDeepnoteToolkitInstaller>();
mockInstaller = mock<IInstaller>();
mockAgentSkillsManager = mock<DeepnoteAgentSkillsManager>();
mockOutputChannel = mock<IOutputChannel>();
mockAsyncRegistry = mock<IAsyncDisposableRegistry>();
mockSqlIntegrationEnvVars = mock<ISqlIntegrationEnvVarsProvider>();

when(mockAsyncRegistry.push(anything())).thenReturn();
when(mockOutputChannel.appendLine(anything())).thenReturn();
when(mockInstaller.isInstalled(anything(), anything())).thenResolve(true);
when(mockInstaller.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed);
when(mockInstaller.onInstalled).thenReturn(new EventEmitter<any>().event);

serverStarter = new DeepnoteServerStarter(
instance(mockProcessServiceFactory),
instance(mockToolkitInstaller),
instance(mockInstaller),
instance(mockAgentSkillsManager),
instance(mockOutputChannel),
instance(mockAsyncRegistry),
Expand All @@ -62,7 +66,7 @@ suite('DeepnoteServerStarter', () => {
// Create a starter without SQL provider
const starterWithoutSql = new DeepnoteServerStarter(
instance(mockProcessServiceFactory),
instance(mockToolkitInstaller),
instance(mockInstaller),
instance(mockAgentSkillsManager),
instance(mockOutputChannel),
instance(mockAsyncRegistry)
Expand All @@ -85,7 +89,7 @@ suite('DeepnoteServerStarter', () => {

const starterWithCancelledSql = new DeepnoteServerStarter(
instance(mockProcessServiceFactory),
instance(mockToolkitInstaller),
instance(mockInstaller),
instance(mockAgentSkillsManager),
instance(mockOutputChannel),
instance(mockAsyncRegistry),
Expand Down
16 changes: 4 additions & 12 deletions src/kernels/deepnote/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,32 +151,24 @@ export interface IDeepnoteToolkitInstaller {
export const IDeepnoteServerStarter = Symbol('IDeepnoteServerStarter');
export interface IDeepnoteServerStarter {
/**
* Starts a deepnote-toolkit Jupyter server for a kernel environment.
* Environment-based method.
* Starts a deepnote-toolkit Jupyter server using the active Python interpreter.
* Handles checking/installing deepnote-toolkit via the IInstaller infrastructure.
* @param interpreter The Python interpreter to use
* @param venvPath The path to the venv
* @param managedVenv Whether the venv is managed by this extension (created by us)
* @param environmentId The environment ID (for server management)
* @param deepnoteFileUri The URI of the .deepnote file
* @param token Cancellation token to cancel the operation
* @returns Connection information (URL, port, etc.)
*/
startServer(
interpreter: PythonEnvironment,
venvPath: vscode.Uri,
managedVenv: boolean,
additionalPackages: string[],
environmentId: string,
deepnoteFileUri: vscode.Uri,
token?: vscode.CancellationToken
): Promise<DeepnoteServerInfo>;

/**
* Stops the deepnote-toolkit server for a kernel environment.
* @param environmentId The environment ID
* Stops the deepnote-toolkit server for a .deepnote file.
* @param deepnoteFileUri The URI of the .deepnote file
* @param token Cancellation token to cancel the operation
*/
// stopServer(environmentId: string, token?: vscode.CancellationToken): Promise<void>;
stopServer(deepnoteFileUri: vscode.Uri, token?: vscode.CancellationToken): Promise<void>;

/**
Expand Down
Loading
Loading