Skip to content

fix: Install kernel spec into venv instead of user directory #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 9, 2025
Merged

fix: Install kernel spec into venv instead of user directory #29

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0f19d6
fix: Install kernel spec into venv instead of user directory
Artmann Oct 9, 2025
42007bd
pr feedback
Artmann Oct 9, 2025
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
12 changes: 10 additions & 2 deletions src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
const venvPath = this.getVenvPath(deepnoteFileUri);
const venvKey = venvPath.fsPath;

logger.info(`Virtual environment at ${venvKey}.`);
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Clarify the log message.

"Virtual environment at ${venvKey}." doesn't convey intent. Consider "Using virtual environment at ${venvKey}" or "Installing to virtual environment at ${venvKey}".

🤖 Prompt for AI Agents 
In src/kernels/deepnote/deepnoteToolkitInstaller.node.ts around line 68, the log
message "Virtual environment at ${venvKey}." is ambiguous; change it to a
clearer intentful message such as "Using virtual environment at ${venvKey}" or
"Installing to virtual environment at ${venvKey}" depending on context so
callers understand whether the venv is being used or written to; update the
logger.info call to include the chosen phrase and keep the venvKey
interpolation.


// Wait for any pending installation for this venv to complete
const pendingInstall = this.pendingInstallations.get(venvKey);
if (pendingInstall) {
Expand Down Expand Up @@ -210,6 +212,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
logger.info('deepnote-toolkit installed successfully in venv');

// Install kernel spec so the kernel uses this venv's Python
// Install into the venv itself (not --user) so the Deepnote server can discover it
logger.info('Installing kernel spec for venv...');
try {
// Reuse the process service with system environment
Expand All @@ -219,15 +222,20 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
'-m',
'ipykernel',
'install',
'--user',
'--prefix',
venvPath.fsPath,
'--name',
`deepnote-venv-${this.getVenvHash(deepnoteFileUri)}`,
'--display-name',
`Deepnote (${this.getDisplayName(deepnoteFileUri)})`
],
{ throwOnStdErr: false }
);
logger.info('Kernel spec installed successfully');
logger.info(
`Kernel spec installed successfully to ${
venvPath.fsPath
}/share/jupyter/kernels/deepnote-venv-${this.getVenvHash(deepnoteFileUri)}`
);
} catch (ex) {
logger.warn(`Failed to install kernel spec: ${ex}`);
// Don't fail the entire installation if kernel spec creation fails
Expand Down