Skip to content

Commit 6a24088

Browse files
authored
Add support for creating environments with existing python virtual environments (#257)
* Add support for creating environments with existing python virtual environments * Update tests * Add tests for external deepnote environments, other minor changes - Added timeout to Python execution for virtual environment detection. - Implemented tests for renaming, deleting, and switching between managed and external environments. - Updated package management for external environments. - Improved error messages for missing deepnote-toolkit installation. * Implement installation of deepnote-toolkit in existing virtual environments - Added a new method `installToolkitInExistingVenv` to the `DeepnoteToolkitInstaller` class for installing the toolkit in an existing external virtual environment. - Updated the `IDeepnoteToolkitInstaller` interface to include the new method. - Enhanced error handling and logging during the installation process. - Modified the `handleKernelSelectionError` method to provide user-friendly actions for missing toolkit installations. - Introduced a new private method `installToolkitAndNotify` in `DeepnoteKernelAutoSelector` to handle toolkit installation and notify users upon completion. * Enhance Deepnote toolkit installation handling - Added logic to wait for any pending installations in the `DeepnoteToolkitInstaller` class, improving user experience during concurrent installation requests. - Updated the error message in `DeepnoteKernelAutoSelector` to inform users about the required deepnote-toolkit version for running Deepnote projects, enhancing clarity and guidance for users. * Properly track toolkit installation promises * Remove commented out code
1 parent 4cdf4a6 commit 6a24088

17 files changed

Lines changed: 836 additions & 160 deletions

src/kernels/deepnote/deepnoteServerStarter.node.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,15 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
9595
* Environment-based method: Start a server for a kernel environment.
9696
* @param interpreter The Python interpreter to use
9797
* @param venvPath The path to the venv
98+
* @param managedVenv Whether the venv is managed by this extension (created by us)
9899
* @param environmentId The environment ID (used as key for server management)
99100
* @param token Cancellation token
100101
* @returns Server connection information
101102
*/
102103
public async startServer(
103104
interpreter: PythonEnvironment,
104105
venvPath: Uri,
106+
managedVenv: boolean,
105107
additionalPackages: string[],
106108
environmentId: string,
107109
deepnoteFileUri: Uri,
@@ -165,6 +167,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
165167
existingContext,
166168
interpreter,
167169
venvPath,
170+
managedVenv,
168171
additionalPackages,
169172
environmentId,
170173
deepnoteFileUri,
@@ -235,6 +238,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
235238
projectContext: ProjectContext,
236239
interpreter: PythonEnvironment,
237240
venvPath: Uri,
241+
managedVenv: boolean,
238242
additionalPackages: string[],
239243
environmentId: string,
240244
deepnoteFileUri: Uri,
@@ -250,6 +254,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
250254
const { pythonInterpreter: venvInterpreter } = await this.toolkitInstaller.ensureVenvAndToolkit(
251255
interpreter,
252256
venvPath,
257+
managedVenv,
253258
token
254259
);
255260

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

Lines changed: 144 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@
44
import { inject, injectable, named } from 'inversify';
55
import { CancellationToken, l10n, Uri, workspace } from 'vscode';
66

7+
import { Cancellation } from '../../platform/common/cancellation';
78
import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants';
89
import { IFileSystem } from '../../platform/common/platform/types';
910
import { IProcessServiceFactory } from '../../platform/common/process/types.node';
1011
import { IExtensionContext, IOutputChannel } from '../../platform/common/types';
11-
import { DeepnoteToolkitInstallError, DeepnoteVenvCreationError } from '../../platform/errors/deepnoteKernelErrors';
12+
import {
13+
DeepnoteToolkitInstallError,
14+
DeepnoteToolkitMissingError,
15+
DeepnoteVenvCreationError
16+
} from '../../platform/errors/deepnoteKernelErrors';
1217
import { logger } from '../../platform/logging';
1318
import { PythonEnvironment } from '../../platform/pythonEnvironments/info';
1419
import { DEEPNOTE_TOOLKIT_VERSION, IDeepnoteToolkitInstaller, VenvAndToolkitInstallation } from './types';
15-
import { Cancellation } from '../../platform/common/cancellation';
1620

1721
/**
1822
* Handles installation of the deepnote-toolkit Python package.
@@ -77,6 +81,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
7781
public async ensureVenvAndToolkit(
7882
baseInterpreter: PythonEnvironment,
7983
venvPath: Uri,
84+
managedVenv: boolean,
8085
token?: CancellationToken
8186
): Promise<VenvAndToolkitInstallation> {
8287
const venvKey = venvPath.fsPath;
@@ -86,7 +91,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
8691

8792
// Validate that venv path is in current globalStorage (not from a different editor like VS Code)
8893
const expectedStoragePrefix = this.context.globalStorageUri.fsPath;
89-
if (!venvKey.startsWith(expectedStoragePrefix)) {
94+
if (managedVenv && !venvKey.startsWith(expectedStoragePrefix)) {
9095
const error = new Error(
9196
`Venv path mismatch! Expected venv under ${expectedStoragePrefix} but got ${venvKey}. ` +
9297
`This might happen if the notebook was previously used in a different editor (VS Code vs Cursor).`
@@ -139,6 +144,10 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
139144
}
140145
}
141146

147+
if (!managedVenv) {
148+
throw new DeepnoteToolkitMissingError(baseInterpreter.uri.fsPath, venvPath.fsPath);
149+
}
150+
142151
// Start the installation and track it
143152
const installation = this.installVenvAndToolkit(baseInterpreter, venvPath, token);
144153
this.pendingInstallations.set(venvKey, installation);
@@ -203,6 +212,46 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
203212
}
204213
}
205214

215+
/**
216+
* Install deepnote-toolkit in an existing external venv.
217+
* This is used when the user has an external venv without toolkit installed.
218+
* @param venvPath Path to the existing venv
219+
* @param token Cancellation token
220+
* @returns The venv Python interpreter and toolkit version if successful
221+
*/
222+
public async installToolkitInExistingVenv(
223+
venvPath: Uri,
224+
token?: CancellationToken
225+
): Promise<VenvAndToolkitInstallation> {
226+
const venvKey = venvPath.fsPath;
227+
228+
// Wait for any pending installation
229+
const pendingInstall = this.pendingInstallations.get(venvKey);
230+
if (pendingInstall) {
231+
logger.info(`Waiting for pending installation for ${venvKey}...`);
232+
return pendingInstall;
233+
}
234+
235+
const venvInterpreter = await this.getVenvInterpreterByPath(venvPath);
236+
if (!venvInterpreter) {
237+
throw new Error(`Venv not found at ${venvPath.fsPath}`);
238+
}
239+
240+
logger.info(`Installing deepnote-toolkit in existing venv at ${venvPath.fsPath}`);
241+
this.outputChannel.appendLine(l10n.t('Installing deepnote-toolkit in existing environment...'));
242+
243+
const installation = this.installToolkitPackages(venvInterpreter, venvPath, token);
244+
this.pendingInstallations.set(venvKey, installation);
245+
246+
try {
247+
return await installation;
248+
} finally {
249+
if (this.pendingInstallations.get(venvKey) === installation) {
250+
this.pendingInstallations.delete(venvKey);
251+
}
252+
}
253+
}
254+
206255
/**
207256
* Install venv and toolkit at a specific path (environment-based).
208257
*/
@@ -257,101 +306,114 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
257306
);
258307
}
259308

260-
// Use undefined as resource to get full system environment (including git in PATH)
261-
const venvProcessService = await this.processServiceFactory.create(undefined);
309+
// Use the shared helper method to install toolkit packages
310+
return await this.installToolkitPackages(venvInterpreter, venvPath, token);
311+
} catch (ex) {
312+
// If this is already a DeepnoteKernelError, rethrow it without wrapping
313+
if (ex instanceof DeepnoteVenvCreationError || ex instanceof DeepnoteToolkitInstallError) {
314+
throw ex;
315+
}
262316

263-
// Upgrade pip in the venv to the latest version
264-
logger.info('Upgrading pip in venv to latest version...');
265-
this.outputChannel.appendLine(l10n.t('Upgrading pip...'));
266-
const pipUpgradeResult = await venvProcessService.exec(
267-
venvInterpreter.uri.fsPath,
268-
['-m', 'pip', 'install', '--upgrade', 'pip'],
269-
{ throwOnStdErr: false }
317+
// Otherwise, log full details and wrap in a generic toolkit install error
318+
logger.error('Failed to set up deepnote-toolkit', ex);
319+
this.outputChannel.appendLine(l10n.t('Failed to set up deepnote-toolkit; see logs for details'));
320+
321+
throw new DeepnoteToolkitInstallError(
322+
baseInterpreter.uri.fsPath,
323+
venvPath.fsPath,
324+
DEEPNOTE_TOOLKIT_VERSION,
325+
'',
326+
ex instanceof Error ? ex.message : String(ex),
327+
ex instanceof Error ? ex : undefined
270328
);
329+
}
330+
}
271331

272-
if (pipUpgradeResult.stdout) {
273-
logger.info(`pip upgrade output: ${pipUpgradeResult.stdout}`);
274-
}
275-
if (pipUpgradeResult.stderr) {
276-
logger.info('pip upgrade stderr', pipUpgradeResult.stderr);
277-
}
332+
/**
333+
* Shared helper to install toolkit packages in a venv.
334+
* Used by both installVenvAndToolkit (managed venvs) and installToolkitInExistingVenv (external venvs).
335+
*/
336+
private async installToolkitPackages(
337+
venvInterpreter: PythonEnvironment,
338+
venvPath: Uri,
339+
token?: CancellationToken
340+
): Promise<VenvAndToolkitInstallation> {
341+
// Use undefined as resource to get full system environment (including git in PATH)
342+
const venvProcessService = await this.processServiceFactory.create(undefined);
278343

279-
Cancellation.throwIfCanceled(token);
344+
// Upgrade pip in the venv to the latest version
345+
logger.info('Upgrading pip in venv to latest version...');
346+
this.outputChannel.appendLine(l10n.t('Upgrading pip...'));
347+
const pipUpgradeResult = await venvProcessService.exec(
348+
venvInterpreter.uri.fsPath,
349+
['-m', 'pip', 'install', '--upgrade', 'pip'],
350+
{ throwOnStdErr: false }
351+
);
280352

281-
// Install deepnote-toolkit, ipykernel, and python-lsp-server in venv
282-
logger.info(
283-
`Installing deepnote-toolkit (${DEEPNOTE_TOOLKIT_VERSION}), ipykernel, and python-lsp-server in venv from PyPI`
284-
);
285-
this.outputChannel.appendLine(l10n.t('Installing deepnote-toolkit, ipykernel, and python-lsp-server...'));
353+
if (pipUpgradeResult.stdout) {
354+
logger.info(`pip upgrade output: ${pipUpgradeResult.stdout}`);
355+
}
356+
if (pipUpgradeResult.stderr) {
357+
logger.info('pip upgrade stderr', pipUpgradeResult.stderr);
358+
}
286359

287-
const installResult = await venvProcessService.exec(
288-
venvInterpreter.uri.fsPath,
289-
[
290-
'-m',
291-
'pip',
292-
'install',
293-
'--upgrade',
294-
`deepnote-toolkit[server]==${DEEPNOTE_TOOLKIT_VERSION}`,
295-
'ipykernel',
296-
'python-lsp-server[all]'
297-
],
298-
{ throwOnStdErr: false }
299-
);
360+
Cancellation.throwIfCanceled(token);
300361

301-
Cancellation.throwIfCanceled(token);
362+
// Install deepnote-toolkit, ipykernel, and python-lsp-server in venv
363+
logger.info(
364+
`Installing deepnote-toolkit (${DEEPNOTE_TOOLKIT_VERSION}), ipykernel, and python-lsp-server in venv from PyPI`
365+
);
366+
this.outputChannel.appendLine(l10n.t('Installing deepnote-toolkit, ipykernel, and python-lsp-server...'));
302367

303-
if (installResult.stdout) {
304-
this.outputChannel.appendLine(installResult.stdout);
305-
}
306-
if (installResult.stderr) {
307-
this.outputChannel.appendLine(installResult.stderr);
308-
}
368+
const installResult = await venvProcessService.exec(
369+
venvInterpreter.uri.fsPath,
370+
[
371+
'-m',
372+
'pip',
373+
'install',
374+
'--upgrade',
375+
`deepnote-toolkit[server]==${DEEPNOTE_TOOLKIT_VERSION}`,
376+
'ipykernel',
377+
'python-lsp-server[all]'
378+
],
379+
{ throwOnStdErr: false }
380+
);
309381

310-
// Verify installation
311-
const installedToolkitVersion = await this.isToolkitInstalled(venvInterpreter);
312-
if (installedToolkitVersion != null) {
313-
logger.info('deepnote-toolkit installed successfully in venv');
382+
Cancellation.throwIfCanceled(token);
314383

315-
// Install kernel spec so the kernel uses this venv's Python
316-
try {
317-
Cancellation.throwIfCanceled(token);
318-
await this.installKernelSpec(venvInterpreter, venvPath, token);
319-
} catch (ex) {
320-
logger.warn('Failed to install kernel spec', ex);
321-
// Don't fail the entire installation if kernel spec creation fails
322-
}
384+
if (installResult.stdout) {
385+
this.outputChannel.appendLine(installResult.stdout);
386+
}
387+
if (installResult.stderr) {
388+
this.outputChannel.appendLine(installResult.stderr);
389+
}
323390

324-
this.outputChannel.appendLine(l10n.t('✓ Deepnote toolkit ready'));
325-
return { pythonInterpreter: venvInterpreter, toolkitVersion: installedToolkitVersion };
326-
} else {
327-
logger.error('deepnote-toolkit installation failed');
328-
this.outputChannel.appendLine(l10n.t('✗ deepnote-toolkit installation failed'));
391+
// Verify installation
392+
const installedToolkitVersion = await this.isToolkitInstalled(venvInterpreter);
393+
if (installedToolkitVersion != null) {
394+
logger.info('deepnote-toolkit installed successfully in venv');
329395

330-
throw new DeepnoteToolkitInstallError(
331-
venvInterpreter.uri.fsPath,
332-
venvPath.fsPath,
333-
DEEPNOTE_TOOLKIT_VERSION,
334-
installResult.stdout || '',
335-
installResult.stderr || 'Package installation completed but verification failed'
336-
);
337-
}
338-
} catch (ex) {
339-
// If this is already a DeepnoteKernelError, rethrow it without wrapping
340-
if (ex instanceof DeepnoteVenvCreationError || ex instanceof DeepnoteToolkitInstallError) {
341-
throw ex;
396+
// Install kernel spec so the kernel uses this venv's Python
397+
try {
398+
Cancellation.throwIfCanceled(token);
399+
await this.installKernelSpec(venvInterpreter, venvPath, token);
400+
} catch (ex) {
401+
logger.warn('Failed to install kernel spec', ex);
402+
// Don't fail the entire installation if kernel spec creation fails
342403
}
343404

344-
// Otherwise, log full details and wrap in a generic toolkit install error
345-
logger.error('Failed to set up deepnote-toolkit', ex);
346-
this.outputChannel.appendLine(l10n.t('Failed to set up deepnote-toolkit; see logs for details'));
405+
this.outputChannel.appendLine(l10n.t('✓ Deepnote toolkit ready'));
406+
return { pythonInterpreter: venvInterpreter, toolkitVersion: installedToolkitVersion };
407+
} else {
408+
logger.error('deepnote-toolkit installation failed');
409+
this.outputChannel.appendLine(l10n.t('✗ deepnote-toolkit installation failed'));
347410

348411
throw new DeepnoteToolkitInstallError(
349-
baseInterpreter.uri.fsPath,
412+
venvInterpreter.uri.fsPath,
350413
venvPath.fsPath,
351414
DEEPNOTE_TOOLKIT_VERSION,
352-
'',
353-
ex instanceof Error ? ex.message : String(ex),
354-
ex instanceof Error ? ex : undefined
415+
installResult.stdout || '',
416+
installResult.stderr || 'Package installation completed but verification failed'
355417
);
356418
}
357419
}
@@ -365,7 +427,8 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
365427
'import deepnote_toolkit; print(deepnote_toolkit.__version__)'
366428
]);
367429
logger.info(`isToolkitInstalled result: ${result.stdout}`);
368-
return result.stdout.trim();
430+
const version = result.stdout.trim();
431+
return version.length > 0 ? version : undefined;
369432
} catch (ex) {
370433
logger.debug('deepnote-toolkit not found', ex);
371434
return undefined;

src/kernels/deepnote/environments/deepnoteEnvironment.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ export interface DeepnoteEnvironment {
2828
*/
2929
venvPath: Uri;
3030

31+
/**
32+
* Whether the venv is managed by this extension (created by us).
33+
* If true, the venv can be deleted when the environment is removed.
34+
* If false, the venv is external and should be preserved.
35+
*/
36+
managedVenv: boolean;
37+
3138
/**
3239
* Server information (set when server is running)
3340
*/
@@ -71,6 +78,7 @@ export interface DeepnoteEnvironmentState {
7178
uri: string;
7279
};
7380
venvPath: string;
81+
managedVenv?: boolean;
7482
createdAt: string;
7583
lastUsedAt: string;
7684
packages?: string[];

0 commit comments

Comments
 (0)