fix: add pip upgrading to deepnote toolkit installation #20

jankuca · 2025-10-05T14:52:50Z

Summary by CodeRabbit

New Features
- Server-native Python kernel selection and persistent server/controller reuse for more stable notebooks.
Bug Fixes
- Fewer installation failures by upgrading pip inside venvs, reordering install steps, and treating venv warnings (stderr) as non‑fatal.
Chores
- Improved install logging and cancellation handling, ensured ipykernel gets installed in venvs, documentation updates, and added a Markdown formatting rule.

coderabbitai · 2025-10-05T14:53:16Z

📝 Walkthrough

Walkthrough

Adds a pip-upgrade step inside the created venv (python -m pip install --upgrade pip) with stdout/stderr logging and cancellation checks.
Defers creation/use of the venv process service until after venv creation and interpreter verification.
Installs deepnote-toolkit and ipykernel using the venv Python; installer stderr is logged but no longer causes a hard throw; cancellation checks added.
Surfaces venv-creation warnings (stderr) without failing the install flow.
Switches kernel handling to prefer server-native kernel specs (startUsingDeepnoteKernel) and venv-based environment config rather than registering per-venv kernel specs.
Adds Deepnote-specific public types/interfaces and services (DeepnoteKernelConnectionMetadata, IDeepnoteToolkitInstaller, IDeepnoteServerStarter, IDeepnoteKernelAutoSelector, DeepnoteServerInfo, DeepnoteServerProvider) and wires them into service registration.
Adds a Prettier override for Markdown files (tabWidth: 2).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant VenvCreator as Venv Creation
  participant VenvProcSvc as Venv Process Service
  participant PythonVenv as venv Python
  participant Installer as pip installer (toolkit + ipykernel)
  participant ServerMgr as Deepnote Server / Kernel Manager

  Note over Client,VenvCreator: start venv creation
  Client->>VenvCreator: create venv
  VenvCreator-->>Client: venv path + stderr warnings (logged) / interpreter verified

  Note over VenvCreator,VenvProcSvc: after venv created
  VenvCreator->>VenvProcSvc: create process service for venv
  VenvProcSvc->>PythonVenv: run "python -m pip install --upgrade pip"
  PythonVenv-->>VenvProcSvc: stdout/stderr (logged) / cancellation check

  VenvProcSvc->>Installer: run pip install deepnote-toolkit ipykernel (use venv Python)
  Installer-->>VenvProcSvc: stdout/stderr (logged, do not throw on stderr) / cancellation check

  VenvProcSvc->>ServerMgr: provide venv-based env (PATH, VIRTUAL_ENV, PYTHONHOME)
  ServerMgr-->>Client: prefer server-native kernel spec (startUsingDeepnoteKernel) / server started or reused

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the main change of adding a pip upgrade step to the Deepnote toolkit installation process and aligns with the key modifications in the PR by focusing on pip upgrading in the installer flow.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 940b663 and 41cb843.

📒 Files selected for processing (1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md (12 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-09-12T12:52:09.726Z

Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitions

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

📚 Learning: 2025-09-25T13:16:13.796Z

Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

📚 Learning: 2025-09-03T12:54:35.368Z

Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

🪛 markdownlint-cli2 (0.18.1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md

182-182: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

187-187: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

192-192: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)

229-229: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

235-235: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c35f414 and b67da89.

📒 Files selected for processing (1)

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use *.node.ts for Desktop-specific implementations that require full file system access and Python environments

Files:

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation with CancellationToken

**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility, then alphabetically
Separate third-party imports from local file imports

Files:

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

🧬 Code graph analysis (1)

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)

src/platform/logging/index.ts (1)

logger (35-48)

src/kernels/execution/cellExecutionCreator.ts (1)

token (41-43)

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b67da89 and 12fd559.

📒 Files selected for processing (1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md (6 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.

🪛 markdownlint-cli2 (0.18.1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md

51-51: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

52-52: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

53-53: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

116-116: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

185-185: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

190-190: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

195-195: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)

232-232: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

238-238: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

421-421: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

424-424: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

425-425: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

426-426: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

DEEPNOTE_KERNEL_IMPLEMENTATION.md

…tall

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md (1)
115-161: Annotate the flow diagram fence.

The fence still lacks a language marker; add one (e.g., ```text) so MD040 stops failing.
-```
+```text

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12fd559 and 2018067.

📒 Files selected for processing (1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md (7 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.

📚 Learning: 2025-09-25T13:16:13.796Z

Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

🪛 markdownlint-cli2 (0.18.1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md

50-50: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

51-51: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

52-52: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

115-115: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

182-182: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

187-187: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

192-192: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)

229-229: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

235-235: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

415-415: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

416-416: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

417-417: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

DEEPNOTE_KERNEL_IMPLEMENTATION.md

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2018067 and 940b663.

📒 Files selected for processing (2)

.prettierrc.js (1 hunks)
DEEPNOTE_KERNEL_IMPLEMENTATION.md (12 hunks)

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.

📚 Learning: 2025-09-12T12:52:09.726Z

Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitions

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

📚 Learning: 2025-09-25T13:16:13.796Z

Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

📚 Learning: 2025-09-03T12:54:35.368Z

Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures

Applied to files:

DEEPNOTE_KERNEL_IMPLEMENTATION.md

🪛 markdownlint-cli2 (0.18.1)

DEEPNOTE_KERNEL_IMPLEMENTATION.md

115-115: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

182-182: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

187-187: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

192-192: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)

229-229: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

235-235: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)

🔇 Additional comments (1)

.prettierrc.js (1)

14-19: LGTM.

Markdown override mirrors the YAML pattern correctly. tabWidth consistency maintained.

DEEPNOTE_KERNEL_IMPLEMENTATION.md

fix: add pip upgrading to deepnote toolkit installation

b67da89

coderabbitai bot requested changes Oct 5, 2025

View reviewed changes

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts Show resolved Hide resolved

coderabbitai bot approved these changes Oct 5, 2025

View reviewed changes

jankuca added 2 commits October 5, 2025 17:52

chore: auto-format DEEPNOTE_KERNEL_IMPLEMENTATION.md

fb0ea81

chore: add info about pip upgrade to DEEPNOTE_KERNEL_IMPLEMENTATION.md

12fd559

coderabbitai bot requested changes Oct 5, 2025

View reviewed changes

DEEPNOTE_KERNEL_IMPLEMENTATION.md Outdated Show resolved Hide resolved

m1so approved these changes Oct 5, 2025

View reviewed changes

chore: update DEEPNOTE_KERNEL_IMPLEMENTATION.md to better explain ins…

2018067

…tall

coderabbitai bot requested changes Oct 5, 2025

View reviewed changes

DEEPNOTE_KERNEL_IMPLEMENTATION.md Outdated Show resolved Hide resolved

jankuca added 2 commits October 5, 2025 18:16

chore: change markdown formatting tabwidth to 2 spaces

0bd252c

chore: reformat DEEPNOTE_KERNEL_IMPLEMENTATION.md with new md settings

940b663

coderabbitai bot requested changes Oct 5, 2025

View reviewed changes

DEEPNOTE_KERNEL_IMPLEMENTATION.md Outdated Show resolved Hide resolved

DEEPNOTE_KERNEL_IMPLEMENTATION.md Show resolved Hide resolved

chore: fix md lint issues about fenced block

41cb843

coderabbitai bot approved these changes Oct 5, 2025

View reviewed changes

jankuca marked this pull request as ready for review October 5, 2025 16:26

jankuca merged commit c5b24c3 into main Oct 5, 2025
3 checks passed

jankuca deleted the jk/fix/pip-install branch October 5, 2025 16:26

This was referenced Oct 9, 2025

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

Merged

feat: add pandas and numpy support #43

Closed

This was referenced Oct 16, 2025

feat: Deepnote kernel management view #52

Merged

feat: Better error messages when installing the Deepnote kernel. #77

Merged

This was referenced Oct 31, 2025

chore: Upgrade Toolkit to v1 #146

Merged

fix: migrate venv paths for cross-editor compatibility and legacy naming #188

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: add pip upgrading to deepnote toolkit installation #20

fix: add pip upgrading to deepnote toolkit installation #20

Uh oh!

jankuca commented Oct 5, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 5, 2025 •

edited

Loading

Walkthrough

Sequence Diagram(s)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: add pip upgrading to deepnote toolkit installation #20

fix: add pip upgrading to deepnote toolkit installation #20

Uh oh!

Conversation

jankuca commented Oct 5, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Sequence Diagram(s)

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jankuca commented Oct 5, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 5, 2025 •

edited

Loading