diff --git a/.vscode/launch.json b/.vscode/launch.json index 8c305723c6..9c47f90ff8 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -9,7 +9,7 @@ "runtimeExecutable": "${execPath}", "args": [ "--extensionDevelopmentPath=${workspaceFolder}", - "--enable-proposed-api=Deepnote.vscode-deepnote" + "--enable-proposed-api" ], "smartStep": true, "sourceMaps": true, diff --git a/CLAUDE.md b/CLAUDE.md index 9f97847798..4338c0e3ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,17 +1,9 @@ ## Code Style & Organization - - Order method, fields and properties, first by accessibility and then by alphabetical order. - Don't add the Microsoft copyright header to new files. - Use `Uri.joinPath()` for constructing file paths to ensure platform-correct path separators (e.g., `Uri.joinPath(venvPath, 'share', 'jupyter', 'kernels')` instead of string concatenation with `/`) -- Follow established patterns, especially when importing new packages (e.g. instead of importing uuid directly, use the helper `import { generateUuid } from '../platform/common/uuid';`) - - -## Code conventions - -- Always run `npx prettier` before committing ## Testing - - Unit tests use Mocha/Chai framework with `.unit.test.ts` extension - Test files should be placed alongside the source files they test - Run all tests: `npm test` or `npm run test:unittests` @@ -19,16 +11,13 @@ - Tests run against compiled JavaScript files in `out/` directory - Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties - ## Project Structure - - VSCode extension for Jupyter notebooks - Uses dependency injection with inversify - Follows separation of concerns pattern - TypeScript codebase that compiles to `out/` directory ## Deepnote Integration - - Located in `src/notebooks/deepnote/` - Refactored architecture: - `deepnoteTypes.ts` - Type definitions @@ -39,4 +28,4 @@ - `deepnoteActivationService.ts` - VSCode activation - Whitespace is good for readability, add a blank line after const groups and before return statements - Separate third-party and local file imports -- How the extension works is described in @architecture.md +- How the extension works is described in @architecture.md \ No newline at end of file diff --git a/DEBUGGING_KERNEL_MANAGEMENT.md b/DEBUGGING_KERNEL_MANAGEMENT.md deleted file mode 100644 index f04794051c..0000000000 --- a/DEBUGGING_KERNEL_MANAGEMENT.md +++ /dev/null @@ -1,445 +0,0 @@ -# Debugging Kernel Configuration Management - -## Quick Start: See It In Action - -### 1. Launch Extension in Debug Mode - -1. Open this project in VS Code -2. Press **F5** (or Run > Start Debugging) -3. Select **"Extension"** configuration -4. A new **Extension Development Host** window opens - -### 2. Find the Kernel Management UI - -**In the Extension Development Host window:** - -1. Look for the **Deepnote icon** in the Activity Bar (left sidebar) -2. Click it to open the Deepnote view -3. You should see two sections: - - **DEEPNOTE EXPLORER** (existing notebook browser) - - **DEEPNOTE KERNEL CONFIGURATIONS** ⬅️ **NEW!** - -**Initial State:** -``` -DEEPNOTE KERNEL CONFIGURATIONS -└─ [+] Create New Configuration -``` - -### 3. Create Your First Configuration - -1. Click **"Create New Configuration"** button -2. Follow the wizard: - - **Select Python interpreter** (choose any available) - - **Enter name**: e.g., "My Test Config" - - **Enter packages** (optional): e.g., "pandas, numpy" -3. Watch the progress notification -4. Configuration appears in the tree! - -**After Creation:** -``` -DEEPNOTE KERNEL CONFIGURATIONS -├─ ⚪ My Test Config [Stopped] -│ ├─ Python: /usr/bin/python3.11 -│ ├─ Venv: .../deepnote-venvs/{uuid} -│ ├─ Packages: pandas, numpy -│ ├─ Created: 1/15/2025, 10:00:00 AM -│ └─ Last used: 1/15/2025, 10:00:00 AM -└─ [+] Create New Configuration -``` - -### 4. Start the Server - -1. Right-click the configuration -2. Select **"Start Server"** (or click the play button ▶️) -3. Watch the output channel for logs -4. Icon changes to 🟢 **[Running]** -5. Port and URL appear in the tree - -**Running State:** -``` -DEEPNOTE KERNEL CONFIGURATIONS -├─ 🟢 My Test Config [Running] -│ ├─ Port: 8888 -│ ├─ URL: http://localhost:8888 -│ ├─ Python: /usr/bin/python3.11 -│ ├─ Venv: .../deepnote-venvs/{uuid} -│ ├─ Packages: pandas, numpy -│ ├─ Created: 1/15/2025, 10:00:00 AM -│ └─ Last used: 1/15/2025, 10:05:23 AM -└─ [+] Create New Configuration -``` - -### 5. Use Configuration with a Notebook - -1. Open (or create) a `.deepnote` file -2. A **picker dialog** appears automatically -3. Select your configuration from the list -4. Notebook connects to the running server -5. Execute cells - they run in your configured environment! - ---- - -## Key Debugging Locations - -### Files to Set Breakpoints In - -#### **1. Activation & Initialization** -**File:** `src/kernels/deepnote/configurations/deepnoteConfigurationsActivationService.ts` - -**Key Lines:** -- Line 27: `activate()` - Entry point -- Line 30: `configurationManager.initialize()` - Load configs from storage - -```typescript -// Set breakpoint here to see extension activation -public activate(): void { - logger.info('Activating Deepnote kernel configurations view'); - // ... -} -``` - -#### **2. Creating Configurations** -**File:** `src/kernels/deepnote/configurations/deepnoteConfigurationsView.ts` - -**Key Lines:** -- Line 64: `createConfiguration()` command handler -- Line 238: `configurationManager.createConfiguration()` call - -```typescript -// Set breakpoint here to see configuration creation -private async createConfiguration(): Promise { - try { - // Step 1: Select Python interpreter - const api = await this.pythonApiProvider.getNewApi(); - // ... - } -} -``` - -#### **3. Configuration Manager (Business Logic)** -**File:** `src/kernels/deepnote/configurations/deepnoteConfigurationManager.ts` - -**Key Lines:** -- Line 45: `initialize()` - Load from storage -- Line 63: `createConfiguration()` - Create new config -- Line 174: `startServer()` - Start Jupyter server -- Line 215: `stopServer()` - Stop server - -```typescript -// Set breakpoint here to see config creation -public async createConfiguration(options: CreateKernelConfigurationOptions): Promise { - const id = uuid(); - const venvPath = Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', id); - // ... - this._onDidChangeConfigurations.fire(); // ← Watch this fire! -} -``` - -#### **4. TreeDataProvider (UI Updates)** -**File:** `src/kernels/deepnote/configurations/deepnoteConfigurationTreeDataProvider.ts` - -**Key Lines:** -- Line 19-21: Event listener setup -- Line 25: `refresh()` - Triggers tree update -- Line 32: `getChildren()` - VS Code calls this to refresh - -```typescript -// Set breakpoint here to see event propagation -constructor(private readonly configurationManager: IDeepnoteConfigurationManager) { - // Listen to configuration changes and refresh the tree - this.configurationManager.onDidChangeConfigurations(() => { - this.refresh(); // ← Breakpoint here! - }); -} -``` - -#### **5. Server Lifecycle** -**File:** `src/kernels/deepnote/configurations/deepnoteConfigurationManager.ts` - -**Key Lines:** -- Line 189: `ensureVenvAndToolkit()` - Create venv -- Line 192: `installAdditionalPackages()` - Install packages -- Line 197: `serverStarter.startServer()` - Launch Jupyter - -```typescript -// Set breakpoint here to see server startup -public async startServer(id: string): Promise { - const config = this.configurations.get(id); - if (!config) { - throw new Error(`Configuration not found: ${id}`); - } - - // First ensure venv is created and toolkit is installed - await this.toolkitInstaller.ensureVenvAndToolkit(config.pythonInterpreter, config.venvPath); - // ... - // Start the Jupyter server - const serverInfo = await this.serverStarter.startServer(config.pythonInterpreter, config.venvPath, id); -} -``` - -#### **6. Notebook Integration (Configuration Picker)** -**File:** `src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts` - -**Key Lines:** -- Look for `ensureKernelSelected()` method -- Look for calls to `configurationPicker.pickConfiguration()` -- Look for calls to `mapper.getConfigurationForNotebook()` - ---- - -## Visual Debugging Tips - -### **1. Watch the Output Channel** - -When the extension is running, look for: -- **Output Panel** (View > Output) -- Select **"Deepnote"** from the dropdown -- You'll see logs like: - -``` -[info] Activating Deepnote kernel configurations view -[info] Initialized configuration manager with 0 configurations -[info] Creating virtual environment at /path/to/venv -[info] Installing deepnote-toolkit and ipykernel in venv from https://... -[info] Created new kernel configuration: My Test Config (uuid-123) -[info] Starting server for configuration: My Test Config (uuid-123) -[info] Deepnote server started successfully at http://localhost:8888 -``` - -### **2. Watch VS Code's Developer Tools** - -Open Developer Tools: -- **Help > Toggle Developer Tools** -- **Console tab**: See any JavaScript errors -- **Network tab**: See server requests (when executing cells) - -### **3. Watch Global State (Persistence)** - -To see what's stored: - -1. Set breakpoint in `deepnoteConfigurationStorage.ts` -2. At line 56: `await this.globalState.update(STORAGE_KEY, states)` -3. Inspect the `states` variable -4. You'll see the JSON being persisted - -**Example stored data:** -```json -{ - "deepnote.kernelConfigurations": [ - { - "id": "abc-123-def", - "name": "My Test Config", - "pythonInterpreterPath": "/usr/bin/python3.11", - "venvPath": "/Users/.../deepnote-venvs/abc-123-def", - "createdAt": "2025-01-15T10:00:00.000Z", - "lastUsedAt": "2025-01-15T10:00:00.000Z", - "packages": ["pandas", "numpy"] - } - ] -} -``` - -### **4. Watch Server Processes** - -In your terminal: -```bash -# Find running deepnote-toolkit servers -ps aux | grep deepnote_toolkit - -# Or on Windows -tasklist | findstr python -``` - -You should see processes like: -``` -python -m deepnote_toolkit server --jupyter-port 8888 -``` - -### **5. Check Venv Directories** - -Navigate to: -```bash -# macOS/Linux -cd ~/.vscode/extensions/.../globalStorage/deepnote-venvs/ - -# Windows -cd %APPDATA%\Code\User\globalStorage\...\deepnote-venvs\ -``` - -You'll see directories named with UUIDs, each containing a Python venv. - ---- - -## Common Debugging Scenarios - -### **Scenario 1: Configuration Not Appearing in Tree** - -**Set breakpoints:** -1. `deepnoteConfigurationManager.ts:80` - Check if `_onDidChangeConfigurations.fire()` is called -2. `deepnoteConfigurationTreeDataProvider.ts:20` - Check if event listener is triggered -3. `deepnoteConfigurationTreeDataProvider.ts:25` - Check if `refresh()` is called -4. `deepnoteConfigurationTreeDataProvider.ts:46` - Check if `getRootItems()` returns the config - -**Debug steps:** -- Verify `this.configurations` Map contains the config -- Verify event propagation chain -- Check if tree view is registered with VS Code - -### **Scenario 2: Server Won't Start** - -**Set breakpoints:** -1. `deepnoteConfigurationManager.ts:174` - `startServer()` entry -2. `deepnoteToolkitInstaller.node.ts:74` - `ensureVenvAndToolkit()` entry -3. `deepnoteServerStarter.node.ts:76` - `startServer()` entry - -**Check:** -- Output channel for error messages -- Venv creation succeeded -- Python interpreter is valid -- Port is not already in use - -### **Scenario 3: Notebook Picker Not Appearing** - -**Set breakpoints:** -1. `deepnoteKernelAutoSelector.node.ts` - `ensureKernelSelected()` method -2. Check if `mapper.getConfigurationForNotebook()` returns undefined -3. Check if `picker.pickConfiguration()` is called - -**Verify:** -- Picker service is registered in DI container -- Mapper service is registered in DI container -- Notebook URI is being normalized correctly - ---- - -## EventEmitter Pattern Debugging - -To trace the event flow (from my earlier explanation): - -### **1. Set Breakpoints in Sequence:** - -```typescript -// 1. Manager fires event -// deepnoteConfigurationManager.ts:80 -this._onDidChangeConfigurations.fire(); - -// 2. TreeProvider receives event -// deepnoteConfigurationTreeDataProvider.ts:20 -this.configurationManager.onDidChangeConfigurations(() => { - this.refresh(); // ← Breakpoint - -// 3. TreeProvider fires its event -// deepnoteConfigurationTreeDataProvider.ts:25 -public refresh(): void { - this._onDidChangeTreeData.fire(); // ← Breakpoint - -// 4. VS Code calls getChildren -// deepnoteConfigurationTreeDataProvider.ts:32 -public async getChildren(element?: DeepnoteConfigurationTreeItem): Promise { - // ← Breakpoint -``` - -### **2. Watch Variables:** - -- In Manager: `this.configurations` - See all configs -- In TreeProvider: `this._onDidChangeTreeData` - See EventEmitter -- In `getChildren`: `element` parameter - See what VS Code is requesting - ---- - -## Testing the Complete Flow - -### **End-to-End Test:** - -1. **Create Configuration** - - Set breakpoint: `deepnoteConfigurationsView.ts:238` - - Click "Create New Configuration" - - Step through: select interpreter → enter name → create venv - -2. **Start Server** - - Set breakpoint: `deepnoteConfigurationManager.ts:197` - - Right-click config → "Start Server" - - Step through: install toolkit → start server → update state - -3. **Open Notebook** - - Set breakpoint: `deepnoteKernelAutoSelector.node.ts` (ensureKernelSelected) - - Open a `.deepnote` file - - Step through: check mapper → show picker → save selection - -4. **Execute Cell** - - Open notebook with selected configuration - - Execute a cell (e.g., `print("Hello")`) - - Watch server logs in Output channel - ---- - -## Quick Test Commands - -```bash -# 1. Build the extension -npm run compile - -# 2. Run unit tests -npm test - -# Or run specific test file -npx mocha --config ./build/.mocha.unittests.js.json \ - ./out/src/kernels/deepnote/configurations/deepnoteConfigurationManager.unit.test.js - -# 3. Check for TypeScript errors -npm run compile:check - -# 4. Format code -npx prettier --write . -``` - ---- - -## Useful VS Code Commands (in Extension Development Host) - -Press `Cmd+Shift+P` (Mac) or `Ctrl+Shift+P` (Windows/Linux): - -- **Developer: Reload Window** - Reload extension after code changes -- **Developer: Open Webview Developer Tools** - Debug webviews -- **Developer: Show Running Extensions** - See if your extension loaded -- **Deepnote: Create Kernel Configuration** - Test command directly -- **Deepnote: Refresh** - Manually refresh tree view - ---- - -## Summary: Where to Look - -| What You Want to See | Where to Look | File/Line | -|---------------------|---------------|-----------| -| **UI Tree View** | Sidebar → Deepnote → Kernel Configurations | Activity Bar | -| **Create Configuration** | Click "+ Create New Configuration" | View controller | -| **Configuration State** | Expand config item in tree | Tree data provider | -| **Server Logs** | Output panel → "Deepnote" channel | Output channel | -| **Persisted Data** | Inspect `globalState` in debugger | Storage layer | -| **Running Processes** | Terminal: `ps aux \| grep deepnote` | System | -| **Event Flow** | Breakpoints in Manager → Provider → getChildren | Event chain | -| **Notebook Picker** | Opens when you open a `.deepnote` file | Auto-selector | - ---- - -## Pro Tips - -1. **Use Logpoints** instead of console.log: - - Right-click in gutter → Add Logpoint - - Logs appear in Debug Console without modifying code - -2. **Watch Expressions:** - - Add to Watch panel: `this.configurations.size` - - See live count of configurations - -3. **Conditional Breakpoints:** - - Right-click breakpoint → Edit Breakpoint → Add condition - - Example: `config.id === "specific-uuid"` - -4. **Call Stack Navigation:** - - When breakpoint hits, examine Call Stack panel - - See the entire event propagation path - -5. **Restart Extension Fast:** - - In Debug toolbar, click restart button (circular arrow) - - Or use `Cmd+Shift+F5` / `Ctrl+Shift+F5` \ No newline at end of file diff --git a/ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md b/ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md new file mode 100644 index 0000000000..7bf862f0bc --- /dev/null +++ b/ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md @@ -0,0 +1,182 @@ +# Orphan Process Cleanup Implementation + +## Overview + +This document describes the implementation of a sophisticated orphan process cleanup mechanism for the Deepnote server starter that prevents terminating active servers from other VS Code windows. + +## Problem Statement + +Previously, the cleanup logic in `cleanupOrphanedProcesses()` would force-kill **every** process matching "deepnote_toolkit server", which could terminate active servers from other VS Code windows, causing disruption to users working in multiple windows. + +## Solution + +The new implementation uses a lock file system combined with parent process verification to only kill genuine orphan processes. + +## Key Components + +### 1. Session Management + +- **Session ID**: Each VS Code window instance generates a unique session ID using `generateUuid()` +- **Lock File Directory**: Lock files are stored in `${os.tmpdir()}/vscode-deepnote-locks/` +- **Lock File Format**: JSON files named `server-{pid}.json` containing: + ```typescript + interface ServerLockFile { + sessionId: string; // Unique ID for the VS Code window + pid: number; // Process ID of the server + timestamp: number; // When the server was started + } + ``` + +### 2. Lock File Lifecycle + +#### Creation + +- When a server starts successfully, a lock file is written with the server's PID and current session ID +- Location: `writeLockFile()` called in `startServerImpl()` after the server process is spawned + +#### Deletion + +- Lock files are deleted when: + 1. The server is explicitly stopped via `stopServerImpl()` + 2. The extension is disposed and all servers are shut down + 3. An orphaned process is successfully killed during cleanup + +### 3. Orphan Detection Logic + +The `isProcessOrphaned()` method checks if a process is truly orphaned by verifying its parent process: + +#### Unix/Linux/macOS + +```bash +# Get parent process ID +ps -o ppid= -p + +# Check if parent exists (using -o pid= to get only PID with no header) +ps -p -o pid= +``` + +- If PPID is 1 (init/systemd), the process is orphaned +- If parent process doesn't exist (empty stdout from `ps -o pid=`), the process is orphaned +- The `-o pid=` flag ensures no header is printed, so empty output reliably indicates a missing process + +#### Windows + +```cmd +# Get parent process ID +wmic process where ProcessId= get ParentProcessId + +# Check if parent exists +tasklist /FI "PID eq " /FO CSV /NH +``` + +- If parent process doesn't exist or PPID is 0, the process is orphaned + +### 4. Cleanup Decision Flow + +When `cleanupOrphanedProcesses()` runs (at extension startup): + +1. **Find all deepnote_toolkit server processes** + + - Use `ps aux` (Unix) or `tasklist` (Windows) + - Extract PIDs of matching processes + +2. **For each candidate PID:** + + a. **Check for lock file** + + - If lock file exists: + + - If session ID matches current session → **SKIP** (shouldn't happen at startup) + - If session ID differs: + - Check if process is orphaned + - If orphaned → **KILL** + - If not orphaned → **SKIP** (active in another window) + + - If no lock file exists: + - Check if process is orphaned + - If orphaned → **KILL** + - If not orphaned → **SKIP** (might be from older version without lock files) + +3. **Kill orphaned processes** + + - Use `kill -9` (Unix) or `taskkill /F /T` (Windows) + - Delete lock file after successful kill + +4. **Log all decisions** + - Processes to kill: logged with reason + - Processes to skip: logged with reason + - Provides full audit trail for debugging + +## Code Changes + +### Modified Files + +- `src/kernels/deepnote/deepnoteServerStarter.node.ts` + +### New Imports + +```typescript +import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import * as fs from 'fs-extra'; +import * as os from 'os'; +import * as path from '../../platform/vscode-path/path'; +import { generateUuid } from '../../platform/common/uuid'; +``` + +### New Class Members + +```typescript +private readonly sessionId: string = generateUuid(); +private readonly lockFileDir: string = path.join(os.tmpdir(), 'vscode-deepnote-locks'); +``` + +### New Methods + +1. `initializeLockFileDirectory()` - Creates lock file directory +2. `getLockFilePath(pid)` - Returns path to lock file for a PID +3. `writeLockFile(pid)` - Creates lock file for a server process +4. `readLockFile(pid)` - Reads lock file data +5. `deleteLockFile(pid)` - Removes lock file +6. `isProcessOrphaned(pid)` - Checks if process is orphaned by verifying parent + +### Modified Methods + +1. `constructor()` - Minimal initialization (dependency injection only) +2. `activate()` - Initializes lock file directory and triggers cleanup (implements IExtensionSyncActivationService) +3. `startServerImpl()` - Writes lock file after server starts +4. `stopServerImpl()` - Deletes lock file when server stops +5. `dispose()` - Deletes lock files for all stopped servers +6. `cleanupOrphanedProcesses()` - Implements sophisticated orphan detection + +## Benefits + +1. **Multi-Window Safety**: Active servers in other VS Code windows are never killed +2. **Backward Compatible**: Handles processes from older versions without lock files +3. **Robust Orphan Detection**: Uses OS-level parent process verification +4. **Full Audit Trail**: Comprehensive logging of all cleanup decisions +5. **Automatic Cleanup**: Stale lock files are removed when processes are killed +6. **Session Isolation**: Each VS Code window operates independently + +## Testing Recommendations + +1. **Single Window**: Verify servers start and stop correctly +2. **Multiple Windows**: Open multiple VS Code windows with Deepnote files, verify servers in other windows aren't killed +3. **Orphan Cleanup**: Kill VS Code process forcefully, restart, verify orphaned servers are cleaned up +4. **Lock File Cleanup**: Verify lock files are created and deleted appropriately +5. **Cross-Platform**: Test on Windows, macOS, and Linux + +## Edge Cases Handled + +1. **No lock file + active parent**: Process is skipped (might be from older version) +2. **Lock file + different session + active parent**: Process is skipped (active in another window) +3. **Lock file + same session**: Process is skipped (shouldn't happen at startup) +4. **No lock file + orphaned**: Process is killed (genuine orphan) +5. **Lock file + different session + orphaned**: Process is killed (orphaned from crashed window) +6. **Failed parent check**: Process is assumed not orphaned (safer default) + +## Future Enhancements + +1. **Stale Lock File Cleanup**: Periodically clean up lock files for non-existent processes +2. **Lock File Expiry**: Add TTL to lock files to handle edge cases +3. **Health Monitoring**: Periodic checks to ensure servers are still responsive +4. **Graceful Shutdown**: Try SIGTERM before SIGKILL for orphaned processes diff --git a/cspell.json b/cspell.json index ea3287b861..3f8516fbb9 100644 --- a/cspell.json +++ b/cspell.json @@ -41,7 +41,6 @@ "jupyter", "jupyterlab", "JVSC", - "matplotlib", "millis", "nbformat", "numpy", @@ -50,8 +49,6 @@ "Pids", "PYTHONHOME", "Reselecting", - "scipy", - "sklearn", "taskkill", "unconfigured", "Unconfigured", diff --git a/package-lock.json b/package-lock.json index b6a9964360..a418c429b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -73,7 +73,6 @@ "tcp-port-used": "^1.0.1", "tmp": "^0.2.4", "url-parse": "^1.5.10", - "uuid": "^13.0.0", "vega": "^5.33.0", "vega-embed": "^6.25.0", "vega-lite": "^5.21.0", @@ -134,6 +133,7 @@ "@types/temp": "^0.8.32", "@types/tmp": "^0.2.3", "@types/url-parse": "^1.4.8", + "@types/uuid": "^3.4.3", "@types/vscode-notebook-renderer": "^1.60.0", "@types/ws": "^6.0.1", "@typescript-eslint/eslint-plugin": "^6.9.0", @@ -3986,6 +3986,12 @@ "integrity": "sha512-zqqcGKyNWgTLFBxmaexGUKQyWqeG7HjXj20EuQJSJWwXe54BjX0ihIo5cJB9yAQzH8dNugJ9GvkBYMjPXs/PJw==", "dev": true }, + "node_modules/@types/uuid": { + "version": "3.4.10", + "resolved": "https://registry.npmjs.org/@types/uuid/-/uuid-3.4.10.tgz", + "integrity": "sha512-BgeaZuElf7DEYZhWYDTc/XcLZXdVgFkVSTa13BqKvbnmUrxr3TJFKofUxCtDO9UQOdhnV+HPOESdHiHKZOJV1A==", + "dev": true + }, "node_modules/@types/vscode-notebook-renderer": { "version": "1.60.0", "resolved": "https://registry.npmjs.org/@types/vscode-notebook-renderer/-/vscode-notebook-renderer-1.60.0.tgz", @@ -23490,6 +23496,12 @@ "integrity": "sha512-zqqcGKyNWgTLFBxmaexGUKQyWqeG7HjXj20EuQJSJWwXe54BjX0ihIo5cJB9yAQzH8dNugJ9GvkBYMjPXs/PJw==", "dev": true }, + "@types/uuid": { + "version": "3.4.10", + "resolved": "https://registry.npmjs.org/@types/uuid/-/uuid-3.4.10.tgz", + "integrity": "sha512-BgeaZuElf7DEYZhWYDTc/XcLZXdVgFkVSTa13BqKvbnmUrxr3TJFKofUxCtDO9UQOdhnV+HPOESdHiHKZOJV1A==", + "dev": true + }, "@types/vscode-notebook-renderer": { "version": "1.60.0", "resolved": "https://registry.npmjs.org/@types/vscode-notebook-renderer/-/vscode-notebook-renderer-1.60.0.tgz", diff --git a/package.json b/package.json index 9876021d86..e1cf46e974 100644 --- a/package.json +++ b/package.json @@ -93,59 +93,6 @@ "category": "Deepnote", "icon": "$(reveal)" }, - { - "command": "deepnote.environments.create", - "title": "%deepnote.commands.environments.create.title%", - "category": "Deepnote", - "icon": "$(add)" - }, - { - "command": "deepnote.environments.start", - "title": "%deepnote.commands.environments.start.title%", - "category": "Deepnote", - "icon": "$(debug-start)" - }, - { - "command": "deepnote.environments.stop", - "title": "%deepnote.commands.environments.stop.title%", - "category": "Deepnote", - "icon": "$(debug-stop)" - }, - { - "command": "deepnote.environments.restart", - "title": "%deepnote.commands.environments.restart.title%", - "category": "Deepnote", - "icon": "$(debug-restart)" - }, - { - "command": "deepnote.environments.delete", - "title": "%deepnote.commands.environments.delete.title%", - "category": "Deepnote", - "icon": "$(trash)" - }, - { - "command": "deepnote.environments.managePackages", - "title": "%deepnote.commands.environments.managePackages.title%", - "category": "Deepnote", - "icon": "$(package)" - }, - { - "command": "deepnote.environments.editName", - "title": "%deepnote.commands.environments.editName.title%", - "category": "Deepnote" - }, - { - "command": "deepnote.environments.refresh", - "title": "%deepnote.commands.environments.refresh.title%", - "category": "Deepnote", - "icon": "$(refresh)" - }, - { - "command": "deepnote.environments.selectForNotebook", - "title": "%deepnote.commands.environments.selectForNotebook.title%", - "category": "Deepnote", - "icon": "$(server-environment)" - }, { "command": "deepnote.manageIntegrations", "title": "%deepnote.commands.manageIntegrations.title%", @@ -160,19 +107,19 @@ }, { "command": "deepnote.newProject", - "title": "%deepnote.commands.newProject.title%", + "title": "New project", "category": "Deepnote", "icon": "$(new-file)" }, { "command": "deepnote.importNotebook", - "title": "%deepnote.commands.importNotebook.title%", + "title": "Import notebook", "category": "Deepnote", "icon": "$(folder-opened)" }, { "command": "deepnote.importJupyterNotebook", - "title": "%deepnote.commands.importJupyterNotebook.title%", + "title": "Import Jupyter notebook", "category": "Deepnote", "icon": "$(notebook)" }, @@ -815,16 +762,6 @@ "command": "deepnote.refreshExplorer", "when": "view == deepnoteExplorer", "group": "navigation@3" - }, - { - "command": "deepnote.environments.create", - "when": "view == deepnoteEnvironments", - "group": "navigation@4" - }, - { - "command": "deepnote.environments.refresh", - "when": "view == deepnoteEnvironments", - "group": "navigation@5" } ], "editor/context": [ @@ -920,64 +857,59 @@ } ], "notebook/toolbar": [ - { - "command": "deepnote.environments.selectForNotebook", - "group": "navigation@0", - "when": "notebookType == 'deepnote' && isWorkspaceTrusted" - }, { "command": "deepnote.manageIntegrations", - "group": "navigation@1", + "group": "navigation@0", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addSqlBlock", - "group": "navigation@2", + "group": "navigation@1", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addChartBlock", - "group": "navigation@3", + "group": "navigation@2", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addBigNumberChartBlock", - "group": "navigation@4", + "group": "navigation@3", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputTextBlock", - "group": "navigation@5", + "group": "navigation@4", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputTextareaBlock", - "group": "navigation@6", + "group": "navigation@5", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputSelectBlock", - "group": "navigation@7", + "group": "navigation@6", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputSliderBlock", - "group": "navigation@8", + "group": "navigation@7", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputCheckboxBlock", - "group": "navigation@9", + "group": "navigation@8", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputDateBlock", - "group": "navigation@10", + "group": "navigation@9", "when": "notebookType == 'deepnote'" }, { "command": "deepnote.addInputDateRangeBlock", - "group": "navigation@11", + "group": "navigation@10", "when": "notebookType == 'deepnote'" }, { @@ -1509,36 +1441,6 @@ "command": "deepnote.revealInExplorer", "when": "view == deepnoteExplorer && viewItem != loading", "group": "inline@2" - }, - { - "command": "deepnote.environments.start", - "when": "view == deepnoteEnvironments && viewItem == deepnoteEnvironment.stopped", - "group": "inline@1" - }, - { - "command": "deepnote.environments.stop", - "when": "view == deepnoteEnvironments && viewItem == deepnoteEnvironment.running", - "group": "inline@1" - }, - { - "command": "deepnote.environments.restart", - "when": "view == deepnoteEnvironments && viewItem == deepnoteEnvironment.running", - "group": "1_lifecycle@1" - }, - { - "command": "deepnote.environments.managePackages", - "when": "view == deepnoteEnvironments && viewItem =~ /deepnoteEnvironment\\.(running|stopped)/", - "group": "2_manage@1" - }, - { - "command": "deepnote.environments.editName", - "when": "view == deepnoteEnvironments && viewItem =~ /deepnoteEnvironment\\.(running|stopped)/", - "group": "2_manage@2" - }, - { - "command": "deepnote.environments.delete", - "when": "view == deepnoteEnvironments && viewItem =~ /deepnoteEnvironment\\.(running|stopped)/", - "group": "4_danger@1" } ] }, @@ -2193,11 +2095,6 @@ "light": "./resources/light/deepnote-icon.svg", "dark": "./resources/dark/deepnote-icon.svg" } - }, - { - "id": "deepnoteEnvironments", - "name": "%deepnote.views.environments.name%", - "when": "workspaceFolderCount != 0" } ] }, @@ -2532,7 +2429,6 @@ "tcp-port-used": "^1.0.1", "tmp": "^0.2.4", "url-parse": "^1.5.10", - "uuid": "^13.0.0", "vega": "^5.33.0", "vega-embed": "^6.25.0", "vega-lite": "^5.21.0", @@ -2593,6 +2489,7 @@ "@types/temp": "^0.8.32", "@types/tmp": "^0.2.3", "@types/url-parse": "^1.4.8", + "@types/uuid": "^3.4.3", "@types/vscode-notebook-renderer": "^1.60.0", "@types/ws": "^6.0.1", "@typescript-eslint/eslint-plugin": "^6.9.0", diff --git a/package.nls.json b/package.nls.json index 5c1f6753ec..504e2565cf 100644 --- a/package.nls.json +++ b/package.nls.json @@ -268,15 +268,5 @@ "deepnote.commands.addButtonBlock.title": "Add Button Block", "deepnote.views.explorer.name": "Explorer", "deepnote.views.explorer.welcome": "No Deepnote notebooks found in this workspace.", - "deepnote.views.environments.name": "Environments", - "deepnote.command.selectNotebook.title": "Select Notebook", - "deepnote.commands.environments.create.title": "Create Environment", - "deepnote.commands.environments.start.title": "Start Server", - "deepnote.commands.environments.stop.title": "Stop Server", - "deepnote.commands.environments.restart.title": "Restart Server", - "deepnote.commands.environments.delete.title": "Delete Environment", - "deepnote.commands.environments.managePackages.title": "Manage Packages", - "deepnote.commands.environments.editName.title": "Rename Environment", - "deepnote.commands.environments.refresh.title": "Refresh", - "deepnote.commands.environments.selectForNotebook.title": "Select Environment for Notebook" + "deepnote.command.selectNotebook.title": "Select Notebook" } diff --git a/src/kernels/deepnote/deepnoteServerProvider.node.ts b/src/kernels/deepnote/deepnoteServerProvider.node.ts index ab2c3f2fc4..22450f3d69 100644 --- a/src/kernels/deepnote/deepnoteServerProvider.node.ts +++ b/src/kernels/deepnote/deepnoteServerProvider.node.ts @@ -85,7 +85,7 @@ export class DeepnoteServerProvider for (const [handle, info] of this.servers.entries()) { servers.push({ id: handle, - label: `Deepnote Toolkit (jupyter:${info.jupyterPort}, lsp:${info.lspPort})`, + label: `Deepnote Toolkit (${info.port})`, connectionInformation: { baseUrl: Uri.parse(info.url), token: info.token || '' diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index ad9a77f390..53c892cb92 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable, named, optional } from 'inversify'; -import { CancellationToken, l10n, Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { IDeepnoteServerStarter, IDeepnoteToolkitInstaller, DeepnoteServerInfo, DEEPNOTE_DEFAULT_PORT } from './types'; import { IProcessServiceFactory, ObservableExecutionResult } from '../../platform/common/process/types.node'; @@ -29,16 +29,6 @@ interface ServerLockFile { timestamp: number; } -type PendingOperation = - | { - type: 'start'; - promise: Promise; - } - | { - type: 'stop'; - promise: Promise; - }; - /** * Starts and manages the deepnote-toolkit Jupyter server. */ @@ -48,9 +38,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension private readonly serverInfos: Map = new Map(); private readonly disposablesByFile: Map = new Map(); // Track in-flight operations per file to prevent concurrent start/stop - private readonly pendingOperations: Map = new Map(); - // Global lock for port allocation to prevent race conditions when multiple environments start concurrently - private portAllocationLock: Promise = Promise.resolve(); + private readonly pendingOperations: Map> = new Map(); // Unique session ID for this VS Code window instance private readonly sessionId: string = generateUuid(); // Directory for lock files @@ -75,163 +63,94 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension public activate(): void { // Ensure lock file directory exists this.initializeLockFileDirectory().catch((ex) => { - logger.warn('Failed to initialize lock file directory', ex); + logger.warn(`Failed to initialize lock file directory: ${ex}`); }); // Clean up any orphaned deepnote-toolkit processes from previous sessions this.cleanupOrphanedProcesses().catch((ex) => { - logger.warn('Failed to cleanup orphaned processes', ex); + logger.warn(`Failed to cleanup orphaned processes: ${ex}`); }); } - /** - * Environment-based method: Start a server for a kernel environment. - * @param interpreter The Python interpreter to use - * @param venvPath The path to the venv - * @param environmentId The environment ID (used as key for server management) - * @param token Cancellation token - * @returns Server connection information - */ - public async startServer( + public async getOrStartServer( interpreter: PythonEnvironment, - venvPath: Uri, - environmentId: string, + deepnoteFileUri: Uri, token?: CancellationToken ): Promise { - // Wait for any pending operations on this environment to complete - let pendingOp = this.pendingOperations.get(environmentId); + const fileKey = deepnoteFileUri.fsPath; + + // Wait for any pending operations on this file to complete + const pendingOp = this.pendingOperations.get(fileKey); if (pendingOp) { - logger.info(`Waiting for pending operation on environment ${environmentId} to complete...`); + logger.info(`Waiting for pending operation on ${fileKey} to complete...`); try { - await pendingOp.promise; + await pendingOp; } catch { // Ignore errors from previous operations } } - // If server is already running for this environment, return existing info - const existingServerInfo = this.serverInfos.get(environmentId); + // If server is already running for this file, return existing info + const existingServerInfo = this.serverInfos.get(fileKey); if (existingServerInfo && (await this.isServerRunning(existingServerInfo))) { - logger.info( - `Deepnote server already running at ${existingServerInfo.url} for environment ${environmentId}` - ); + logger.info(`Deepnote server already running at ${existingServerInfo.url} for ${fileKey}`); return existingServerInfo; } - // Start the operation if not already pending - pendingOp = this.pendingOperations.get(environmentId); - - if (pendingOp && pendingOp.type === 'start') { - return await pendingOp.promise; - } - // Start the operation and track it - const operation = { - type: 'start' as const, - promise: this.startServerForEnvironment(interpreter, venvPath, environmentId, token) - }; - this.pendingOperations.set(environmentId, operation); + const operation = this.startServerImpl(interpreter, deepnoteFileUri, token); + this.pendingOperations.set(fileKey, operation); try { - const result = await operation.promise; + const result = await operation; return result; } finally { // Remove from pending operations when done - if (this.pendingOperations.get(environmentId) === operation) { - this.pendingOperations.delete(environmentId); - } - } - } - - /** - * Environment-based method: Stop the server for a kernel environment. - * @param environmentId The environment ID - */ - public async stopServer(environmentId: string, token?: CancellationToken): Promise { - if (token?.isCancellationRequested) { - throw new Error('Operation cancelled'); - } - - // Wait for any pending operations on this environment to complete - const pendingOp = this.pendingOperations.get(environmentId); - if (pendingOp) { - logger.info(`Waiting for pending operation on environment ${environmentId} before stopping...`); - try { - await pendingOp.promise; - } catch { - // Ignore errors from previous operations - } - } - - if (token?.isCancellationRequested) { - throw new Error('Operation cancelled'); - } - - // Start the stop operation and track it - const operation = { type: 'stop' as const, promise: this.stopServerForEnvironment(environmentId, token) }; - this.pendingOperations.set(environmentId, operation); - - try { - await operation.promise; - } finally { - // Remove from pending operations when done - if (this.pendingOperations.get(environmentId) === operation) { - this.pendingOperations.delete(environmentId); + if (this.pendingOperations.get(fileKey) === operation) { + this.pendingOperations.delete(fileKey); } } } - /** - * Environment-based server start implementation. - */ - private async startServerForEnvironment( + private async startServerImpl( interpreter: PythonEnvironment, - venvPath: Uri, - environmentId: string, + deepnoteFileUri: Uri, token?: CancellationToken ): Promise { + const fileKey = deepnoteFileUri.fsPath; + Cancellation.throwIfCanceled(token); - // Ensure toolkit is installed in venv and get venv's Python interpreter - logger.info(`Ensuring deepnote-toolkit is installed in venv for environment ${environmentId}...`); - const { pythonInterpreter: venvInterpreter } = await this.toolkitInstaller.ensureVenvAndToolkit( - interpreter, - venvPath, - token - ); + // Ensure toolkit is installed (will throw typed errors on failure) + logger.info(`Ensuring deepnote-toolkit is installed for ${fileKey}...`); + await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); Cancellation.throwIfCanceled(token); - // Allocate both ports with global lock to prevent race conditions - // Note: allocatePorts reserves both ports immediately in serverInfos - const { jupyterPort, lspPort } = await this.allocatePorts(environmentId); - - logger.info( - `Starting deepnote-toolkit server on jupyter port ${jupyterPort} and lsp port ${lspPort} for environment ${environmentId}` - ); - this.outputChannel.appendLine( - l10n.t('Starting Deepnote server on jupyter port {0} and lsp port {1}...', jupyterPort, lspPort) - ); + // Find available port + const port = await getPort({ host: 'localhost', port: DEEPNOTE_DEFAULT_PORT }); + logger.info(`Starting deepnote-toolkit server on port ${port} for ${fileKey}`); + this.outputChannel.appendLine(`Starting Deepnote server on port ${port} for ${deepnoteFileUri.fsPath}...`); // Start the server with venv's Python in PATH + // This ensures shell commands (!) in notebooks use the venv's Python + // Use undefined as resource to get full system environment (including git in PATH) const processService = await this.processServiceFactory.create(undefined); // Set up environment to ensure the venv's Python is used for shell commands - const venvBinDir = path.dirname(venvInterpreter.uri.fsPath); + const venvBinDir = interpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, ''); const env = { ...process.env }; // Prepend venv bin directory to PATH so shell commands use venv's Python env.PATH = `${venvBinDir}${process.platform === 'win32' ? ';' : ':'}${env.PATH || ''}`; // Also set VIRTUAL_ENV to indicate we're in a venv - env.VIRTUAL_ENV = venvPath.fsPath; + const venvPath = venvBinDir.replace(/\/bin$/, '').replace(/\\Scripts$/, ''); + env.VIRTUAL_ENV = venvPath; // Enforce published pip constraints to prevent breaking Deepnote Toolkit's dependencies env.DEEPNOTE_ENFORCE_PIP_CONSTRAINTS = 'true'; - // Detached mode - env.DEEPNOTE_RUNTIME__RUNNING_IN_DETACHED_MODE = 'true'; - // Detached mode ensures no requests are made to the backend (directly, or via proxy) // as there is no backend running in the extension, therefore: // 1. integration environment variables are injected here instead @@ -240,10 +159,9 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension // Inject SQL integration environment variables if (this.sqlIntegrationEnvVars) { - logger.debug(`DeepnoteServerStarter: Injecting SQL integration env vars for environment ${environmentId}`); + logger.debug(`DeepnoteServerStarter: Injecting SQL integration env vars for ${deepnoteFileUri.toString()}`); try { - // const sqlEnvVars = await this.sqlIntegrationEnvVars.getEnvironmentVariables(deepnoteFileUri, token); - const sqlEnvVars = {}; // TODO: update how environment variables are retrieved + const sqlEnvVars = await this.sqlIntegrationEnvVars.getEnvironmentVariables(deepnoteFileUri, token); if (sqlEnvVars && Object.keys(sqlEnvVars).length > 0) { logger.debug(`DeepnoteServerStarter: Injecting ${Object.keys(sqlEnvVars).length} SQL env vars`); Object.assign(env, sqlEnvVars); @@ -260,42 +178,41 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension // Remove PYTHONHOME if it exists (can interfere with venv) delete env.PYTHONHOME; + // Get the directory containing the notebook file to set as working directory + // This ensures relative file paths in the notebook work correctly + const notebookDir = Uri.joinPath(deepnoteFileUri, '..').fsPath; + const serverProcess = processService.execObservable( - venvInterpreter.uri.fsPath, - [ - '-m', - 'deepnote_toolkit', - 'server', - '--jupyter-port', - jupyterPort.toString(), - '--ls-port', - lspPort.toString() - ], - { env } + interpreter.uri.fsPath, + ['-m', 'deepnote_toolkit', 'server', '--jupyter-port', port.toString()], + { + env, + cwd: notebookDir + } ); - this.serverProcesses.set(environmentId, serverProcess); + this.serverProcesses.set(fileKey, serverProcess); - // Track disposables for this environment + // Track disposables for this file const disposables: IDisposable[] = []; - this.disposablesByFile.set(environmentId, disposables); + this.disposablesByFile.set(fileKey, disposables); // Initialize output tracking for error reporting - this.serverOutputByFile.set(environmentId, { stdout: '', stderr: '' }); + this.serverOutputByFile.set(fileKey, { stdout: '', stderr: '' }); // Monitor server output serverProcess.out.onDidChange( (output) => { - const outputTracking = this.serverOutputByFile.get(environmentId); + const outputTracking = this.serverOutputByFile.get(fileKey); if (output.source === 'stdout') { - logger.trace(`Deepnote server (${environmentId}): ${output.out}`); + logger.trace(`Deepnote server (${fileKey}): ${output.out}`); this.outputChannel.appendLine(output.out); if (outputTracking) { // Keep last 5000 characters of output for error reporting outputTracking.stdout = (outputTracking.stdout + output.out).slice(-5000); } } else if (output.source === 'stderr') { - logger.warn(`Deepnote server stderr (${environmentId}): ${output.out}`); + logger.warn(`Deepnote server stderr (${fileKey}): ${output.out}`); this.outputChannel.appendLine(output.out); if (outputTracking) { // Keep last 5000 characters of error output for error reporting @@ -308,47 +225,45 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension ); // Wait for server to be ready - const url = `http://localhost:${jupyterPort}`; - const serverInfo = { url, jupyterPort, lspPort }; - this.serverInfos.set(environmentId, serverInfo); + const url = `http://localhost:${port}`; + const serverInfo = { url, port }; + this.serverInfos.set(fileKey, serverInfo); // Write lock file for the server process const serverPid = serverProcess.proc?.pid; if (serverPid) { await this.writeLockFile(serverPid); } else { - logger.warn(`Could not get PID for server process for environment ${environmentId}`); + logger.warn(`Could not get PID for server process for ${fileKey}`); } try { const serverReady = await this.waitForServer(serverInfo, 120000, token); if (!serverReady) { - const output = this.serverOutputByFile.get(environmentId); + const output = this.serverOutputByFile.get(fileKey); + await this.stopServerImpl(deepnoteFileUri); throw new DeepnoteServerTimeoutError(serverInfo.url, 120000, output?.stderr || undefined); } } catch (error) { + // If this is already a DeepnoteKernelError, clean up and rethrow it if (error instanceof DeepnoteServerTimeoutError || error instanceof DeepnoteServerStartupError) { - // await this.stopServerImpl(deepnoteFileUri); - await this.stopServerForEnvironment(environmentId); + await this.stopServerImpl(deepnoteFileUri); throw error; } // Capture output BEFORE cleaning up (stopServerImpl deletes it) - // const output = this.serverOutputByFile.get(fileKey); - const output = this.serverOutputByFile.get(environmentId); + const output = this.serverOutputByFile.get(fileKey); const capturedStdout = output?.stdout || ''; const capturedStderr = output?.stderr || ''; - // Clean up leaked server before rethrowing - await this.stopServerForEnvironment(environmentId); - // throw error; + // Clean up leaked server after capturing output + await this.stopServerImpl(deepnoteFileUri); - // TODO // Wrap in a generic server startup error with captured output throw new DeepnoteServerStartupError( interpreter.uri.fsPath, - serverInfo.jupyterPort, + port, 'unknown', capturedStdout, capturedStderr, @@ -356,46 +271,68 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension ); } - logger.info(`Deepnote server started successfully at ${url} for environment ${environmentId}`); - this.outputChannel.appendLine(l10n.t('✓ Deepnote server running at {0}', url)); + logger.info(`Deepnote server started successfully at ${url} for ${fileKey}`); + this.outputChannel.appendLine(`✓ Deepnote server running at ${url}`); return serverInfo; } - /** - * Environment-based server stop implementation. - */ - private async stopServerForEnvironment(environmentId: string, token?: CancellationToken): Promise { - Cancellation.throwIfCanceled(token); + public async stopServer(deepnoteFileUri: Uri): Promise { + const fileKey = deepnoteFileUri.fsPath; - const serverProcess = this.serverProcesses.get(environmentId); + // Wait for any pending operations on this file to complete + const pendingOp = this.pendingOperations.get(fileKey); + if (pendingOp) { + logger.info(`Waiting for pending operation on ${fileKey} before stopping...`); + try { + await pendingOp; + } catch { + // Ignore errors from previous operations + } + } + + // Start the stop operation and track it + const operation = this.stopServerImpl(deepnoteFileUri); + this.pendingOperations.set(fileKey, operation); + + try { + await operation; + } finally { + // Remove from pending operations when done + if (this.pendingOperations.get(fileKey) === operation) { + this.pendingOperations.delete(fileKey); + } + } + } + + private async stopServerImpl(deepnoteFileUri: Uri): Promise { + const fileKey = deepnoteFileUri.fsPath; + const serverProcess = this.serverProcesses.get(fileKey); if (serverProcess) { const serverPid = serverProcess.proc?.pid; try { - logger.info(`Stopping Deepnote server for environment ${environmentId}...`); + logger.info(`Stopping Deepnote server for ${fileKey}...`); serverProcess.proc?.kill(); - this.serverProcesses.delete(environmentId); - this.serverInfos.delete(environmentId); - this.serverOutputByFile.delete(environmentId); - this.outputChannel.appendLine(l10n.t('Deepnote server stopped for environment {0}', environmentId)); - } catch (ex) { - logger.error('Error stopping Deepnote server', ex); - } finally { + this.serverProcesses.delete(fileKey); + this.serverInfos.delete(fileKey); + this.serverOutputByFile.delete(fileKey); + this.outputChannel.appendLine(`Deepnote server stopped for ${fileKey}`); + // Clean up lock file after stopping the server if (serverPid) { await this.deleteLockFile(serverPid); } + } catch (ex) { + logger.error(`Error stopping Deepnote server: ${ex}`); } } - Cancellation.throwIfCanceled(token); - - const disposables = this.disposablesByFile.get(environmentId); + const disposables = this.disposablesByFile.get(fileKey); if (disposables) { disposables.forEach((d) => d.dispose()); - this.disposablesByFile.delete(environmentId); + this.disposablesByFile.delete(fileKey); } } @@ -425,119 +362,6 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension } } - /** - * Allocate both Jupyter and LSP ports atomically with global serialization. - * When multiple environments start simultaneously, this ensures each gets unique ports. - * - * @param key The environment ID to reserve ports for - * @returns Object with jupyterPort and lspPort - */ - private async allocatePorts(key: string): Promise<{ jupyterPort: number; lspPort: number }> { - // Wait for any ongoing port allocation to complete - await this.portAllocationLock; - - // Create new allocation promise and update the lock - let releaseLock: () => void; - this.portAllocationLock = new Promise((resolve) => { - releaseLock = resolve; - }); - - try { - // Get all ports currently in use by our managed servers - const portsInUse = new Set(); - for (const serverInfo of this.serverInfos.values()) { - if (serverInfo.jupyterPort) { - portsInUse.add(serverInfo.jupyterPort); - } - if (serverInfo.lspPort) { - portsInUse.add(serverInfo.lspPort); - } - } - - // Allocate Jupyter port first - const jupyterPort = await this.findAvailablePort(DEEPNOTE_DEFAULT_PORT, portsInUse); - portsInUse.add(jupyterPort); // Reserve it immediately - - // Allocate LSP port (starting from jupyterPort + 1 to avoid conflicts) - const lspPort = await this.findAvailablePort(jupyterPort + 1, portsInUse); - portsInUse.add(lspPort); // Reserve it immediately - - // Reserve both ports by adding to serverInfos - // This prevents other concurrent allocations from getting the same ports - const serverInfo = { - url: `http://localhost:${jupyterPort}`, - jupyterPort, - lspPort - }; - this.serverInfos.set(key, serverInfo); - - logger.info( - `Allocated ports for ${key}: jupyter=${jupyterPort}, lsp=${lspPort} (excluded: ${ - portsInUse.size > 2 - ? Array.from(portsInUse) - .filter((p) => p !== jupyterPort && p !== lspPort) - .join(', ') - : 'none' - })` - ); - - return { jupyterPort, lspPort }; - } finally { - // Release the lock to allow next allocation - releaseLock!(); - } - } - - /** - * Find an available port starting from the given port number. - * Checks both our internal portsInUse set and system availability. - */ - private async findAvailablePort(startPort: number, portsInUse: Set): Promise { - let port = startPort; - let attempts = 0; - const maxAttempts = 100; - - while (attempts < maxAttempts) { - // Skip ports already in use by our servers - if (!portsInUse.has(port)) { - // Check if this port is available on the system - const availablePort = await getPort({ - host: 'localhost', - port - }); - - // If get-port returned the same port, it's available - if (availablePort === port) { - return port; - } - - // get-port returned a different port - check if that one is in use - if (!portsInUse.has(availablePort)) { - return availablePort; - } - - // Both our requested port and get-port's suggestion are in use, try next - } - - // Try next port - port++; - attempts++; - } - - throw new DeepnoteServerStartupError( - 'python', // unknown here - startPort, - 'process_failed', - '', - l10n.t( - 'Failed to find available port after {0} attempts (started at {1}). Ports in use: {2}', - maxAttempts, - startPort, - Array.from(portsInUse).join(', ') - ) - ); - } - public async dispose(): Promise { logger.info('Disposing DeepnoteServerStarter - stopping all servers...'); @@ -585,7 +409,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension killPromises.push(exitPromise); } } catch (ex) { - logger.error(`Error stopping Deepnote server for ${fileKey}`, ex); + logger.error(`Error stopping Deepnote server for ${fileKey}: ${ex}`); } } @@ -605,7 +429,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension try { disposables.forEach((d) => d.dispose()); } catch (ex) { - logger.error(`Error disposing resources for ${fileKey}`, ex); + logger.error(`Error disposing resources for ${fileKey}: ${ex}`); } } @@ -627,7 +451,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension await fs.ensureDir(this.lockFileDir); logger.info(`Lock file directory initialized at ${this.lockFileDir} with session ID ${this.sessionId}`); } catch (ex) { - logger.error('Failed to create lock file directory', ex); + logger.error(`Failed to create lock file directory: ${ex}`); } } @@ -652,7 +476,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension await fs.writeJson(lockFilePath, lockData, { spaces: 2 }); logger.info(`Created lock file for PID ${pid} with session ID ${this.sessionId}`); } catch (ex) { - logger.warn(`Failed to write lock file for PID ${pid}`, ex); + logger.warn(`Failed to write lock file for PID ${pid}: ${ex}`); } } @@ -666,7 +490,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension return await fs.readJson(lockFilePath); } } catch (ex) { - logger.warn(`Failed to read lock file for PID ${pid}`, ex); + logger.warn(`Failed to read lock file for PID ${pid}: ${ex}`); } return null; } @@ -682,7 +506,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension logger.info(`Deleted lock file for PID ${pid}`); } } catch (ex) { - logger.warn(`Failed to delete lock file for PID ${pid}`, ex); + logger.warn(`Failed to delete lock file for PID ${pid}: ${ex}`); } } @@ -758,7 +582,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension } } } catch (ex) { - logger.warn(`Failed to check if process ${pid} is orphaned`, ex); + logger.warn(`Failed to check if process ${pid} is orphaned: ${ex}`); } // If we can't determine, assume it's not orphaned (safer) @@ -876,7 +700,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension if (pidsToKill.length > 0) { logger.info(`Killing ${pidsToKill.length} orphaned process(es): ${pidsToKill.join(', ')}`); this.outputChannel.appendLine( - l10n.t('Cleaning up {0} orphaned deepnote-toolkit process(es)...', pidsToKill.length) + `Cleaning up ${pidsToKill.length} orphaned deepnote-toolkit process(es)...` ); for (const pid of pidsToKill) { @@ -893,11 +717,11 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension // Clean up the lock file after killing await this.deleteLockFile(pid); } catch (ex) { - logger.warn(`Failed to kill process ${pid}`, ex); + logger.warn(`Failed to kill process ${pid}: ${ex}`); } } - this.outputChannel.appendLine(l10n.t('✓ Cleanup complete')); + this.outputChannel.appendLine('✓ Cleanup complete'); } else { logger.info('No orphaned deepnote-toolkit processes found (all processes are active)'); } @@ -907,7 +731,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension } } catch (ex) { // Don't fail startup if cleanup fails - logger.warn('Error during orphaned process cleanup', ex); + logger.warn(`Error during orphaned process cleanup: ${ex}`); } } } diff --git a/src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts b/src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts index 54f37c3dcc..0cfd9d23b3 100644 --- a/src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts +++ b/src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable, named } from 'inversify'; -import { CancellationToken, l10n, Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { IProcessServiceFactory } from '../../platform/common/process/types.node'; import { logger } from '../../platform/logging'; @@ -98,11 +98,11 @@ export class DeepnoteSharedToolkitInstaller { const success = result.stdout.toLowerCase().includes('shared import successful'); logger.info(`Shared installation test result: ${success ? 'SUCCESS' : 'FAILED'}`); if (!success) { - logger.warn('Shared installation test failed', { stdout: result.stdout, stderr: result.stderr }); + logger.warn(`Shared installation test failed: stdout=${result.stdout}, stderr=${result.stderr}`); } return success; } catch (ex) { - logger.error('Shared installation test error', ex); + logger.error(`Shared installation test error: ${ex}`); return false; } } @@ -180,7 +180,7 @@ export class DeepnoteSharedToolkitInstaller { const packagePath = Uri.joinPath(this.sharedInstallationPath, 'deepnote_toolkit'); return await this.fs.exists(packagePath); } catch (ex) { - logger.debug('Error checking shared installation', ex); + logger.debug(`Error checking shared installation: ${ex}`); return false; } } @@ -198,7 +198,7 @@ export class DeepnoteSharedToolkitInstaller { logger.info( `Installing shared deepnote-toolkit v${this.toolkitVersion} to ${this.sharedInstallationPath.fsPath}` ); - this.outputChannel.appendLine(l10n.t('Installing shared deepnote-toolkit v{0}...', this.toolkitVersion)); + this.outputChannel.appendLine(`Installing shared deepnote-toolkit v${this.toolkitVersion}...`); // Create shared installation directory await this.fs.createDirectory(this.sharedInstallationPath); @@ -240,17 +240,16 @@ export class DeepnoteSharedToolkitInstaller { await this.fs.writeFile(this.versionFilePath, Buffer.from(this.toolkitVersion, 'utf8')); logger.info(`Shared deepnote-toolkit v${this.toolkitVersion} installed successfully`); - this.outputChannel.appendLine(l10n.t('✓ Shared deepnote-toolkit v{0} ready', this.toolkitVersion)); + this.outputChannel.appendLine(`✓ Shared deepnote-toolkit v${this.toolkitVersion} ready`); return true; } else { logger.error('Shared deepnote-toolkit installation failed - package not found'); - this.outputChannel.appendLine(l10n.t('✗ Shared deepnote-toolkit installation failed')); + this.outputChannel.appendLine('✗ Shared deepnote-toolkit installation failed'); return false; } } catch (ex) { - logger.error('Failed to install shared deepnote-toolkit', ex); - const msg = ex instanceof Error ? ex.message : String(ex); - this.outputChannel.appendLine(l10n.t('Error installing shared deepnote-toolkit: {0}', msg)); + logger.error(`Failed to install shared deepnote-toolkit: ${ex}`); + this.outputChannel.appendLine(`Error installing shared deepnote-toolkit: ${ex}`); return false; } } diff --git a/src/kernels/deepnote/deepnoteToolkitInstaller.node.ts b/src/kernels/deepnote/deepnoteToolkitInstaller.node.ts index c36d314ec2..97cb07a9ea 100644 --- a/src/kernels/deepnote/deepnoteToolkitInstaller.node.ts +++ b/src/kernels/deepnote/deepnoteToolkitInstaller.node.ts @@ -2,17 +2,16 @@ // Licensed under the MIT License. import { inject, injectable, named } from 'inversify'; -import { CancellationToken, l10n, Uri, workspace } from 'vscode'; - -import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants'; -import { IFileSystem } from '../../platform/common/platform/types'; +import { CancellationToken, Uri, workspace } from 'vscode'; +import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; +import { IDeepnoteToolkitInstaller, DEEPNOTE_TOOLKIT_VERSION } from './types'; import { IProcessServiceFactory } from '../../platform/common/process/types.node'; -import { IExtensionContext, IOutputChannel } from '../../platform/common/types'; -import { DeepnoteToolkitInstallError, DeepnoteVenvCreationError } from '../../platform/errors/deepnoteKernelErrors'; import { logger } from '../../platform/logging'; -import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; -import { DEEPNOTE_TOOLKIT_VERSION, IDeepnoteToolkitInstaller, VenvAndToolkitInstallation } from './types'; +import { IOutputChannel, IExtensionContext } from '../../platform/common/types'; +import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants'; +import { IFileSystem } from '../../platform/common/platform/types'; import { Cancellation } from '../../platform/common/cancellation'; +import { DeepnoteVenvCreationError, DeepnoteToolkitInstallError } from '../../platform/errors/deepnoteKernelErrors'; /** * Handles installation of the deepnote-toolkit Python package. @@ -21,7 +20,7 @@ import { Cancellation } from '../../platform/common/cancellation'; export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { private readonly venvPythonPaths: Map = new Map(); // Track in-flight installations per venv path to prevent concurrent installs - private readonly pendingInstallations: Map> = new Map(); + private readonly pendingInstallations: Map> = new Map(); constructor( @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory, @@ -38,10 +37,8 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { return Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', `${hash}-${DEEPNOTE_TOOLKIT_VERSION}`); } - /** - * Get the venv Python interpreter by direct venv path. - */ - private async getVenvInterpreterByPath(venvPath: Uri): Promise { + public async getVenvInterpreter(deepnoteFileUri: Uri): Promise { + const venvPath = this.getVenvPath(deepnoteFileUri); const cacheKey = venvPath.fsPath; if (this.venvPythonPaths.has(cacheKey)) { @@ -62,23 +59,12 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { return undefined; } - public async getVenvInterpreter(deepnoteFileUri: Uri): Promise { - const venvPath = this.getVenvPath(deepnoteFileUri); - return this.getVenvInterpreterByPath(venvPath); - } - - /** - * Environment-based method: Ensure venv and toolkit are installed at a specific path. - * @param baseInterpreter The base Python interpreter to use for creating the venv - * @param venvPath The exact path where the venv should be created - * @param token Cancellation token - * @returns The venv Python interpreter if successful - */ - public async ensureVenvAndToolkit( + public async ensureInstalled( baseInterpreter: PythonEnvironment, - venvPath: Uri, + deepnoteFileUri: Uri, token?: CancellationToken - ): Promise { + ): Promise { + const venvPath = this.getVenvPath(deepnoteFileUri); const venvKey = venvPath.fsPath; logger.info(`Ensuring virtual environment at ${venvKey}`); @@ -96,27 +82,14 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { } // Check if venv already exists with toolkit installed - const existingVenv = await this.getVenvInterpreterByPath(venvPath); - if (existingVenv) { - const toolkitVersion = await this.isToolkitInstalled(existingVenv); - if (toolkitVersion != null) { - logger.info(`deepnote-toolkit venv already exists at ${venvPath.fsPath}`); - - // Ensure kernel spec is installed (may have been deleted or never installed) - try { - Cancellation.throwIfCanceled(token); - await this.installKernelSpec(existingVenv, venvPath, token); - } catch (ex) { - logger.warn('Failed to ensure kernel spec installed', ex); - // Don't fail - continue with existing venv - } - - logger.info(`Venv ready at ${venvPath.fsPath}`); - return { pythonInterpreter: existingVenv, toolkitVersion }; - } + const existingVenv = await this.getVenvInterpreter(deepnoteFileUri); + if (existingVenv && (await this.isToolkitInstalled(existingVenv))) { + logger.info(`deepnote-toolkit venv already exists and is ready for ${deepnoteFileUri.fsPath}`); + return existingVenv; } - // Double-check for race condition + // Double-check for race condition: another caller might have started installation + // while we were checking the venv const pendingAfterCheck = this.pendingInstallations.get(venvKey); if (pendingAfterCheck) { logger.info(`Another installation started for ${venvKey} while checking, waiting for it...`); @@ -128,7 +101,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { } // Start the installation and track it - const installation = this.installVenvAndToolkit(baseInterpreter, venvPath, token); + const installation = this.installImpl(baseInterpreter, deepnoteFileUri, venvPath, token); this.pendingInstallations.set(venvKey, installation); try { @@ -142,68 +115,17 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { } } - /** - * Install additional packages in an existing venv. - * @param venvPath Path to the venv - * @param packages List of package names to install - * @param token Cancellation token - */ - public async installAdditionalPackages( - venvPath: Uri, - packages: string[], - token?: CancellationToken - ): Promise { - if (packages.length === 0) { - return; - } - - const venvInterpreter = await this.getVenvInterpreterByPath(venvPath); - if (!venvInterpreter) { - throw new Error(`Venv not found at ${venvPath.fsPath}`); - } - - logger.info(`Installing additional packages in ${venvPath.fsPath}: ${packages.join(', ')}`); - this.outputChannel.appendLine(l10n.t('Installing packages: {0}...', packages.join(', '))); - - try { - Cancellation.throwIfCanceled(token); - - const venvProcessService = await this.processServiceFactory.create(undefined); - const installResult = await venvProcessService.exec( - venvInterpreter.uri.fsPath, - ['-m', 'pip', 'install', '--upgrade', ...packages], - { throwOnStdErr: false } - ); - - if (installResult.stdout) { - this.outputChannel.appendLine(installResult.stdout); - } - if (installResult.stderr) { - this.outputChannel.appendLine(installResult.stderr); - } - - logger.info('Additional packages installed successfully'); - this.outputChannel.appendLine(l10n.t('✓ Packages installed successfully')); - } catch (ex) { - logger.error('Failed to install additional packages', ex); - this.outputChannel.appendLine(l10n.t('✗ Failed to install packages: {0}', ex)); - throw ex; - } - } - - /** - * Install venv and toolkit at a specific path (environment-based). - */ - private async installVenvAndToolkit( + private async installImpl( baseInterpreter: PythonEnvironment, + deepnoteFileUri: Uri, venvPath: Uri, token?: CancellationToken - ): Promise { + ): Promise { try { Cancellation.throwIfCanceled(token); - logger.info(`Creating virtual environment at ${venvPath.fsPath}`); - this.outputChannel.appendLine(l10n.t('Setting up Deepnote toolkit environment...')); + logger.info(`Creating virtual environment at ${venvPath.fsPath} for ${deepnoteFileUri.fsPath}`); + this.outputChannel.appendLine(`Setting up Deepnote toolkit environment for ${deepnoteFileUri.fsPath}...`); // Create venv parent directory if it doesn't exist const venvParentDir = Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs'); @@ -224,19 +146,19 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { // Log any stderr output (warnings, etc.) but don't fail on it if (venvResult.stderr) { - logger.info('venv creation stderr', venvResult.stderr); + logger.info(`venv creation stderr: ${venvResult.stderr}`); } Cancellation.throwIfCanceled(token); // Verify venv was created successfully by checking for the Python interpreter - const venvInterpreter = await this.getVenvInterpreterByPath(venvPath); + const venvInterpreter = await this.getVenvInterpreter(deepnoteFileUri); if (!venvInterpreter) { logger.error('Failed to create venv: Python interpreter not found after venv creation'); if (venvResult.stderr) { - logger.error('venv stderr', venvResult.stderr); + logger.error(`venv stderr: ${venvResult.stderr}`); } - this.outputChannel.appendLine(l10n.t('Error: Failed to create virtual environment')); + this.outputChannel.appendLine('Error: Failed to create virtual environment'); throw new DeepnoteVenvCreationError( baseInterpreter.uri.fsPath, @@ -250,7 +172,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { // Upgrade pip in the venv to the latest version logger.info('Upgrading pip in venv to latest version...'); - this.outputChannel.appendLine(l10n.t('Upgrading pip...')); + this.outputChannel.appendLine('Upgrading pip...'); const pipUpgradeResult = await venvProcessService.exec( venvInterpreter.uri.fsPath, ['-m', 'pip', 'install', '--upgrade', 'pip'], @@ -261,14 +183,14 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { logger.info(`pip upgrade output: ${pipUpgradeResult.stdout}`); } if (pipUpgradeResult.stderr) { - logger.info('pip upgrade stderr', pipUpgradeResult.stderr); + logger.info(`pip upgrade stderr: ${pipUpgradeResult.stderr}`); } Cancellation.throwIfCanceled(token); // Install deepnote-toolkit and ipykernel in venv logger.info(`Installing deepnote-toolkit (${DEEPNOTE_TOOLKIT_VERSION}) and ipykernel in venv from PyPI`); - this.outputChannel.appendLine(l10n.t('Installing deepnote-toolkit and ipykernel...')); + this.outputChannel.appendLine('Installing deepnote-toolkit and ipykernel...'); const installResult = await venvProcessService.exec( venvInterpreter.uri.fsPath, @@ -293,24 +215,47 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { } // Verify installation - const installedToolkitVersion = await this.isToolkitInstalled(venvInterpreter); - if (installedToolkitVersion != null) { + if (await this.isToolkitInstalled(venvInterpreter)) { 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 { - Cancellation.throwIfCanceled(token); - await this.installKernelSpec(venvInterpreter, venvPath, token); + // Reuse the process service with system environment + await venvProcessService.exec( + venvInterpreter.uri.fsPath, + [ + '-m', + 'ipykernel', + 'install', + '--prefix', + venvPath.fsPath, + '--name', + `deepnote-venv-${this.getVenvHash(deepnoteFileUri)}`, + '--display-name', + `Deepnote (${this.getDisplayName(deepnoteFileUri)})` + ], + { throwOnStdErr: false } + ); + const kernelSpecPath = Uri.joinPath( + venvPath, + 'share', + 'jupyter', + 'kernels', + `deepnote-venv-${this.getVenvHash(deepnoteFileUri)}` + ); + logger.info(`Kernel spec installed successfully to ${kernelSpecPath.fsPath}`); } catch (ex) { - logger.warn('Failed to install kernel spec', ex); + logger.warn(`Failed to install kernel spec: ${ex}`); // Don't fail the entire installation if kernel spec creation fails } - this.outputChannel.appendLine(l10n.t('✓ Deepnote toolkit ready')); - return { pythonInterpreter: venvInterpreter, toolkitVersion: installedToolkitVersion }; + this.outputChannel.appendLine('✓ Deepnote toolkit ready'); + return venvInterpreter; } else { logger.error('deepnote-toolkit installation failed'); - this.outputChannel.appendLine(l10n.t('✗ deepnote-toolkit installation failed')); + this.outputChannel.appendLine('✗ deepnote-toolkit installation failed'); throw new DeepnoteToolkitInstallError( venvInterpreter.uri.fsPath, @@ -327,8 +272,8 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { } // Otherwise, log full details and wrap in a generic toolkit install error - logger.error('Failed to set up deepnote-toolkit', ex); - this.outputChannel.appendLine(l10n.t('Failed to set up deepnote-toolkit; see logs for details')); + logger.error(`Failed to set up deepnote-toolkit: ${ex}`); + this.outputChannel.appendLine('Failed to set up deepnote-toolkit; see logs for details'); throw new DeepnoteToolkitInstallError( baseInterpreter.uri.fsPath, @@ -341,97 +286,21 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { } } - private async isToolkitInstalled(interpreter: PythonEnvironment): Promise { + private async isToolkitInstalled(interpreter: PythonEnvironment): Promise { try { // Use undefined as resource to get full system environment const processService = await this.processServiceFactory.create(undefined); const result = await processService.exec(interpreter.uri.fsPath, [ '-c', - 'import deepnote_toolkit; print(deepnote_toolkit.__version__)' + "import deepnote_toolkit; print('installed')" ]); - logger.info(`isToolkitInstalled result: ${result.stdout}`); - return result.stdout.trim(); + return result.stdout.toLowerCase().includes('installed'); } catch (ex) { - logger.debug('deepnote-toolkit not found', ex); - return undefined; + logger.debug(`deepnote-toolkit not found: ${ex}`); + return false; } } - /** - * Generate a kernel spec name from a venv path. - * This is used for both file-based and environment-based venvs. - */ - private getKernelSpecName(venvPath: Uri): string { - // Extract the venv directory name (last segment of path) - const raw = venvPath.fsPath.split(/[/\\]/).filter(Boolean).pop() || 'venv'; - const safe = raw - .toLowerCase() - .replace(/[^a-z0-9._-]/g, '-') - .replace(/-+/g, '-') - .replace(/^-|-$|^\.+/g, ''); - return `deepnote-${safe}`; - } - - /** - * Generate a display name from a venv path. - */ - private getKernelDisplayName(venvPath: Uri): string { - const raw = venvPath.fsPath.split(/[/\\]/).filter(Boolean).pop() || 'venv'; - const printable = raw.replace(/[\r\n\t]/g, ' ').trim(); - return `Deepnote (${printable})`; - } - - /** - * Install ipykernel kernel spec for a venv. - * This is idempotent - safe to call multiple times. - * @param venvInterpreter The venv Python interpreter - * @param venvPath The venv path - * @param token Cancellation token - */ - private async installKernelSpec( - venvInterpreter: PythonEnvironment, - venvPath: Uri, - token?: CancellationToken - ): Promise { - Cancellation.throwIfCanceled(token); - - const kernelSpecName = this.getKernelSpecName(venvPath); - const kernelSpecPath = Uri.joinPath(venvPath, 'share', 'jupyter', 'kernels', kernelSpecName); - - // Check if kernel spec already exists - if (await this.fs.exists(kernelSpecPath)) { - logger.info(`Kernel spec already exists at ${kernelSpecPath.fsPath}`); - return; - } - - Cancellation.throwIfCanceled(token); - - logger.info(`Installing kernel spec '${kernelSpecName}' for venv at ${venvPath.fsPath}...`); - const kernelDisplayName = this.getKernelDisplayName(venvPath); - - const venvProcessService = await this.processServiceFactory.create(undefined); - - Cancellation.throwIfCanceled(token); - - await venvProcessService.exec( - venvInterpreter.uri.fsPath, - [ - '-m', - 'ipykernel', - 'install', - '--prefix', - venvPath.fsPath, - '--name', - kernelSpecName, - '--display-name', - kernelDisplayName - ], - { throwOnStdErr: false } - ); - - logger.info(`Kernel spec installed successfully to ${kernelSpecPath.fsPath}`); - } - public getVenvHash(deepnoteFileUri: Uri): string { // Create a short hash from the file path for kernel naming and venv directory // This provides better uniqueness and prevents directory structure leakage @@ -449,4 +318,10 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller { const hashStr = Math.abs(hash).toString(16); return `venv_${hashStr}`.substring(0, 16); } + + private getDisplayName(deepnoteFileUri: Uri): string { + // Get a friendly display name from the file path + const parts = deepnoteFileUri.fsPath.split('/'); + return parts[parts.length - 1] || 'notebook'; + } } diff --git a/src/kernels/deepnote/environments/deepnoteEnvironment.ts b/src/kernels/deepnote/environments/deepnoteEnvironment.ts deleted file mode 100644 index 3f7f230420..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironment.ts +++ /dev/null @@ -1,122 +0,0 @@ -import { Uri } from 'vscode'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; -import { DeepnoteServerInfo } from '../types'; - -/** - * Represents a Deepnote kernel environment. - * This is the runtime model with full objects. - */ -export interface DeepnoteEnvironment { - /** - * Unique identifier for this environment (UUID) - */ - id: string; - - /** - * User-friendly name for the environment - * Example: "Python 3.11 (Data Science)" - */ - name: string; - - /** - * Python interpreter to use for this kernel - */ - pythonInterpreter: PythonEnvironment; - - /** - * Path to the virtual environment for this environment - */ - venvPath: Uri; - - /** - * Server information (set when server is running) - */ - serverInfo?: DeepnoteServerInfo; - - /** - * Timestamp when this environment was created - */ - createdAt: Date; - - /** - * Timestamp when this environment was last used - */ - lastUsedAt: Date; - - /** - * Optional list of additional packages to install in the venv - */ - packages?: string[]; - - /** - * Version of deepnote-toolkit installed (if known) - */ - toolkitVersion?: string; - - /** - * Optional description for this environment - */ - description?: string; -} - -/** - * Serializable state for storing environments. - * Uses string paths instead of Uri objects for JSON serialization. - */ -export interface DeepnoteEnvironmentState { - id: string; - name: string; - pythonInterpreterPath: { - id: string; - uri: string; - }; - venvPath: string; - createdAt: string; - lastUsedAt: string; - packages?: string[]; - toolkitVersion?: string; - description?: string; -} - -/** - * Options for creating a new kernel environment - */ -export interface CreateDeepnoteEnvironmentOptions { - name: string; - pythonInterpreter: PythonEnvironment; - packages?: string[]; - description?: string; -} - -/** - * Status of a kernel environment - */ -export enum EnvironmentStatus { - /** - * Environment exists but server is not running - */ - Stopped = 'stopped', - - /** - * Server is currently starting - */ - Starting = 'starting', - - /** - * Server is running and ready - */ - Running = 'running', - - /** - * Server encountered an error - */ - Error = 'error' -} - -/** - * Extended environment with runtime status information - */ -export interface DeepnoteEnvironmentWithStatus extends DeepnoteEnvironment { - status: EnvironmentStatus; - errorMessage?: string; -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts deleted file mode 100644 index cf383d16a8..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts +++ /dev/null @@ -1,318 +0,0 @@ -import { injectable, inject, named } from 'inversify'; -import { EventEmitter, Uri, CancellationToken, l10n } from 'vscode'; -import { generateUuid as uuid } from '../../../platform/common/uuid'; -import { IExtensionContext, IOutputChannel } from '../../../platform/common/types'; -import { IExtensionSyncActivationService } from '../../../platform/activation/types'; -import { logger } from '../../../platform/logging'; -import { DeepnoteEnvironmentStorage } from './deepnoteEnvironmentStorage.node'; -import { - CreateDeepnoteEnvironmentOptions, - DeepnoteEnvironment, - DeepnoteEnvironmentWithStatus, - EnvironmentStatus -} from './deepnoteEnvironment'; -import { IDeepnoteEnvironmentManager, IDeepnoteServerStarter, IDeepnoteToolkitInstaller } from '../types'; -import { Cancellation } from '../../../platform/common/cancellation'; -import { STANDARD_OUTPUT_CHANNEL } from '../../../platform/common/constants'; - -/** - * Manager for Deepnote kernel environments. - * Handles CRUD operations and server lifecycle management. - */ -@injectable() -export class DeepnoteEnvironmentManager implements IExtensionSyncActivationService, IDeepnoteEnvironmentManager { - private environments: Map = new Map(); - private readonly _onDidChangeEnvironments = new EventEmitter(); - public readonly onDidChangeEnvironments = this._onDidChangeEnvironments.event; - private initializationPromise: Promise | undefined; - - constructor( - @inject(IExtensionContext) private readonly context: IExtensionContext, - @inject(DeepnoteEnvironmentStorage) private readonly storage: DeepnoteEnvironmentStorage, - @inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller, - @inject(IDeepnoteServerStarter) private readonly serverStarter: IDeepnoteServerStarter, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel - ) {} - - /** - * Activate the service (called by VS Code on extension activation) - */ - public activate(): void { - // Store the initialization promise so other components can wait for it - this.initializationPromise = this.initialize().catch((error) => { - logger.error('Failed to activate environment manager', error); - const msg = error instanceof Error ? error.message : String(error); - this.outputChannel.appendLine(l10n.t('Failed to activate environment manager: {0}', msg)); - }); - } - - /** - * Initialize the manager by loading environments from storage - */ - public async initialize(): Promise { - try { - const configs = await this.storage.loadEnvironments(); - this.environments.clear(); - - for (const config of configs) { - this.environments.set(config.id, config); - } - - logger.info(`Initialized environment manager with ${this.environments.size} environments`); - - // Fire event to notify tree view of loaded environments - this._onDidChangeEnvironments.fire(); - } catch (error) { - logger.error('Failed to initialize environment manager', error); - } - } - - /** - * Wait for initialization to complete - */ - public async waitForInitialization(): Promise { - if (this.initializationPromise) { - await this.initializationPromise; - } - } - - /** - * Create a new kernel environment - */ - public async createEnvironment( - options: CreateDeepnoteEnvironmentOptions, - token?: CancellationToken - ): Promise { - Cancellation.throwIfCanceled(token); - - const id = uuid(); - const venvPath = Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', id); - - const environment: DeepnoteEnvironment = { - id, - name: options.name, - pythonInterpreter: options.pythonInterpreter, - venvPath, - createdAt: new Date(), - lastUsedAt: new Date(), - packages: options.packages, - description: options.description - }; - - Cancellation.throwIfCanceled(token); - - this.environments.set(id, environment); - await this.persistEnvironments(); - this._onDidChangeEnvironments.fire(); - - logger.info(`Created new environment: ${environment.name} (${id})`); - return environment; - } - - /** - * Get all environments - */ - public listEnvironments(): DeepnoteEnvironment[] { - return Array.from(this.environments.values()); - } - - /** - * Get a specific environment by ID - */ - public getEnvironment(id: string): DeepnoteEnvironment | undefined { - return this.environments.get(id); - } - - /** - * Get environment with status information - */ - public getEnvironmentWithStatus(id: string): DeepnoteEnvironmentWithStatus | undefined { - const config = this.environments.get(id); - if (!config) { - return undefined; - } - - let status: EnvironmentStatus; - if (config.serverInfo) { - status = EnvironmentStatus.Running; - } else { - status = EnvironmentStatus.Stopped; - } - - return { - ...config, - status - }; - } - - /** - * Update an environment's metadata - */ - public async updateEnvironment( - id: string, - updates: Partial> - ): Promise { - const config = this.environments.get(id); - if (!config) { - throw new Error(l10n.t('Environment not found: {0}', id)); - } - - if (updates.name !== undefined) { - config.name = updates.name; - } - if (updates.packages !== undefined) { - config.packages = updates.packages; - } - if (updates.description !== undefined) { - config.description = updates.description; - } - - await this.persistEnvironments(); - this._onDidChangeEnvironments.fire(); - - logger.info(`Updated environment: ${config.name} (${id})`); - } - - /** - * Delete an environment - */ - public async deleteEnvironment(id: string, token?: CancellationToken): Promise { - Cancellation.throwIfCanceled(token); - - const config = this.environments.get(id); - if (!config) { - throw new Error(`Environment not found: ${id}`); - } - - // Stop the server if running - if (config.serverInfo) { - await this.stopServer(id, token); - } - - Cancellation.throwIfCanceled(token); - - this.environments.delete(id); - await this.persistEnvironments(); - this._onDidChangeEnvironments.fire(); - - logger.info(`Deleted environment: ${config.name} (${id})`); - } - - /** - * Start the Jupyter server for an environment - */ - public async startServer(id: string, token?: CancellationToken): Promise { - const config = this.environments.get(id); - if (!config) { - throw new Error(`Environment not found: ${id}`); - } - - try { - logger.info(`Ensuring server is running for environment: ${config.name} (${id})`); - - // First ensure venv is created and toolkit is installed - const { pythonInterpreter, toolkitVersion } = await this.toolkitInstaller.ensureVenvAndToolkit( - config.pythonInterpreter, - config.venvPath, - token - ); - - // Install additional packages if specified - if (config.packages && config.packages.length > 0) { - await this.toolkitInstaller.installAdditionalPackages(config.venvPath, config.packages, token); - } - - // Start the Jupyter server (serverStarter is idempotent - returns existing if running) - // IMPORTANT: Always call this to ensure we get the current server info - // Don't return early based on config.serverInfo - it may be stale! - const serverInfo = await this.serverStarter.startServer(pythonInterpreter, config.venvPath, id, token); - - config.pythonInterpreter = pythonInterpreter; - config.toolkitVersion = toolkitVersion; - config.serverInfo = serverInfo; - config.lastUsedAt = new Date(); - - await this.persistEnvironments(); - this._onDidChangeEnvironments.fire(); - - logger.info(`Server running for environment: ${config.name} (${id}) at ${serverInfo.url}`); - } catch (error) { - logger.error(`Failed to start server for environment: ${config.name} (${id})`, error); - throw error; - } - } - - /** - * Stop the Jupyter server for an environment - */ - public async stopServer(id: string, token?: CancellationToken): Promise { - Cancellation.throwIfCanceled(token); - - const config = this.environments.get(id); - if (!config) { - throw new Error(`Environment not found: ${id}`); - } - - if (!config.serverInfo) { - logger.info(`No server running for environment: ${config.name} (${id})`); - return; - } - - try { - logger.info(`Stopping server for environment: ${config.name} (${id})`); - - await this.serverStarter.stopServer(id, token); - - Cancellation.throwIfCanceled(token); - - config.serverInfo = undefined; - - await this.persistEnvironments(); - this._onDidChangeEnvironments.fire(); - - logger.info(`Server stopped successfully for environment: ${config.name} (${id})`); - } catch (error) { - logger.error(`Failed to stop server for environment: ${config.name} (${id})`, error); - throw error; - } - } - - /** - * Restart the Jupyter server for an environment - */ - public async restartServer(id: string, token?: CancellationToken): Promise { - logger.info(`Restarting server for environment: ${id}`); - await this.stopServer(id, token); - Cancellation.throwIfCanceled(token); - await this.startServer(id, token); - } - - /** - * Update the last used timestamp for an environment - */ - public async updateLastUsed(id: string): Promise { - const config = this.environments.get(id); - if (!config) { - return; - } - - config.lastUsedAt = new Date(); - await this.persistEnvironments(); - } - - /** - * Persist all environments to storage - */ - private async persistEnvironments(): Promise { - const configs = Array.from(this.environments.values()); - await this.storage.saveEnvironments(configs); - } - - /** - * Dispose of all resources - */ - public dispose(): void { - this.outputChannel.dispose(); - this._onDidChangeEnvironments.dispose(); - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts deleted file mode 100644 index d4bf451043..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts +++ /dev/null @@ -1,557 +0,0 @@ -import { assert, use } from 'chai'; -import chaiAsPromised from 'chai-as-promised'; -import { anything, instance, mock, when, verify, deepEqual } from 'ts-mockito'; -import { Uri } from 'vscode'; -import { DeepnoteEnvironmentManager } from './deepnoteEnvironmentManager.node'; -import { DeepnoteEnvironmentStorage } from './deepnoteEnvironmentStorage.node'; -import { IExtensionContext, IOutputChannel } from '../../../platform/common/types'; -import { - IDeepnoteServerStarter, - IDeepnoteToolkitInstaller, - DeepnoteServerInfo, - VenvAndToolkitInstallation, - DEEPNOTE_TOOLKIT_VERSION -} from '../types'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; -import { EnvironmentStatus } from './deepnoteEnvironment'; - -use(chaiAsPromised); - -suite('DeepnoteEnvironmentManager', () => { - let manager: DeepnoteEnvironmentManager; - let mockContext: IExtensionContext; - let mockStorage: DeepnoteEnvironmentStorage; - let mockToolkitInstaller: IDeepnoteToolkitInstaller; - let mockServerStarter: IDeepnoteServerStarter; - let mockOutputChannel: IOutputChannel; - - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const testServerInfo: DeepnoteServerInfo = { - url: 'http://localhost:8888', - jupyterPort: 8888, - lspPort: 8889, - token: 'test-token' - }; - - const testVenvAndToolkit: VenvAndToolkitInstallation = { - pythonInterpreter: testInterpreter, - toolkitVersion: DEEPNOTE_TOOLKIT_VERSION - }; - - setup(() => { - mockContext = mock(); - mockStorage = mock(); - mockToolkitInstaller = mock(); - mockServerStarter = mock(); - mockOutputChannel = mock(); - - when(mockContext.globalStorageUri).thenReturn(Uri.file('/global/storage')); - when(mockStorage.loadEnvironments()).thenResolve([]); - - manager = new DeepnoteEnvironmentManager( - instance(mockContext), - instance(mockStorage), - instance(mockToolkitInstaller), - instance(mockServerStarter), - instance(mockOutputChannel) - ); - }); - - suite('activate', () => { - test('should load environments on activation', async () => { - const existingConfigs = [ - { - id: 'existing-config', - name: 'Existing', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date(), - lastUsedAt: new Date() - } - ]; - - when(mockStorage.loadEnvironments()).thenResolve(existingConfigs); - - manager.activate(); - // Wait for async initialization - await manager.waitForInitialization(); - - const configs = manager.listEnvironments(); - assert.strictEqual(configs.length, 1); - assert.strictEqual(configs[0].id, 'existing-config'); - }); - }); - - suite('createEnvironment', () => { - test('should create a new kernel environment', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test Config', - pythonInterpreter: testInterpreter, - packages: ['numpy'], - description: 'Test description' - }); - - assert.strictEqual(config.name, 'Test Config'); - assert.strictEqual(config.pythonInterpreter, testInterpreter); - assert.deepStrictEqual(config.packages, ['numpy']); - assert.strictEqual(config.description, 'Test description'); - assert.ok(config.id); - assert.ok(config.venvPath); - assert.ok(config.createdAt); - assert.ok(config.lastUsedAt); - - verify(mockStorage.saveEnvironments(anything())).once(); - }); - - test('should generate unique IDs for each environment', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config1 = await manager.createEnvironment({ - name: 'Config 1', - pythonInterpreter: testInterpreter - }); - - const config2 = await manager.createEnvironment({ - name: 'Config 2', - pythonInterpreter: testInterpreter - }); - - assert.notEqual(config1.id, config2.id); - }); - - test('should fire onDidChangeEnvironments event', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - let eventFired = false; - manager.onDidChangeEnvironments(() => { - eventFired = true; - }); - - await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - assert.isTrue(eventFired); - }); - }); - - suite('listEnvironments', () => { - test('should return empty array initially', () => { - const configs = manager.listEnvironments(); - assert.deepStrictEqual(configs, []); - }); - - test('should return all created environments', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - await manager.createEnvironment({ name: 'Config 1', pythonInterpreter: testInterpreter }); - await manager.createEnvironment({ name: 'Config 2', pythonInterpreter: testInterpreter }); - - const configs = manager.listEnvironments(); - assert.strictEqual(configs.length, 2); - }); - }); - - suite('getEnvironment', () => { - test('should return undefined for non-existent ID', () => { - const config = manager.getEnvironment('non-existent'); - assert.isUndefined(config); - }); - - test('should return environment by ID', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const created = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - const found = manager.getEnvironment(created.id); - assert.strictEqual(found?.id, created.id); - assert.strictEqual(found?.name, 'Test'); - }); - }); - - suite('getEnvironmentWithStatus', () => { - test('should return environment with stopped status when server is not running', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const created = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - const withStatus = manager.getEnvironmentWithStatus(created.id); - assert.strictEqual(withStatus?.status, EnvironmentStatus.Stopped); - }); - - test('should return environment with running status when server is running', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - - const created = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.startServer(created.id); - - const withStatus = manager.getEnvironmentWithStatus(created.id); - assert.strictEqual(withStatus?.status, EnvironmentStatus.Running); - }); - }); - - suite('updateEnvironment', () => { - test('should update environment name', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Original Name', - pythonInterpreter: testInterpreter - }); - - await manager.updateEnvironment(config.id, { name: 'Updated Name' }); - - const updated = manager.getEnvironment(config.id); - assert.strictEqual(updated?.name, 'Updated Name'); - verify(mockStorage.saveEnvironments(anything())).atLeast(1); - }); - - test('should update packages', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter, - packages: ['numpy'] - }); - - await manager.updateEnvironment(config.id, { packages: ['numpy', 'pandas'] }); - - const updated = manager.getEnvironment(config.id); - assert.deepStrictEqual(updated?.packages, ['numpy', 'pandas']); - verify(mockStorage.saveEnvironments(anything())).atLeast(1); - }); - - test('should throw error for non-existent environment', async () => { - await assert.isRejected( - manager.updateEnvironment('non-existent', { name: 'Test' }), - 'Environment not found: non-existent' - ); - }); - - test('should fire onDidChangeEnvironments event', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - let eventFired = false; - manager.onDidChangeEnvironments(() => { - eventFired = true; - }); - - await manager.updateEnvironment(config.id, { name: 'Updated' }); - - assert.isTrue(eventFired); - }); - }); - - suite('deleteEnvironment', () => { - test('should delete environment', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.deleteEnvironment(config.id); - - const deleted = manager.getEnvironment(config.id); - assert.isUndefined(deleted); - verify(mockStorage.saveEnvironments(anything())).atLeast(1); - }); - - test('should stop server before deleting if running', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - when(mockServerStarter.stopServer(anything(), anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.startServer(config.id); - await manager.deleteEnvironment(config.id); - - verify(mockServerStarter.stopServer(config.id, anything())).once(); - }); - - test('should throw error for non-existent environment', async () => { - await assert.isRejected(manager.deleteEnvironment('non-existent'), 'Environment not found: non-existent'); - }); - }); - - suite('startServer', () => { - test('should start server for environment', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.startServer(config.id); - - const updated = manager.getEnvironment(config.id); - assert.deepStrictEqual(updated?.serverInfo, testServerInfo); - - verify(mockToolkitInstaller.ensureVenvAndToolkit(testInterpreter, anything(), anything())).once(); - verify(mockServerStarter.startServer(testInterpreter, anything(), config.id, anything())).once(); - }); - - test('should install additional packages when specified', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockToolkitInstaller.installAdditionalPackages(anything(), anything(), anything())).thenResolve(); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter, - packages: ['numpy', 'pandas'] - }); - - await manager.startServer(config.id); - - verify( - mockToolkitInstaller.installAdditionalPackages(anything(), deepEqual(['numpy', 'pandas']), anything()) - ).once(); - }); - - test('should always call serverStarter.startServer to ensure fresh serverInfo (UT-6)', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.startServer(config.id); - await manager.startServer(config.id); - - // IMPORTANT: Should call TWICE - this ensures we always get fresh serverInfo - // The serverStarter itself is idempotent and returns existing server if running - // But the environment manager always calls it to ensure config.serverInfo is updated - verify(mockServerStarter.startServer(anything(), anything(), anything(), anything())).twice(); - }); - - test('should update environment.serverInfo even if server was already running (INV-10)', async () => { - // This test explicitly verifies INV-10: serverInfo must always reflect current server state - // This is critical for environment switching - prevents using stale serverInfo - - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - - // First call returns initial serverInfo - const initialServerInfo: DeepnoteServerInfo = { - url: 'http://localhost:8888', - jupyterPort: 8888, - lspPort: 8889, - token: 'initial-token' - }; - - // Second call returns updated serverInfo (simulating server restart or port change) - const updatedServerInfo: DeepnoteServerInfo = { - url: 'http://localhost:9999', - jupyterPort: 9999, - lspPort: 10000, - token: 'updated-token' - }; - - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())) - .thenResolve(initialServerInfo) - .thenResolve(updatedServerInfo); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - // First startServer call - await manager.startServer(config.id); - let retrieved = manager.getEnvironment(config.id); - assert.deepStrictEqual(retrieved?.serverInfo, initialServerInfo, 'Should have initial serverInfo'); - - // Second startServer call - should update to new serverInfo - await manager.startServer(config.id); - retrieved = manager.getEnvironment(config.id); - assert.deepStrictEqual(retrieved?.serverInfo, updatedServerInfo, 'Should have updated serverInfo'); - - // This proves that getEnvironment() after startServer() always returns fresh data - // which is exactly what the kernel selector relies on (see deepnoteKernelAutoSelector.node.ts:454-467) - }); - - test('should update lastUsedAt timestamp', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - const originalLastUsed = config.lastUsedAt.getTime(); - await manager.startServer(config.id); - const updated = manager.getEnvironment(config.id)!; - assert.isAtLeast(updated.lastUsedAt.getTime(), originalLastUsed); - }); - - test('should throw error for non-existent environment', async () => { - await assert.isRejected(manager.startServer('non-existent'), 'Environment not found: non-existent'); - }); - }); - - suite('stopServer', () => { - test('should stop running server', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - when(mockServerStarter.stopServer(anything(), anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.startServer(config.id); - await manager.stopServer(config.id); - - const updated = manager.getEnvironment(config.id); - assert.isUndefined(updated?.serverInfo); - - verify(mockServerStarter.stopServer(config.id, anything())).once(); - }); - - test('should do nothing if server is not running', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.stopServer(config.id); - - verify(mockServerStarter.stopServer(anything(), anything())).never(); - }); - - test('should throw error for non-existent environment', async () => { - await assert.isRejected(manager.stopServer('non-existent'), 'Environment not found: non-existent'); - }); - }); - - suite('restartServer', () => { - test('should stop and start server', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - when(mockToolkitInstaller.ensureVenvAndToolkit(anything(), anything(), anything())).thenResolve( - testVenvAndToolkit - ); - when(mockServerStarter.startServer(anything(), anything(), anything(), anything())).thenResolve( - testServerInfo - ); - when(mockServerStarter.stopServer(anything(), anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - await manager.startServer(config.id); - await manager.restartServer(config.id); - - verify(mockServerStarter.stopServer(config.id, anything())).once(); - // Called twice: once for initial start, once for restart - verify(mockServerStarter.startServer(anything(), anything(), anything(), anything())).twice(); - }); - }); - - suite('updateLastUsed', () => { - test('should update lastUsedAt timestamp', async () => { - when(mockStorage.saveEnvironments(anything())).thenResolve(); - - const config = await manager.createEnvironment({ - name: 'Test', - pythonInterpreter: testInterpreter - }); - - const originalLastUsed = config.lastUsedAt; - await new Promise((resolve) => setTimeout(resolve, 10)); - await manager.updateLastUsed(config.id); - - const updated = manager.getEnvironment(config.id); - assert.isTrue(updated!.lastUsedAt > originalLastUsed); - }); - - test('should do nothing for non-existent environment', async () => { - await manager.updateLastUsed('non-existent'); - // Should not throw - }); - }); - - suite('dispose', () => { - test('should dispose event emitter', () => { - manager.dispose(); - // Should not throw - }); - }); -}); diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentPicker.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentPicker.ts deleted file mode 100644 index 06f2081945..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentPicker.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { inject, injectable } from 'inversify'; -import { QuickPickItem, window, Uri, commands, l10n } from 'vscode'; -import { logger } from '../../../platform/logging'; -import { IDeepnoteEnvironmentManager } from '../types'; -import { DeepnoteEnvironment, EnvironmentStatus } from './deepnoteEnvironment'; -import { getDisplayPath } from '../../../platform/common/platform/fs-paths'; -import { getDeepnoteEnvironmentStatusVisual } from './deepnoteEnvironmentUi'; - -/** - * Handles showing environment picker UI for notebook selection - */ -@injectable() -export class DeepnoteEnvironmentPicker { - constructor( - @inject(IDeepnoteEnvironmentManager) private readonly environmentManager: IDeepnoteEnvironmentManager - ) {} - - /** - * Show a quick pick to select an environment for a notebook - * @param notebookUri The notebook URI (for context in messages) - * @returns Selected environment, or undefined if cancelled - */ - public async pickEnvironment(notebookUri: Uri): Promise { - // Wait for environment manager to finish loading environments from storage - await this.environmentManager.waitForInitialization(); - - const environments = this.environmentManager.listEnvironments(); - - // Build quick pick items - const items: (QuickPickItem & { environment?: DeepnoteEnvironment })[] = environments.map((env) => { - const envWithStatus = this.environmentManager.getEnvironmentWithStatus(env.id); - const { icon, text } = getDeepnoteEnvironmentStatusVisual( - envWithStatus?.status || EnvironmentStatus.Stopped - ); - - return { - label: `$(${icon}) ${env.name} [${text}]`, - description: getDisplayPath(env.pythonInterpreter.uri), - detail: env.packages?.length - ? l10n.t('Packages: {0}', env.packages.join(', ')) - : l10n.t('No additional packages'), - environment: env - }; - }); - - // Add "Create new" option at the end - items.push({ - label: '$(add) Create New Environment', - description: 'Set up a new kernel environment', - alwaysShow: true - }); - - const selected = await window.showQuickPick(items, { - placeHolder: `Select an environment for ${getDisplayPath(notebookUri)}`, - matchOnDescription: true, - matchOnDetail: true - }); - - if (!selected) { - return undefined; // User cancelled - } - - if (!selected.environment) { - // User chose "Create new" - execute the create command and retry - logger.info('User chose to create new environment - triggering create command'); - await commands.executeCommand('deepnote.environments.create'); - - // After creation, refresh the list and show picker again - const newEnvironments = this.environmentManager.listEnvironments(); - if (newEnvironments.length > environments.length) { - // A new environment was created, show the picker again - logger.info('Environment created, showing picker again'); - return this.pickEnvironment(notebookUri); - } - - // User cancelled creation - logger.info('No new environment created'); - return undefined; - } - - logger.info(`Selected environment "${selected.environment.name}" for notebook ${getDisplayPath(notebookUri)}`); - return selected.environment; - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts deleted file mode 100644 index 52508603b7..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts +++ /dev/null @@ -1,122 +0,0 @@ -import { injectable, inject } from 'inversify'; -import { Memento, Uri } from 'vscode'; -import { IExtensionContext } from '../../../platform/common/types'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; -import { logger } from '../../../platform/logging'; -import { DeepnoteEnvironment, DeepnoteEnvironmentState } from './deepnoteEnvironment'; - -const STORAGE_KEY = 'deepnote.kernelEnvironments'; - -/** - * Service for persisting and loading environments from global storage. - */ -@injectable() -export class DeepnoteEnvironmentStorage { - private readonly globalState: Memento; - - constructor(@inject(IExtensionContext) context: IExtensionContext) { - this.globalState = context.globalState; - } - - /** - * Load all environments from storage - */ - public async loadEnvironments(): Promise { - try { - const states = this.globalState.get(STORAGE_KEY, []); - const environments: DeepnoteEnvironment[] = []; - - for (const state of states) { - const config = this.deserializeEnvironment(state); - if (config) { - environments.push(config); - } else { - logger.error(`Failed to deserialize environment: ${state.id}`); - } - } - - logger.info(`Loaded ${environments.length} environments from storage`); - return environments; - } catch (error) { - logger.error('Failed to load environments', error); - return []; - } - } - - /** - * Save all environments to storage - */ - public async saveEnvironments(environments: DeepnoteEnvironment[]): Promise { - try { - const states = environments.map((config) => this.serializeEnvironment(config)); - await this.globalState.update(STORAGE_KEY, states); - logger.info(`Saved ${environments.length} environments to storage`); - } catch (error) { - logger.error('Failed to save environments', error); - throw error; - } - } - - /** - * Serialize an environment to a storable state - */ - private serializeEnvironment(config: DeepnoteEnvironment): DeepnoteEnvironmentState { - return { - id: config.id, - name: config.name, - pythonInterpreterPath: { - id: config.pythonInterpreter.id, - uri: config.pythonInterpreter.uri.toString(true) - }, - venvPath: config.venvPath.toString(true), - createdAt: config.createdAt.toISOString(), - lastUsedAt: config.lastUsedAt.toISOString(), - packages: config.packages, - toolkitVersion: config.toolkitVersion, - description: config.description - }; - } - - /** - * Deserialize a stored state back to an environment - */ - private deserializeEnvironment(state: DeepnoteEnvironmentState): DeepnoteEnvironment | undefined { - try { - // Create PythonEnvironment directly from stored path - // No need to resolve through interpreter service - we just need the path - const interpreter: PythonEnvironment = { - uri: Uri.parse(state.pythonInterpreterPath.uri), - id: state.pythonInterpreterPath.id - }; - - return { - id: state.id, - name: state.name, - pythonInterpreter: interpreter, - venvPath: Uri.parse(state.venvPath), - createdAt: new Date(state.createdAt), - lastUsedAt: new Date(state.lastUsedAt), - packages: state.packages, - toolkitVersion: state.toolkitVersion, - description: state.description, - serverInfo: undefined // Don't persist server info across sessions - }; - } catch (error) { - logger.error(`Failed to deserialize environment ${state.id}`, error); - return undefined; - } - } - - /** - * Clear all environments from storage - */ - public async clearEnvironments(): Promise { - try { - await this.globalState.update(STORAGE_KEY, []); - logger.info('Cleared all environments from storage'); - } catch (error) { - logger.error('Failed to clear environments', error); - throw error; - } - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts deleted file mode 100644 index c706121bd5..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts +++ /dev/null @@ -1,220 +0,0 @@ -import { assert, use } from 'chai'; -import { anything, instance, mock, when, verify, deepEqual } from 'ts-mockito'; -import { Memento, Uri } from 'vscode'; -import { DeepnoteEnvironmentStorage } from './deepnoteEnvironmentStorage.node'; -import { IExtensionContext } from '../../../platform/common/types'; -import { IInterpreterService } from '../../../platform/interpreter/contracts'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; -import { DeepnoteEnvironmentState } from './deepnoteEnvironment'; -import chaiAsPromised from 'chai-as-promised'; - -use(chaiAsPromised); - -suite('DeepnoteEnvironmentStorage', () => { - let storage: DeepnoteEnvironmentStorage; - let mockContext: IExtensionContext; - let mockInterpreterService: IInterpreterService; - let mockGlobalState: Memento; - - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3') - }; - - setup(() => { - mockContext = mock(); - mockInterpreterService = mock(); - mockGlobalState = mock(); - - when(mockContext.globalState).thenReturn(instance(mockGlobalState) as any); - - storage = new DeepnoteEnvironmentStorage(instance(mockContext)); - }); - - suite('loadEnvironments', () => { - test('should return empty array when no environments are stored', async () => { - when(mockGlobalState.get('deepnote.kernelEnvironments', anything())).thenReturn([]); - - const configs = await storage.loadEnvironments(); - - assert.deepStrictEqual(configs, []); - }); - - test('should load and deserialize stored environments', async () => { - const storedState: DeepnoteEnvironmentState = { - id: 'config-1', - name: 'Test Config', - pythonInterpreterPath: { - id: 'test-python-id', - uri: '/usr/bin/python3' - }, - venvPath: '/path/to/venv', - createdAt: '2025-01-01T00:00:00.000Z', - lastUsedAt: '2025-01-01T00:00:00.000Z', - packages: ['numpy', 'pandas'], - toolkitVersion: '0.2.30', - description: 'Test environment' - }; - - when(mockGlobalState.get('deepnote.kernelEnvironments', anything())).thenReturn([storedState]); - when(mockInterpreterService.getInterpreterDetails(anything())).thenResolve(testInterpreter); - - const configs = await storage.loadEnvironments(); - - assert.strictEqual(configs.length, 1); - assert.strictEqual(configs[0].id, 'config-1'); - assert.strictEqual(configs[0].name, 'Test Config'); - const expectedInterpreterFsPath = Uri.file(storedState.pythonInterpreterPath.uri).fsPath; - const expectedVenvFsPath = Uri.file(storedState.venvPath).fsPath; - assert.strictEqual(configs[0].pythonInterpreter.uri.fsPath, expectedInterpreterFsPath); - assert.strictEqual(configs[0].venvPath.fsPath, expectedVenvFsPath); - assert.deepStrictEqual(configs[0].packages, ['numpy', 'pandas']); - assert.strictEqual(configs[0].toolkitVersion, '0.2.30'); - assert.strictEqual(configs[0].description, 'Test environment'); - }); - - test('should load all environments including those with potentially invalid paths', async () => { - const storedStates: DeepnoteEnvironmentState[] = [ - { - id: 'config-1', - name: 'Valid Config', - pythonInterpreterPath: { - id: 'test-python-id', - uri: '/usr/bin/python3' - }, - venvPath: '/path/to/venv1', - createdAt: '2025-01-01T00:00:00.000Z', - lastUsedAt: '2025-01-01T00:00:00.000Z' - }, - { - id: 'config-2', - name: 'Potentially Invalid Config', - pythonInterpreterPath: { - id: 'test-python-id', - uri: '/invalid/python' - }, - venvPath: '/path/to/venv2', - createdAt: '2025-01-01T00:00:00.000Z', - lastUsedAt: '2025-01-01T00:00:00.000Z' - } - ]; - - when(mockGlobalState.get('deepnote.kernelEnvironments', anything())).thenReturn(storedStates); - - const configs = await storage.loadEnvironments(); - - // All environments should be loaded - interpreter validation happens at usage time, not load time - assert.strictEqual(configs.length, 2); - assert.strictEqual(configs[0].id, 'config-1'); - assert.strictEqual(configs[1].id, 'config-2'); - }); - - test('should handle errors gracefully and return empty array', async () => { - when(mockGlobalState.get('deepnote.kernelEnvironments', anything())).thenThrow(new Error('Storage error')); - - const configs = await storage.loadEnvironments(); - - assert.deepStrictEqual(configs, []); - }); - }); - - suite('saveEnvironments', () => { - test('should serialize and save environments', async () => { - const config = { - id: 'config-1', - name: 'Test Config', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date('2025-01-01T00:00:00.000Z'), - lastUsedAt: new Date('2025-01-01T00:00:00.000Z'), - packages: ['numpy'], - toolkitVersion: '0.2.30', - description: 'Test' - }; - - when(mockGlobalState.update(anything(), anything())).thenResolve(); - - await storage.saveEnvironments([config]); - - verify( - mockGlobalState.update( - 'deepnote.kernelEnvironments', - deepEqual([ - { - id: 'config-1', - name: 'Test Config', - pythonInterpreterPath: { - id: 'test-python-id', - uri: 'file:///usr/bin/python3' - }, - venvPath: 'file:///path/to/venv', - createdAt: '2025-01-01T00:00:00.000Z', - lastUsedAt: '2025-01-01T00:00:00.000Z', - packages: ['numpy'], - toolkitVersion: '0.2.30', - description: 'Test' - } - ]) - ) - ).once(); - }); - - test('should save multiple environments', async () => { - const configs = [ - { - id: 'config-1', - name: 'Config 1', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv1'), - createdAt: new Date('2025-01-01T00:00:00.000Z'), - lastUsedAt: new Date('2025-01-01T00:00:00.000Z') - }, - { - id: 'config-2', - name: 'Config 2', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv2'), - createdAt: new Date('2025-01-02T00:00:00.000Z'), - lastUsedAt: new Date('2025-01-02T00:00:00.000Z') - } - ]; - - when(mockGlobalState.update(anything(), anything())).thenResolve(); - - await storage.saveEnvironments(configs); - - verify(mockGlobalState.update('deepnote.kernelEnvironments', anything())).once(); - }); - - test('should throw error if storage update fails', async () => { - const config = { - id: 'config-1', - name: 'Test Config', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date(), - lastUsedAt: new Date() - }; - - when(mockGlobalState.update(anything(), anything())).thenReject(new Error('Storage error')); - - await assert.isRejected(storage.saveEnvironments([config]), 'Storage error'); - }); - }); - - suite('clearEnvironments', () => { - test('should clear all stored environments', async () => { - when(mockGlobalState.update(anything(), anything())).thenResolve(); - - await storage.clearEnvironments(); - - verify(mockGlobalState.update('deepnote.kernelEnvironments', deepEqual([]))).once(); - }); - - test('should throw error if clear fails', async () => { - when(mockGlobalState.update(anything(), anything())).thenReject(new Error('Storage error')); - - await assert.isRejected(storage.clearEnvironments(), 'Storage error'); - }); - }); -}); diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts deleted file mode 100644 index 1cdd915d48..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts +++ /dev/null @@ -1,175 +0,0 @@ -import { Disposable, Event, EventEmitter, TreeDataProvider, TreeItem } from 'vscode'; -import { IDeepnoteEnvironmentManager } from '../types'; -import { EnvironmentTreeItemType, DeepnoteEnvironmentTreeItem } from './deepnoteEnvironmentTreeItem.node'; -import { EnvironmentStatus } from './deepnoteEnvironment'; -import { inject, injectable } from 'inversify'; -import { IExtensionSyncActivationService } from '../../../platform/activation/types'; - -/** - * Tree data provider for the Deepnote kernel environments view - */ -@injectable() -export class DeepnoteEnvironmentTreeDataProvider - implements TreeDataProvider, IExtensionSyncActivationService, Disposable -{ - private readonly _onDidChangeTreeData = new EventEmitter(); - private readonly disposables: Disposable[] = []; - - constructor(@inject(IDeepnoteEnvironmentManager) private readonly environmentManager: IDeepnoteEnvironmentManager) { - // Listen to environment changes and refresh the tree - this.disposables.push( - this.environmentManager.onDidChangeEnvironments(() => { - this.refresh(); - }) - ); - } - - public activate(): void { - this.refresh(); - } - - public get onDidChangeTreeData(): Event { - return this._onDidChangeTreeData.event; - } - - public refresh(): void { - this._onDidChangeTreeData.fire(); - } - - public getTreeItem(element: DeepnoteEnvironmentTreeItem): TreeItem { - return element; - } - - public async getChildren(element?: DeepnoteEnvironmentTreeItem): Promise { - if (!element) { - // Root level: show all environments + create action - return this.getRootItems(); - } - - // Expanded environment: show info items - if (element.type === EnvironmentTreeItemType.Environment && element.environment) { - return this.getEnvironmentInfoItems(element); - } - - return []; - } - - private async getRootItems(): Promise { - const environments = this.environmentManager.listEnvironments(); - const items: DeepnoteEnvironmentTreeItem[] = []; - - // Add environment items - for (const config of environments) { - const statusInfo = this.environmentManager.getEnvironmentWithStatus(config.id); - const status = statusInfo?.status || EnvironmentStatus.Stopped; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - // deepnoteEnvironmentToView(config), - config, - status - ); - - items.push(item); - } - - // Add create action at the end - items.push(new DeepnoteEnvironmentTreeItem(EnvironmentTreeItemType.CreateAction)); - - return items; - } - - private getEnvironmentInfoItems(element: DeepnoteEnvironmentTreeItem): DeepnoteEnvironmentTreeItem[] { - const config = element.environment; - if (!config) { - return []; - } - - const items: DeepnoteEnvironmentTreeItem[] = []; - const statusInfo = this.environmentManager.getEnvironmentWithStatus(config.id); - - // Server status and ports - if (statusInfo?.status === EnvironmentStatus.Running && config.serverInfo) { - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem( - 'ports', - config.id, - `Ports: jupyter=${config.serverInfo.jupyterPort}, lsp=${config.serverInfo.lspPort}`, - 'port' - ) - ); - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem('url', config.id, `URL: ${config.serverInfo.url}`, 'globe') - ); - } - - // Python interpreter - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem( - 'python', - config.id, - `Python: ${config.pythonInterpreter.uri.fsPath}`, - 'symbol-namespace' - ) - ); - - // Venv path - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem('venv', config.id, `Venv: ${config.venvPath.fsPath}`, 'folder') - ); - - // Packages - if (config.packages && config.packages.length > 0) { - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem( - 'packages', - config.id, - `Packages: ${config.packages.join(', ')}`, - 'package' - ) - ); - } else { - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem('packages', config.id, 'Packages: (none)', 'package') - ); - } - - // Toolkit version - if (config.toolkitVersion) { - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem( - 'toolkit', - config.id, - `Toolkit: ${config.toolkitVersion}`, - 'versions' - ) - ); - } - - // Timestamps - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem( - 'created', - config.id, - `Created: ${config.createdAt.toLocaleString()}`, - 'history' - ) - ); - - items.push( - DeepnoteEnvironmentTreeItem.createInfoItem( - 'lastUsed', - config.id, - `Last used: ${config.lastUsedAt.toLocaleString()}`, - 'clock' - ) - ); - - return items; - } - - public dispose(): void { - this._onDidChangeTreeData.dispose(); - this.disposables.forEach((d) => d.dispose()); - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts deleted file mode 100644 index 2dfe2348d5..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts +++ /dev/null @@ -1,223 +0,0 @@ -import { assert } from 'chai'; -import { instance, mock, when } from 'ts-mockito'; -import { Uri, EventEmitter } from 'vscode'; -import { DeepnoteEnvironmentTreeDataProvider } from './deepnoteEnvironmentTreeDataProvider.node'; -import { IDeepnoteEnvironmentManager } from '../types'; -import { DeepnoteEnvironment, DeepnoteEnvironmentWithStatus, EnvironmentStatus } from './deepnoteEnvironment'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; -import { EnvironmentTreeItemType } from './deepnoteEnvironmentTreeItem.node'; - -suite('DeepnoteEnvironmentTreeDataProvider', () => { - let provider: DeepnoteEnvironmentTreeDataProvider; - let mockConfigManager: IDeepnoteEnvironmentManager; - let configChangeEmitter: EventEmitter; - - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3') - }; - - const testConfig1: DeepnoteEnvironment = { - id: 'config-1', - name: 'Config 1', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv1'), - createdAt: new Date(), - lastUsedAt: new Date() - }; - - const testConfig2: DeepnoteEnvironment = { - id: 'config-2', - name: 'Config 2', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv2'), - createdAt: new Date(), - lastUsedAt: new Date(), - packages: ['numpy'], - serverInfo: { - url: 'http://localhost:8888', - jupyterPort: 8888, - lspPort: 8889, - token: 'test-token' - } - }; - - setup(() => { - mockConfigManager = mock(); - configChangeEmitter = new EventEmitter(); - - when(mockConfigManager.onDidChangeEnvironments).thenReturn(configChangeEmitter.event); - when(mockConfigManager.listEnvironments()).thenReturn([]); - - provider = new DeepnoteEnvironmentTreeDataProvider(instance(mockConfigManager)); - }); - - suite('getChildren - Root Level', () => { - test('should return create action when no environments exist', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([]); - - const children = await provider.getChildren(); - - assert.strictEqual(children.length, 1); - assert.strictEqual(children[0].type, EnvironmentTreeItemType.CreateAction); - }); - - test('should return environments and create action', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig1, testConfig2]); - when(mockConfigManager.getEnvironmentWithStatus('config-1')).thenReturn({ - ...testConfig1, - status: EnvironmentStatus.Stopped - } as DeepnoteEnvironmentWithStatus); - when(mockConfigManager.getEnvironmentWithStatus('config-2')).thenReturn({ - ...testConfig2, - status: EnvironmentStatus.Running - } as DeepnoteEnvironmentWithStatus); - - const children = await provider.getChildren(); - - assert.strictEqual(children.length, 3); // 2 configs + create action - assert.strictEqual(children[0].type, EnvironmentTreeItemType.Environment); - assert.strictEqual(children[1].type, EnvironmentTreeItemType.Environment); - assert.strictEqual(children[2].type, EnvironmentTreeItemType.CreateAction); - }); - - test('should include status for each environment', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig1, testConfig2]); - when(mockConfigManager.getEnvironmentWithStatus('config-1')).thenReturn({ - ...testConfig1, - status: EnvironmentStatus.Stopped - } as DeepnoteEnvironmentWithStatus); - when(mockConfigManager.getEnvironmentWithStatus('config-2')).thenReturn({ - ...testConfig2, - status: EnvironmentStatus.Running - } as DeepnoteEnvironmentWithStatus); - - const children = await provider.getChildren(); - - assert.strictEqual(children[0].status, EnvironmentStatus.Stopped); - assert.strictEqual(children[1].status, EnvironmentStatus.Running); - }); - }); - - suite('getChildren - Environment Children', () => { - test('should return info items for stopped environment', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig1]); - when(mockConfigManager.getEnvironmentWithStatus('config-1')).thenReturn({ - ...testConfig1, - status: EnvironmentStatus.Stopped - } as DeepnoteEnvironmentWithStatus); - - const rootChildren = await provider.getChildren(); - const configItem = rootChildren[0]; - const infoItems = await provider.getChildren(configItem); - - assert.isAtLeast(infoItems.length, 3); // At least: Python, Venv, Last used - assert.isTrue(infoItems.every((item) => item.type === EnvironmentTreeItemType.InfoItem)); - }); - - test('should include port and URL for running environment', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig2]); - when(mockConfigManager.getEnvironmentWithStatus('config-2')).thenReturn({ - ...testConfig2, - status: EnvironmentStatus.Running - } as DeepnoteEnvironmentWithStatus); - - const rootChildren = await provider.getChildren(); - const configItem = rootChildren[0]; - const infoItems = await provider.getChildren(configItem); - - const labels = infoItems.map((item) => item.label as string); - const hasPort = labels.some((label) => label.includes('Ports:') && label.includes('8888')); - const hasUrl = labels.some((label) => label.includes('URL:') && label.includes('http://localhost:8888')); - - assert.isTrue(hasPort, 'Should include port info'); - assert.isTrue(hasUrl, 'Should include URL info'); - }); - - test('should include packages when present', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig2]); - when(mockConfigManager.getEnvironmentWithStatus('config-2')).thenReturn({ - ...testConfig2, - status: EnvironmentStatus.Running - } as DeepnoteEnvironmentWithStatus); - - const rootChildren = await provider.getChildren(); - const configItem = rootChildren[0]; - const infoItems = await provider.getChildren(configItem); - - const labels = infoItems.map((item) => item.label as string); - const hasPackages = labels.some((label) => label.includes('Packages:') && label.includes('numpy')); - - assert.isTrue(hasPackages); - }); - - test('should return empty array for non-environment items', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([]); - - const rootChildren = await provider.getChildren(); - const createAction = rootChildren[0]; - const children = await provider.getChildren(createAction); - - assert.deepStrictEqual(children, []); - }); - - test('should return empty array for info items', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig1]); - when(mockConfigManager.getEnvironmentWithStatus('config-1')).thenReturn({ - ...testConfig1, - status: EnvironmentStatus.Stopped - } as DeepnoteEnvironmentWithStatus); - - const rootChildren = await provider.getChildren(); - const configItem = rootChildren[0]; - const infoItems = await provider.getChildren(configItem); - const children = await provider.getChildren(infoItems[0]); - - assert.deepStrictEqual(children, []); - }); - }); - - suite('getTreeItem', () => { - test('should return the same tree item', async () => { - when(mockConfigManager.listEnvironments()).thenReturn([testConfig1]); - when(mockConfigManager.getEnvironmentWithStatus('config-1')).thenReturn({ - ...testConfig1, - status: EnvironmentStatus.Stopped - } as DeepnoteEnvironmentWithStatus); - - const children = await provider.getChildren(); - const item = children[0]; - const treeItem = provider.getTreeItem(item); - - assert.strictEqual(treeItem, item); - }); - }); - - suite('refresh', () => { - test('should fire onDidChangeTreeData event', (done) => { - provider.onDidChangeTreeData(() => { - done(); - }); - - provider.refresh(); - }); - }); - - suite('Auto-refresh on environment changes', () => { - test('should refresh when environments change', (done) => { - provider.onDidChangeTreeData(() => { - done(); - }); - - // Simulate environment change - configChangeEmitter.fire(); - }); - }); - - suite('dispose', () => { - test('should dispose without errors', () => { - provider.dispose(); - // Should not throw - }); - }); -}); diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts deleted file mode 100644 index c0b6cb9e48..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts +++ /dev/null @@ -1,156 +0,0 @@ -import { l10n, ThemeColor, ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode'; - -import { DeepnoteEnvironment, EnvironmentStatus } from './deepnoteEnvironment'; -import { getDeepnoteEnvironmentStatusVisual } from './deepnoteEnvironmentUi'; - -/** - * Type of tree item in the environments view - */ -export enum EnvironmentTreeItemType { - Environment = 'environment', - InfoItem = 'info', - CreateAction = 'create' -} - -export type DeepnoteEnvironmentTreeInfoItemId = - | 'ports' - | 'url' - | 'python' - | 'venv' - | 'packages' - | 'toolkit' - | 'created' - | 'lastUsed'; - -/** - * Tree item for displaying environments and related info - */ -export class DeepnoteEnvironmentTreeItem extends TreeItem { - constructor( - public readonly type: EnvironmentTreeItemType, - public readonly environment?: DeepnoteEnvironment, - public readonly status?: EnvironmentStatus, - label?: string, - collapsibleState?: TreeItemCollapsibleState - ) { - super(label || '', collapsibleState); - - if (type === EnvironmentTreeItemType.Environment && environment) { - this.setupEnvironmentItem(); - } else if (type === EnvironmentTreeItemType.InfoItem) { - this.setupInfoItem(); - } else if (type === EnvironmentTreeItemType.CreateAction) { - this.setupCreateAction(); - } - } - - /** - * Create an info item to display under an environment - */ - public static createInfoItem( - id: DeepnoteEnvironmentTreeInfoItemId, - environmentId: string, - label: string, - icon?: string - ): DeepnoteEnvironmentTreeItem { - const item = new DeepnoteEnvironmentTreeItem(EnvironmentTreeItemType.InfoItem, undefined, undefined, label); - item.id = `info-${environmentId}-${id}`; - - if (icon) { - item.iconPath = new ThemeIcon(icon); - } - - return item; - } - - private setupEnvironmentItem(): void { - if (!this.environment) { - return; - } - - const statusVisual = getDeepnoteEnvironmentStatusVisual(this.status ?? EnvironmentStatus.Stopped); - - this.id = this.environment.id; - this.label = `${this.environment.name} [${statusVisual.text}]`; - this.iconPath = new ThemeIcon(statusVisual.icon, new ThemeColor(statusVisual.themeColorId)); - this.contextValue = statusVisual.contextValue; - - // Make it collapsible to show info items - this.collapsibleState = TreeItemCollapsibleState.Collapsed; - - // Set description with last used time - const lastUsed = this.getRelativeTime(this.environment.lastUsedAt); - this.description = l10n.t('Last used: {0}', lastUsed); - - // Set tooltip with detailed info - this.tooltip = this.buildTooltip(); - } - - private setupInfoItem(): void { - // Info items are not clickable and don't have context menus - this.contextValue = 'deepnoteEnvironment.info'; - this.collapsibleState = TreeItemCollapsibleState.None; - } - - private setupCreateAction(): void { - this.id = 'create'; - this.label = l10n.t('Create New Environment'); - this.iconPath = new ThemeIcon('add'); - this.contextValue = 'deepnoteEnvironment.create'; - this.collapsibleState = TreeItemCollapsibleState.None; - this.command = { - command: 'deepnote.environments.create', - title: l10n.t('Create Environment') - }; - } - - private buildTooltip(): string { - if (!this.environment) { - return ''; - } - - const { text } = getDeepnoteEnvironmentStatusVisual(this.status ?? EnvironmentStatus.Stopped); - - const lines: string[] = []; - lines.push(`**${this.environment.name}**`); - lines.push(''); - lines.push(l10n.t('Status: {0}', text)); - lines.push(l10n.t('Python: {0}', this.environment.pythonInterpreter.uri.toString(true))); - lines.push(l10n.t('Venv: {0}', this.environment.venvPath.toString(true))); - - if (this.environment.packages && this.environment.packages.length > 0) { - lines.push(l10n.t('Packages: {0}', this.environment.packages.join(', '))); - } - - if (this.environment.toolkitVersion) { - lines.push(l10n.t('Toolkit: {0}', this.environment.toolkitVersion)); - } - - lines.push(''); - lines.push(l10n.t('Created: {0}', this.environment.createdAt.toLocaleString())); - lines.push(l10n.t('Last used: {0}', this.environment.lastUsedAt.toLocaleString())); - - return lines.join('\n'); - } - - private getRelativeTime(date: Date): string { - const now = new Date(); - const diff = now.getTime() - date.getTime(); - const seconds = Math.floor(diff / 1000); - const minutes = Math.floor(seconds / 60); - const hours = Math.floor(minutes / 60); - const days = Math.floor(hours / 24); - - if (seconds < 60) { - return l10n.t('just now'); - } else if (minutes < 60) { - return minutes === 1 ? l10n.t('1 minute ago') : l10n.t('{0} minutes ago', minutes); - } else if (hours < 24) { - return hours === 1 ? l10n.t('1 hour ago') : l10n.t('{0} hours ago', hours); - } else if (days < 7) { - return days === 1 ? l10n.t('1 day ago') : l10n.t('{0} days ago', days); - } else { - return date.toLocaleDateString(); - } - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts deleted file mode 100644 index 08b12d231e..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts +++ /dev/null @@ -1,276 +0,0 @@ -import { assert } from 'chai'; -import { ThemeIcon, TreeItemCollapsibleState, Uri } from 'vscode'; - -import { DeepnoteEnvironmentTreeItem, EnvironmentTreeItemType } from './deepnoteEnvironmentTreeItem.node'; -import { DeepnoteEnvironment, EnvironmentStatus } from './deepnoteEnvironment'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; - -suite('DeepnoteEnvironmentTreeItem', () => { - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3') - }; - - const testEnvironment: DeepnoteEnvironment = { - id: 'test-config-id', - name: 'Test Environment', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date('2024-01-01T10:00:00Z'), - lastUsedAt: new Date('2024-01-01T12:00:00Z') - }; - - suite('Environment Item', () => { - test('should create running environment item', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Running - ); - - assert.strictEqual(item.type, EnvironmentTreeItemType.Environment); - assert.strictEqual(item.environment, testEnvironment); - assert.strictEqual(item.status, EnvironmentStatus.Running); - assert.include(item.label as string, 'Test Environment'); - assert.include(item.label as string, '[Running]'); - assert.strictEqual(item.collapsibleState, TreeItemCollapsibleState.Collapsed); - assert.strictEqual(item.contextValue, 'deepnoteEnvironment.running'); - }); - - test('should create stopped environment item', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Stopped - ); - - assert.include(item.label as string, '[Stopped]'); - assert.strictEqual(item.contextValue, 'deepnoteEnvironment.stopped'); - }); - - test('should create starting environment item', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Starting - ); - - assert.include(item.label as string, '[Starting...]'); - assert.strictEqual(item.contextValue, 'deepnoteEnvironment.starting'); - }); - - test('should have correct icon for running state', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Running - ); - - assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'vm-running'); - }); - - test('should have correct icon for stopped state', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Stopped - ); - - assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'vm-outline'); - }); - - test('should have correct icon for starting state', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Starting - ); - - assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'loading~spin'); - }); - - test('should include last used time in description', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Stopped - ); - - assert.include(item.description as string, 'Last used:'); - }); - - test('should have tooltip with environment details', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - testEnvironment, - EnvironmentStatus.Running - ); - - const tooltip = `${item.tooltip}`; - assert.include(tooltip, 'Test Environment'); - assert.include(tooltip, 'Running'); - assert.include(tooltip, testInterpreter.uri.toString(true)); - }); - - test('should include packages in tooltip when present', () => { - const configWithPackages: DeepnoteEnvironment = { - ...testEnvironment, - packages: ['numpy', 'pandas'] - }; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - configWithPackages, - EnvironmentStatus.Stopped - ); - - const tooltip = item.tooltip as string; - assert.include(tooltip, 'numpy'); - assert.include(tooltip, 'pandas'); - }); - }); - - suite('Info Item', () => { - test('should create info item', () => { - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.InfoItem, - undefined, - undefined, - 'Info Label' - ); - - assert.strictEqual(item.type, EnvironmentTreeItemType.InfoItem); - assert.strictEqual(item.label, 'Info Label'); - assert.strictEqual(item.contextValue, 'deepnoteEnvironment.info'); - assert.strictEqual(item.collapsibleState, TreeItemCollapsibleState.None); - }); - - test('should create info item with icon', () => { - const item = DeepnoteEnvironmentTreeItem.createInfoItem( - 'ports', - 'test-config-id', - 'Port: 8888', - 'circle-filled' - ); - - assert.strictEqual(item.label, 'Port: 8888'); - assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'circle-filled'); - }); - - test('should create info item without icon', () => { - const item = DeepnoteEnvironmentTreeItem.createInfoItem('ports', 'test-config-id', 'No icon'); - - assert.strictEqual(item.label, 'No icon'); - assert.isUndefined(item.iconPath); - }); - }); - - suite('Create Action Item', () => { - test('should create action item', () => { - const item = new DeepnoteEnvironmentTreeItem(EnvironmentTreeItemType.CreateAction); - - assert.strictEqual(item.type, EnvironmentTreeItemType.CreateAction); - assert.strictEqual(item.label, 'Create New Environment'); - assert.strictEqual(item.contextValue, 'deepnoteEnvironment.create'); - assert.strictEqual(item.collapsibleState, TreeItemCollapsibleState.None); - assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'add'); - }); - - test('should have command', () => { - const item = new DeepnoteEnvironmentTreeItem(EnvironmentTreeItemType.CreateAction); - - assert.ok(item.command); - assert.strictEqual(item.command?.command, 'deepnote.environments.create'); - assert.strictEqual(item.command?.title, 'Create Environment'); - }); - }); - - suite('Relative Time Formatting', () => { - test('should show "just now" for recent times', () => { - const recentConfig: DeepnoteEnvironment = { - ...testEnvironment, - lastUsedAt: new Date() - }; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - recentConfig, - EnvironmentStatus.Stopped - ); - - assert.include(item.description as string, 'just now'); - }); - - test('should handle negative time (few seconds in the past)', () => { - const fewSecondsAgo = new Date(Date.now() - 5 * 1000); - const config: DeepnoteEnvironment = { - ...testEnvironment, - lastUsedAt: fewSecondsAgo - }; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - config, - EnvironmentStatus.Stopped - ); - - assert.include(item.description as string, 'just now'); - }); - - test('should show minutes ago', () => { - const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000); - const config: DeepnoteEnvironment = { - ...testEnvironment, - lastUsedAt: fiveMinutesAgo - }; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - config, - EnvironmentStatus.Stopped - ); - - assert.include(item.description as string, 'minute'); - assert.include(item.description as string, 'ago'); - }); - - test('should show hours ago', () => { - const twoHoursAgo = new Date(Date.now() - 2 * 60 * 60 * 1000); - const config: DeepnoteEnvironment = { - ...testEnvironment, - lastUsedAt: twoHoursAgo - }; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - config, - EnvironmentStatus.Stopped - ); - - assert.include(item.description as string, 'hour'); - assert.include(item.description as string, 'ago'); - }); - - test('should show days ago', () => { - const threeDaysAgo = new Date(Date.now() - 3 * 24 * 60 * 60 * 1000); - const config: DeepnoteEnvironment = { - ...testEnvironment, - lastUsedAt: threeDaysAgo - }; - - const item = new DeepnoteEnvironmentTreeItem( - EnvironmentTreeItemType.Environment, - config, - EnvironmentStatus.Stopped - ); - - assert.include(item.description as string, 'day'); - assert.include(item.description as string, 'ago'); - }); - }); -}); diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentUi.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentUi.ts deleted file mode 100644 index 82aee15880..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentUi.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { l10n } from 'vscode'; - -import { EnvironmentStatus } from './deepnoteEnvironment'; - -export function getDeepnoteEnvironmentStatusVisual(status: EnvironmentStatus): { - icon: string; - text: string; - themeColorId: string; - contextValue: string; -} { - switch (status) { - case EnvironmentStatus.Running: - return { - icon: 'vm-running', - text: l10n.t('Running'), - contextValue: 'deepnoteEnvironment.running', - themeColorId: 'charts.green' - }; - case EnvironmentStatus.Starting: - return { - icon: 'loading~spin', - text: l10n.t('Starting...'), - contextValue: 'deepnoteEnvironment.starting', - themeColorId: 'charts.yellow' - }; - case EnvironmentStatus.Stopped: - return { - icon: 'vm-outline', - text: l10n.t('Stopped'), - contextValue: 'deepnoteEnvironment.stopped', - themeColorId: 'charts.gray' - }; - case EnvironmentStatus.Error: - return { - icon: 'error', - text: l10n.t('Error'), - contextValue: 'deepnoteEnvironment.error', - themeColorId: 'errorForeground' - }; - default: - status satisfies never; - return { - icon: 'vm-outline', - text: l10n.t('Unknown'), - contextValue: 'deepnoteEnvironment.stopped', - themeColorId: 'charts.gray' - }; - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts deleted file mode 100644 index 4ccf8534e9..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { inject, injectable, named } from 'inversify'; -import { IExtensionSyncActivationService } from '../../../platform/activation/types'; -import { IDeepnoteEnvironmentManager } from '../types'; -import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node'; -import { logger } from '../../../platform/logging'; -import { IOutputChannel } from '../../../platform/common/types'; -import { STANDARD_OUTPUT_CHANNEL } from '../../../platform/common/constants'; -import { l10n } from 'vscode'; - -/** - * Activation service for the Deepnote kernel environments view. - * Initializes the environment manager and registers the tree view. - */ -@injectable() -export class DeepnoteEnvironmentsActivationService implements IExtensionSyncActivationService { - constructor( - @inject(IDeepnoteEnvironmentManager) - private readonly environmentManager: IDeepnoteEnvironmentManager, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, - @inject(DeepnoteEnvironmentsView) - _environmentsView: DeepnoteEnvironmentsView - ) { - // _environmentsView is injected to ensure the view is created, - // but we don't need to store a reference to it - } - - public activate(): void { - logger.info('Activating Deepnote kernel environments view'); - - // Initialize the environment manager (loads environments from storage) - this.environmentManager.initialize().then( - () => { - logger.info('Deepnote kernel environments initialized'); - }, - (error: unknown) => { - logger.error('Failed to initialize Deepnote kernel environments', error); - const msg = error instanceof Error ? error.message : String(error); - this.outputChannel.appendLine(l10n.t('Failed to initialize Deepnote kernel environments: {0}', msg)); - } - ); - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts deleted file mode 100644 index 03c2be0902..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { assert } from 'chai'; -import { instance, mock, when, verify } from 'ts-mockito'; -import { DeepnoteEnvironmentsActivationService } from './deepnoteEnvironmentsActivationService'; -import { IDeepnoteEnvironmentManager } from '../types'; -import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node'; -import { IOutputChannel } from '../../../platform/common/types'; - -suite('DeepnoteEnvironmentsActivationService', () => { - let activationService: DeepnoteEnvironmentsActivationService; - let mockConfigManager: IDeepnoteEnvironmentManager; - let mockEnvironmentsView: DeepnoteEnvironmentsView; - let mockOutputChannel: IOutputChannel; - - setup(() => { - mockConfigManager = mock(); - mockEnvironmentsView = mock(); - mockOutputChannel = mock(); - - activationService = new DeepnoteEnvironmentsActivationService( - instance(mockConfigManager), - instance(mockOutputChannel), - instance(mockEnvironmentsView) - ); - }); - - suite('activate', () => { - test('should call initialize on environment manager', async () => { - when(mockConfigManager.initialize()).thenResolve(); - - activationService.activate(); - - // Wait for async initialization - await new Promise((resolve) => setTimeout(resolve, 100)); - - verify(mockConfigManager.initialize()).once(); - }); - - test('should handle initialization errors gracefully', async () => { - when(mockConfigManager.initialize()).thenReject(new Error('Initialization failed')); - - // Should not throw - activationService.activate(); - - // Wait for async initialization - await new Promise((resolve) => setTimeout(resolve, 100)); - - verify(mockConfigManager.initialize()).once(); - }); - - test('should not throw when activate is called', () => { - when(mockConfigManager.initialize()).thenResolve(); - - assert.doesNotThrow(() => { - activationService.activate(); - }); - }); - }); - - suite('constructor', () => { - test('should create service with dependencies', () => { - assert.ok(activationService); - }); - - test('should accept dependencies', () => { - const service = new DeepnoteEnvironmentsActivationService( - instance(mockConfigManager), - instance(mockOutputChannel), - instance(mockEnvironmentsView) - ); - - assert.ok(service); - }); - }); -}); diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts deleted file mode 100644 index 81116ade90..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ /dev/null @@ -1,680 +0,0 @@ -import { inject, injectable } from 'inversify'; -import { commands, Disposable, l10n, ProgressLocation, QuickPickItem, TreeView, window, workspace } from 'vscode'; -import { IDisposableRegistry } from '../../../platform/common/types'; -import { logger } from '../../../platform/logging'; -import { IPythonApiProvider } from '../../../platform/api/types'; -import { - DeepnoteKernelConnectionMetadata, - IDeepnoteEnvironmentManager, - IDeepnoteKernelAutoSelector, - IDeepnoteNotebookEnvironmentMapper -} from '../types'; -import { DeepnoteEnvironmentTreeDataProvider } from './deepnoteEnvironmentTreeDataProvider.node'; -import { DeepnoteEnvironmentTreeItem } from './deepnoteEnvironmentTreeItem.node'; -import { CreateDeepnoteEnvironmentOptions, DeepnoteEnvironment, EnvironmentStatus } from './deepnoteEnvironment'; -import { - getCachedEnvironment, - resolvedPythonEnvToJupyterEnv, - getPythonEnvironmentName -} from '../../../platform/interpreter/helpers'; -import { getDisplayPath } from '../../../platform/common/platform/fs-paths'; -import { IKernelProvider } from '../../types'; -import { getDeepnoteEnvironmentStatusVisual } from './deepnoteEnvironmentUi'; - -/** - * View controller for the Deepnote kernel environments tree view. - * Manages the tree view and handles all environment-related commands. - */ -@injectable() -export class DeepnoteEnvironmentsView implements Disposable { - private readonly treeView: TreeView; - private readonly disposables: Disposable[] = []; - - constructor( - @inject(IDeepnoteEnvironmentManager) private readonly environmentManager: IDeepnoteEnvironmentManager, - @inject(DeepnoteEnvironmentTreeDataProvider) - private readonly treeDataProvider: DeepnoteEnvironmentTreeDataProvider, - @inject(IPythonApiProvider) private readonly pythonApiProvider: IPythonApiProvider, - @inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry, - @inject(IDeepnoteKernelAutoSelector) private readonly kernelAutoSelector: IDeepnoteKernelAutoSelector, - @inject(IDeepnoteNotebookEnvironmentMapper) - private readonly notebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper, - @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider - ) { - // Create tree data provider - - // Create tree view - this.treeView = window.createTreeView('deepnoteEnvironments', { - treeDataProvider: this.treeDataProvider, - showCollapseAll: true - }); - - this.disposables.push(this.treeView); - - // Register commands - this.registerCommands(); - - // Register for disposal - disposableRegistry.push(this); - } - - public async createEnvironmentCommand(): Promise { - try { - // Step 1: Select Python interpreter - const api = await this.pythonApiProvider.getNewApi(); - if (!api || !api.environments.known || api.environments.known.length === 0) { - void window.showErrorMessage(l10n.t('No Python interpreters found. Please install Python first.')); - return; - } - - const interpreterItems = api.environments.known - .map((env) => { - const interpreter = resolvedPythonEnvToJupyterEnv(getCachedEnvironment(env)); - if (!interpreter) { - return undefined; - } - return { - label: getPythonEnvironmentName(interpreter) || getDisplayPath(interpreter.uri), - description: getDisplayPath(interpreter.uri), - interpreter - }; - }) - .filter( - ( - item - ): item is { - label: string; - description: string; - interpreter: import('../../../platform/pythonEnvironments/info').PythonEnvironment; - } => item !== undefined - ); - - const selectedInterpreter = await window.showQuickPick(interpreterItems, { - placeHolder: l10n.t('Select a Python interpreter for this environment'), - matchOnDescription: true - }); - - if (!selectedInterpreter) { - return; - } - - // Step 2: Enter environment name - const name = await window.showInputBox({ - prompt: l10n.t('Enter a name for this environment'), - placeHolder: l10n.t('e.g., Python 3.11 (Data Science)'), - validateInput: (value: string) => { - if (!value || value.trim().length === 0) { - return l10n.t('Name cannot be empty'); - } - return undefined; - } - }); - - if (!name) { - return; - } - - // Check if name is already in use - const existingEnvironments = this.environmentManager.listEnvironments(); - if (existingEnvironments.some((env) => env.name === name)) { - void window.showErrorMessage(l10n.t('An environment with this name already exists')); - return; - } - - // Step 3: Enter packages (optional) - const packagesInput = await window.showInputBox({ - prompt: l10n.t('Enter additional packages to install (comma-separated, optional)'), - placeHolder: l10n.t('e.g., matplotlib, tensorflow'), - validateInput: (value: string) => { - if (!value || value.trim().length === 0) { - return undefined; // Empty is OK - } - // Basic validation: check for valid package names - const packages = value.split(',').map((p: string) => p.trim()); - for (const pkg of packages) { - const isValid = - /^[A-Za-z0-9._\-]+(\[[A-Za-z0-9_,.\-]+\])?(\s*(==|>=|<=|~=|>|<)\s*[A-Za-z0-9.*+!\-_.]+)?(?:\s*;.+)?$/.test( - pkg - ); - if (!isValid) { - return l10n.t('Invalid package name: {0}', pkg); - } - } - return undefined; - } - }); - - // Parse packages - const packages = - packagesInput && packagesInput.trim().length > 0 - ? packagesInput - .split(',') - .map((p: string) => p.trim()) - .filter((p: string) => p.length > 0) - : undefined; - - // Step 4: Enter description (optional) - const description = await window.showInputBox({ - prompt: l10n.t('Enter a description for this environment (optional)'), - placeHolder: l10n.t('e.g., Environment for data science projects') - }); - - // Create environment with progress - return await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Creating environment "{0}"...', name), - cancellable: true - }, - async (progress: { report: (value: { message?: string; increment?: number }) => void }, token) => { - progress.report({ message: l10n.t('Setting up virtual environment...') }); - - const options: CreateDeepnoteEnvironmentOptions = { - name: name.trim(), - pythonInterpreter: selectedInterpreter.interpreter, - packages, - description: description?.trim() - }; - - try { - const config = await this.environmentManager.createEnvironment(options, token); - logger.info(`Created environment: ${config.id} (${config.name})`); - - void window.showInformationMessage( - l10n.t('Environment "{0}" created successfully!', config.name) - ); - - return config; - } catch (error) { - logger.error('Failed to create environment', error); - throw error; - } - } - ); - } catch (error) { - void window.showErrorMessage(l10n.t('Failed to create environment. See output for details.')); - } - } - - private registerCommands(): void { - // Refresh command - this.disposables.push( - commands.registerCommand('deepnote.environments.refresh', () => { - this.treeDataProvider.refresh(); - }) - ); - - // Create environment command - this.disposables.push( - commands.registerCommand('deepnote.environments.create', async () => { - await this.createEnvironmentCommand(); - }) - ); - - // Start server command - this.disposables.push( - commands.registerCommand('deepnote.environments.start', async (item: DeepnoteEnvironmentTreeItem) => { - if (item?.environment) { - await this.startServer(item.environment.id); - } - }) - ); - - // Stop server command - this.disposables.push( - commands.registerCommand('deepnote.environments.stop', async (item: DeepnoteEnvironmentTreeItem) => { - if (item?.environment) { - await this.stopServer(item.environment.id); - } - }) - ); - - // Restart server command - this.disposables.push( - commands.registerCommand('deepnote.environments.restart', async (item: DeepnoteEnvironmentTreeItem) => { - if (item?.environment) { - await this.restartServer(item.environment.id); - } - }) - ); - - // Delete environment command - this.disposables.push( - commands.registerCommand('deepnote.environments.delete', async (item: DeepnoteEnvironmentTreeItem) => { - if (item?.environment) { - await this.deleteEnvironmentCommand(item.environment.id); - } - }) - ); - - // Edit name command - this.disposables.push( - commands.registerCommand('deepnote.environments.editName', async (item: DeepnoteEnvironmentTreeItem) => { - if (item?.environment) { - await this.editEnvironmentName(item.environment.id); - } - }) - ); - - // Manage packages command - this.disposables.push( - commands.registerCommand( - 'deepnote.environments.managePackages', - async (item: DeepnoteEnvironmentTreeItem) => { - if (item?.environment) { - await this.managePackages(item.environment.id); - } - } - ) - ); - - // Switch environment for notebook command - this.disposables.push( - commands.registerCommand('deepnote.environments.selectForNotebook', async () => { - await this.selectEnvironmentForNotebook(); - }) - ); - } - - public async deleteEnvironmentCommand(environmentId: string): Promise { - const config = this.environmentManager.getEnvironment(environmentId); - if (!config) { - return; - } - - // Confirm deletion - const confirmation = await window.showWarningMessage( - l10n.t( - 'Are you sure you want to delete "{0}"? This will remove the virtual environment and cannot be undone.', - config.name - ), - { modal: true }, - l10n.t('Delete') - ); - - if (confirmation !== l10n.t('Delete')) { - return; - } - - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Deleting environment "{0}"...', config.name), - cancellable: true - }, - async (_progress, token) => { - // Clean up notebook mappings referencing this env - const notebooks = this.notebookEnvironmentMapper.getNotebooksUsingEnvironment(environmentId); - for (const nb of notebooks) { - await this.notebookEnvironmentMapper.removeEnvironmentForNotebook(nb); - } - - // Dispose kernels from any open notebooks using this environment - await this.disposeKernelsUsingEnvironment(environmentId); - - await this.environmentManager.deleteEnvironment(environmentId, token); - logger.info(`Deleted environment: ${environmentId}`); - } - ); - - void window.showInformationMessage(l10n.t('Environment "{0}" deleted', config.name)); - } catch (error) { - logger.error('Failed to delete environment', error); - void window.showErrorMessage(l10n.t('Failed to delete environment. See output for details.')); - } - } - - /** - * Dispose kernels from any open notebooks that are using the specified environment. - * This ensures the UI reflects that the kernel is no longer available. - */ - private async disposeKernelsUsingEnvironment(environmentId: string): Promise { - const openNotebooks = workspace.notebookDocuments; - - for (const notebook of openNotebooks) { - // Only check Deepnote notebooks - if (notebook.notebookType !== 'deepnote') { - continue; - } - - // Get the kernel for this notebook - const kernel = this.kernelProvider.get(notebook); - if (!kernel) { - continue; - } - - // Check if this kernel is using the environment being deleted - const connectionMetadata = kernel.kernelConnectionMetadata; - if (connectionMetadata.kind === 'startUsingDeepnoteKernel') { - const deepnoteMetadata = connectionMetadata as DeepnoteKernelConnectionMetadata; - const expectedHandle = `deepnote-config-server-${environmentId}`; - - if (deepnoteMetadata.serverProviderHandle.handle === expectedHandle) { - logger.info( - `Disposing kernel for notebook ${getDisplayPath( - notebook.uri - )} as it uses deleted environment ${environmentId}` - ); - - try { - // First, unselect the controller from the notebook UI - this.kernelAutoSelector.clearControllerForEnvironment(notebook, environmentId); - - // Then dispose the kernel - await kernel.dispose(); - } catch (error) { - logger.error(`Failed to dispose kernel for ${getDisplayPath(notebook.uri)}`, error); - } - } - } - } - } - - public async selectEnvironmentForNotebook(): Promise { - // Get the active notebook - const activeNotebook = window.activeNotebookEditor?.notebook; - if (!activeNotebook || activeNotebook.notebookType !== 'deepnote') { - void window.showWarningMessage(l10n.t('No active Deepnote notebook found')); - return; - } - - // Get base file URI (without query/fragment) - const baseFileUri = activeNotebook.uri.with({ query: '', fragment: '' }); - - // Get current environment selection - const currentEnvironmentId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri); - const currentEnvironment = currentEnvironmentId - ? this.environmentManager.getEnvironment(currentEnvironmentId) - : undefined; - - // Get all environments - const environments = this.environmentManager.listEnvironments(); - - // Build quick pick items - const items: (QuickPickItem & { environmentId?: string })[] = environments.map((env) => { - const envWithStatus = this.environmentManager.getEnvironmentWithStatus(env.id); - - const { icon, text } = getDeepnoteEnvironmentStatusVisual( - envWithStatus?.status ?? EnvironmentStatus.Stopped - ); - - const isCurrent = currentEnvironment?.id === env.id; - - return { - label: `$(${icon}) ${env.name} [${text}]${isCurrent ? ' $(check)' : ''}`, - description: getDisplayPath(env.pythonInterpreter.uri), - detail: env.packages?.length - ? l10n.t('Packages: {0}', env.packages.join(', ')) - : l10n.t('No additional packages'), - environmentId: env.id - }; - }); - - const createNewLabel = l10n.t('$(add) Create New Environment'); - - // Add "Create new" option at the end - items.push({ - label: createNewLabel, - description: l10n.t('Set up a new kernel environment'), - alwaysShow: true - }); - - const selected = await window.showQuickPick(items, { - placeHolder: l10n.t('Select an environment for this notebook'), - matchOnDescription: true, - matchOnDetail: true - }); - - if (!selected) { - return; // User cancelled - } - - let selectedEnvironmentId: string | undefined; - - if (selected.label === createNewLabel) { - const newEnvironment = await this.createEnvironmentCommand(); - if (newEnvironment == null) { - return; - } - // return; - selectedEnvironmentId = newEnvironment.id; - } else { - selectedEnvironmentId = selected.environmentId; - } - - // Check if user selected the same environment - if (selectedEnvironmentId === currentEnvironmentId) { - logger.info(`User selected the same environment - no changes needed`); - return; - } else if (selectedEnvironmentId == null) { - logger.info('User cancelled environment selection'); - return; - } - - // Check if any cells are currently executing using the kernel execution state - // This is more reliable than checking executionSummary - const kernel = this.kernelProvider.get(activeNotebook); - const hasExecutingCells = kernel - ? this.kernelProvider.getKernelExecution(kernel).pendingCells.length > 0 - : false; - - if (hasExecutingCells) { - const proceed = await window.showWarningMessage( - l10n.t( - 'Some cells are currently executing. Switching environments now may cause errors. Do you want to continue?' - ), - { modal: true }, - l10n.t('Yes, Switch Anyway'), - l10n.t('Cancel') - ); - - if (proceed !== l10n.t('Yes, Switch Anyway')) { - logger.info('User cancelled environment switch due to executing cells'); - return; - } - } - - // User selected a different environment - switch to it - logger.info(`Switching notebook ${getDisplayPath(activeNotebook.uri)} to environment ${selectedEnvironmentId}`); - - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Switching to environment...'), - cancellable: false - }, - async () => { - // Update the notebook-to-environment mapping - await this.notebookEnvironmentMapper.setEnvironmentForNotebook(baseFileUri, selectedEnvironmentId); - - // Force rebuild the controller with the new environment - // This clears cached metadata and creates a fresh controller. - await this.kernelAutoSelector.rebuildController(activeNotebook); - - logger.info(`Successfully switched to environment ${selectedEnvironmentId}`); - } - ); - - void window.showInformationMessage(l10n.t('Environment switched successfully')); - } catch (error) { - logger.error('Failed to switch environment', error); - void window.showErrorMessage(l10n.t('Failed to switch environment. See output for details.')); - } - } - - private async startServer(environmentId: string): Promise { - const config = this.environmentManager.getEnvironment(environmentId); - if (!config) { - return; - } - - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Starting server for "{0}"...', config.name), - cancellable: true - }, - async (_progress, token) => { - await this.environmentManager.startServer(environmentId, token); - logger.info(`Started server for environment: ${environmentId}`); - } - ); - - void window.showInformationMessage(l10n.t('Server started for "{0}"', config.name)); - } catch (error) { - logger.error('Failed to start server', error); - void window.showErrorMessage(l10n.t('Failed to start server. See output for details.')); - } - } - - private async stopServer(environmentId: string): Promise { - const config = this.environmentManager.getEnvironment(environmentId); - if (!config) { - return; - } - - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Stopping server for "{0}"...', config.name), - cancellable: true - }, - async (_progress, token) => { - await this.environmentManager.stopServer(environmentId, token); - logger.info(`Stopped server for environment: ${environmentId}`); - } - ); - - void window.showInformationMessage(l10n.t('Server stopped for "{0}"', config.name)); - } catch (error) { - logger.error('Failed to stop server', error); - void window.showErrorMessage(l10n.t('Failed to stop server. See output for details.')); - } - } - - private async restartServer(environmentId: string): Promise { - const config = this.environmentManager.getEnvironment(environmentId); - if (!config) { - return; - } - - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Restarting server for "{0}"...', config.name), - cancellable: true - }, - async (_progress, token) => { - await this.environmentManager.restartServer(environmentId, token); - logger.info(`Restarted server for environment: ${environmentId}`); - } - ); - - void window.showInformationMessage(l10n.t('Server restarted for "{0}"', config.name)); - } catch (error) { - logger.error('Failed to restart server', error); - void window.showErrorMessage(l10n.t('Failed to restart server. See output for details.')); - } - } - - public async editEnvironmentName(environmentId: string): Promise { - const config = this.environmentManager.getEnvironment(environmentId); - if (!config) { - return; - } - - const newName = await window.showInputBox({ - prompt: l10n.t('Enter a new name for this environment'), - value: config.name, - validateInput: (value: string) => { - if (!value || value.trim().length === 0) { - return l10n.t('Name cannot be empty'); - } - return undefined; - } - }); - - if (!newName || newName === config.name) { - return; - } - - try { - await this.environmentManager.updateEnvironment(environmentId, { - name: newName.trim() - }); - - logger.info(`Renamed environment ${environmentId} to "${newName}"`); - void window.showInformationMessage(l10n.t('Environment renamed to "{0}"', newName)); - } catch (error) { - logger.error('Failed to rename environment', error); - void window.showErrorMessage(l10n.t('Failed to rename environment. See output for details.')); - } - } - - private async managePackages(environmentId: string): Promise { - const config = this.environmentManager.getEnvironment(environmentId); - if (!config) { - return; - } - - // Show input box for package names - const packagesInput = await window.showInputBox({ - prompt: l10n.t('Enter packages to install (comma-separated)'), - placeHolder: l10n.t('e.g., pandas, numpy, matplotlib'), - value: config.packages?.join(', ') || '', - validateInput: (value: string) => { - if (!value || value.trim().length === 0) { - return l10n.t('Please enter at least one package'); - } - const packages = value.split(',').map((p: string) => p.trim()); - for (const pkg of packages) { - const isValid = - /^[A-Za-z0-9._\-]+(\[[A-Za-z0-9_,.\-]+\])?(\s*(==|>=|<=|~=|>|<)\s*[A-Za-z0-9.*+!\-_.]+)?(?:\s*;.+)?$/.test( - pkg - ); - if (!isValid) { - return l10n.t('Invalid package name: {0}', pkg); - } - } - return undefined; - } - }); - - if (!packagesInput) { - return; - } - - const packages = packagesInput - .split(',') - .map((p: string) => p.trim()) - .filter((p: string) => p.length > 0); - - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Updating packages for "{0}"...', config.name), - cancellable: false - }, - async () => { - await this.environmentManager.updateEnvironment(environmentId, { packages }); - logger.info(`Updated packages for environment ${environmentId}`); - } - ); - - void window.showInformationMessage(l10n.t('Packages updated for "{0}"', config.name)); - } catch (error) { - logger.error('Failed to update packages', error); - void window.showErrorMessage(l10n.t('Failed to update packages. See output for details.')); - } - } - - public dispose(): void { - this.disposables.forEach((d) => d?.dispose()); - } -} diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts deleted file mode 100644 index 3331e599a0..0000000000 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts +++ /dev/null @@ -1,809 +0,0 @@ -import { assert } from 'chai'; -import * as sinon from 'sinon'; -import { anything, capture, instance, mock, when, verify, deepEqual, resetCalls } from 'ts-mockito'; -import { CancellationToken, Disposable, ProgressOptions, Uri } from 'vscode'; -import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node'; -import { IDeepnoteEnvironmentManager, IDeepnoteKernelAutoSelector, IDeepnoteNotebookEnvironmentMapper } from '../types'; -import { IPythonApiProvider } from '../../../platform/api/types'; -import { IDisposableRegistry } from '../../../platform/common/types'; -import { IKernelProvider } from '../../../kernels/types'; -import { DeepnoteEnvironment, EnvironmentStatus } from './deepnoteEnvironment'; -import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; -import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; -import { DeepnoteEnvironmentTreeDataProvider } from './deepnoteEnvironmentTreeDataProvider.node'; -import * as interpreterHelpers from '../../../platform/interpreter/helpers'; - -suite('DeepnoteEnvironmentsView', () => { - let view: DeepnoteEnvironmentsView; - let mockConfigManager: IDeepnoteEnvironmentManager; - let mockTreeDataProvider: DeepnoteEnvironmentTreeDataProvider; - let mockPythonApiProvider: IPythonApiProvider; - let mockDisposableRegistry: IDisposableRegistry; - let mockKernelAutoSelector: IDeepnoteKernelAutoSelector; - let mockNotebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper; - let mockKernelProvider: IKernelProvider; - let disposables: Disposable[] = []; - - setup(() => { - resetVSCodeMocks(); - disposables.push(new Disposable(() => resetVSCodeMocks())); - - mockConfigManager = mock(); - mockTreeDataProvider = mock(); - mockPythonApiProvider = mock(); - mockDisposableRegistry = mock(); - mockKernelAutoSelector = mock(); - mockNotebookEnvironmentMapper = mock(); - mockKernelProvider = mock(); - - // Mock onDidChangeEnvironments to return a disposable event - when(mockConfigManager.onDidChangeEnvironments).thenReturn((_listener: () => void) => { - return { - dispose: () => { - /* noop */ - } - }; - }); - - view = new DeepnoteEnvironmentsView( - instance(mockConfigManager), - instance(mockTreeDataProvider), - instance(mockPythonApiProvider), - instance(mockDisposableRegistry), - instance(mockKernelAutoSelector), - instance(mockNotebookEnvironmentMapper), - instance(mockKernelProvider) - ); - }); - - teardown(() => { - if (view) { - view.dispose(); - } - disposables.forEach((d) => d.dispose()); - disposables = []; - }); - - suite('constructor', () => { - test('should create tree view', () => { - // View should be created without errors - assert.ok(view); - }); - - test('should register with disposable registry', () => { - verify(mockDisposableRegistry.push(anything())).atLeast(1); - }); - }); - - suite('dispose', () => { - test('should dispose all resources', () => { - view.dispose(); - // Should not throw - }); - - test('should dispose tree view', () => { - view.dispose(); - // Tree view should be disposed - // In a real test, we would verify the tree view's dispose was called - }); - }); - - suite('editEnvironmentName', () => { - const testEnvironmentId = 'test-env-id'; - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const testEnvironment: DeepnoteEnvironment = { - id: testEnvironmentId, - name: 'Original Name', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date(), - lastUsedAt: new Date() - }; - - setup(() => { - // Reset mocks between tests - resetCalls(mockConfigManager); - resetCalls(mockedVSCodeNamespaces.window); - }); - - test('should return early if environment not found', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(undefined); - - await view.editEnvironmentName(testEnvironmentId); - - // Should not call showInputBox or updateEnvironment - verify(mockedVSCodeNamespaces.window.showInputBox(anything())).never(); - verify(mockConfigManager.updateEnvironment(anything(), anything())).never(); - }); - - test('should return early if user cancels input', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn(Promise.resolve(undefined)); - - await view.editEnvironmentName(testEnvironmentId); - - verify(mockedVSCodeNamespaces.window.showInputBox(anything())).once(); - verify(mockConfigManager.updateEnvironment(anything(), anything())).never(); - }); - - test('should return early if user provides same name', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn(Promise.resolve('Original Name')); - - await view.editEnvironmentName(testEnvironmentId); - - verify(mockedVSCodeNamespaces.window.showInputBox(anything())).once(); - verify(mockConfigManager.updateEnvironment(anything(), anything())).never(); - }); - - test('should validate that name cannot be empty', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - - // Capture the validator function - let validatorFn: ((value: string) => string | undefined) | undefined; - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenCall((options) => { - validatorFn = options.validateInput; - return Promise.resolve(undefined); - }); - - await view.editEnvironmentName(testEnvironmentId); - - assert.ok(validatorFn, 'Validator function should be provided'); - assert.strictEqual(validatorFn!(''), 'Name cannot be empty'); - assert.strictEqual(validatorFn!(' '), 'Name cannot be empty'); - assert.strictEqual(validatorFn!('Valid Name'), undefined); - }); - - test('should successfully rename environment with trimmed name', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn(Promise.resolve(' New Name ')); - when(mockConfigManager.updateEnvironment(anything(), anything())).thenResolve(); - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(); - - await view.editEnvironmentName(testEnvironmentId); - - verify(mockConfigManager.updateEnvironment(testEnvironmentId, deepEqual({ name: 'New Name' }))).once(); - verify(mockedVSCodeNamespaces.window.showInformationMessage(anything())).once(); - }); - - test('should show error message if update fails', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn(Promise.resolve('New Name')); - when(mockConfigManager.updateEnvironment(anything(), anything())).thenReject(new Error('Update failed')); - when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenResolve(); - - await view.editEnvironmentName(testEnvironmentId); - - verify(mockConfigManager.updateEnvironment(anything(), anything())).once(); - verify(mockedVSCodeNamespaces.window.showErrorMessage(anything())).once(); - }); - - test('should call updateEnvironment with correct parameters', async () => { - const newName = 'Updated Environment Name'; - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn(Promise.resolve(newName)); - when(mockConfigManager.updateEnvironment(anything(), anything())).thenResolve(); - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(); - - await view.editEnvironmentName(testEnvironmentId); - - verify(mockConfigManager.updateEnvironment(testEnvironmentId, deepEqual({ name: newName }))).once(); - }); - - test('should preserve existing environment configuration except name', async () => { - const envWithPackages: DeepnoteEnvironment = { - ...testEnvironment, - packages: ['numpy', 'pandas'], - description: 'Test description' - }; - - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(envWithPackages); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn(Promise.resolve('New Name')); - when(mockConfigManager.updateEnvironment(anything(), anything())).thenResolve(); - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(); - - await view.editEnvironmentName(testEnvironmentId); - - // Should only update the name, not other properties - verify(mockConfigManager.updateEnvironment(testEnvironmentId, deepEqual({ name: 'New Name' }))).once(); - }); - - test('should show input box with current name as default value', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - - let capturedOptions: any; - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenCall((options) => { - capturedOptions = options; - return Promise.resolve(undefined); - }); - - await view.editEnvironmentName(testEnvironmentId); - - assert.ok(capturedOptions, 'Options should be provided'); - assert.strictEqual(capturedOptions.value, 'Original Name'); - }); - }); - - suite('createEnvironmentCommand', () => { - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3.11'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const createdEnvironment: DeepnoteEnvironment = { - id: 'new-env-id', - name: 'My Data Science Environment', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/new/venv'), - packages: ['pandas', 'numpy', 'matplotlib'], - description: 'Environment for data science work', - createdAt: new Date(), - lastUsedAt: new Date() - }; - - let getCachedEnvironmentStub: sinon.SinonStub; - let resolvedPythonEnvToJupyterEnvStub: sinon.SinonStub; - let getPythonEnvironmentNameStub: sinon.SinonStub; - - setup(() => { - resetCalls(mockConfigManager); - resetCalls(mockPythonApiProvider); - resetCalls(mockedVSCodeNamespaces.window); - - // Stub the helper functions - getCachedEnvironmentStub = sinon.stub(interpreterHelpers, 'getCachedEnvironment'); - resolvedPythonEnvToJupyterEnvStub = sinon.stub(interpreterHelpers, 'resolvedPythonEnvToJupyterEnv'); - getPythonEnvironmentNameStub = sinon.stub(interpreterHelpers, 'getPythonEnvironmentName'); - }); - - teardown(() => { - getCachedEnvironmentStub?.restore(); - resolvedPythonEnvToJupyterEnvStub?.restore(); - getPythonEnvironmentNameStub?.restore(); - }); - - test('should successfully create environment with all inputs', async () => { - // Mock Python API to return available interpreters - const mockResolvedEnvironment = { - id: testInterpreter.id, - path: testInterpreter.uri.fsPath, - version: { - major: 3, - minor: 11, - micro: 0 - } - }; - const mockPythonApi = { - environments: { - known: [mockResolvedEnvironment] - } - }; - when(mockPythonApiProvider.getNewApi()).thenResolve(mockPythonApi as any); - - // Stub helper functions to return the test interpreter - getCachedEnvironmentStub.returns(testInterpreter); - resolvedPythonEnvToJupyterEnvStub.returns(testInterpreter); - getPythonEnvironmentNameStub.returns('Python 3.11'); - - // Mock interpreter selection - when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenCall((items: any[]) => { - return Promise.resolve(items[0]); - }); - - // Mock input boxes for name, packages, and description - let inputBoxCallCount = 0; - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenCall(() => { - inputBoxCallCount++; - if (inputBoxCallCount === 1) { - // First call: environment name - return Promise.resolve('My Data Science Environment'); - } else if (inputBoxCallCount === 2) { - // Second call: packages - return Promise.resolve('pandas, numpy, matplotlib'); - } else { - // Third call: description - return Promise.resolve('Environment for data science work'); - } - }); - - // Mock list environments to return empty (no duplicates) - when(mockConfigManager.listEnvironments()).thenReturn([]); - - // Mock window.withProgress to execute the callback - when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( - (_options: ProgressOptions, callback: Function) => { - const mockProgress = { - report: (_value: { message?: string; increment?: number }) => { - // Mock progress reporting - } - }; - const mockToken = { - isCancellationRequested: false, - onCancellationRequested: (_listener: any) => { - return { - dispose: () => { - // Mock disposable - } - }; - } - }; - return callback(mockProgress, mockToken); - } - ); - - // Mock environment creation - when(mockConfigManager.createEnvironment(anything(), anything())).thenResolve(createdEnvironment); - - // Mock success message - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined); - - // Execute the command - await view.createEnvironmentCommand(); - - // Verify API calls - verify(mockPythonApiProvider.getNewApi()).once(); - verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).once(); - verify(mockedVSCodeNamespaces.window.showInputBox(anything())).times(3); - verify(mockConfigManager.listEnvironments()).once(); - verify(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).once(); - - // Verify createEnvironment was called with correct options - verify(mockConfigManager.createEnvironment(anything(), anything())).once(); - const [capturedOptions, capturedToken] = capture(mockConfigManager.createEnvironment).last(); - assert.strictEqual(capturedOptions.name, 'My Data Science Environment'); - assert.deepStrictEqual(capturedOptions.packages, ['pandas', 'numpy', 'matplotlib']); - assert.strictEqual(capturedOptions.description, 'Environment for data science work'); - assert.strictEqual(capturedOptions.pythonInterpreter.id, testInterpreter.id); - assert.ok(capturedToken, 'Cancellation token should be provided'); - - // Verify success message was shown - verify(mockedVSCodeNamespaces.window.showInformationMessage(anything())).once(); - }); - }); - - suite('deleteEnvironmentCommand', () => { - const testEnvironmentId = 'test-env-id-to-delete'; - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3.11'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const testEnvironment: DeepnoteEnvironment = { - id: testEnvironmentId, - name: 'Environment to Delete', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date(), - lastUsedAt: new Date() - }; - - setup(() => { - resetCalls(mockConfigManager); - resetCalls(mockNotebookEnvironmentMapper); - resetCalls(mockedVSCodeNamespaces.window); - }); - - test('should successfully delete environment with notebooks using it', async () => { - // Mock environment exists - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - - // Mock user confirmation - user clicks "Delete" button - when(mockedVSCodeNamespaces.window.showWarningMessage(anything(), anything(), anything())).thenReturn( - Promise.resolve('Delete') - ); - - // Mock notebooks using this environment - const notebook1Uri = Uri.file('/workspace/notebook1.deepnote'); - const notebook2Uri = Uri.file('/workspace/notebook2.deepnote'); - when(mockNotebookEnvironmentMapper.getNotebooksUsingEnvironment(testEnvironmentId)).thenReturn([ - notebook1Uri, - notebook2Uri - ]); - - // Mock removing environment mappings - when(mockNotebookEnvironmentMapper.removeEnvironmentForNotebook(anything())).thenResolve(); - - // Mock window.withProgress to execute the callback - when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( - (_options: ProgressOptions, callback: Function) => { - const mockProgress = { - report: (_value: { message?: string; increment?: number }) => { - // Mock progress reporting - } - }; - const mockToken: CancellationToken = { - isCancellationRequested: false, - onCancellationRequested: (_listener: any) => { - return { - dispose: () => { - // Mock disposable - } - }; - } - }; - return callback(mockProgress, mockToken); - } - ); - - // Mock environment deletion - when(mockConfigManager.deleteEnvironment(testEnvironmentId, anything())).thenResolve(); - - // Mock success message - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined); - - // Execute the command - await view.deleteEnvironmentCommand(testEnvironmentId); - - // Verify API calls - verify(mockConfigManager.getEnvironment(testEnvironmentId)).once(); - verify(mockedVSCodeNamespaces.window.showWarningMessage(anything(), anything(), anything())).once(); - verify(mockNotebookEnvironmentMapper.getNotebooksUsingEnvironment(testEnvironmentId)).once(); - - // Verify environment mappings were removed for both notebooks - verify(mockNotebookEnvironmentMapper.removeEnvironmentForNotebook(notebook1Uri)).once(); - verify(mockNotebookEnvironmentMapper.removeEnvironmentForNotebook(notebook2Uri)).once(); - - // Verify environment deletion - verify(mockConfigManager.deleteEnvironment(testEnvironmentId, anything())).once(); - - // Verify success message was shown - verify(mockedVSCodeNamespaces.window.showInformationMessage(anything())).once(); - }); - - test('should dispose kernels from open notebooks using the deleted environment', async () => { - // Mock environment exists - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - - // Mock user confirmation - when(mockedVSCodeNamespaces.window.showWarningMessage(anything(), anything(), anything())).thenReturn( - Promise.resolve('Delete') - ); - - // Mock notebooks using this environment - when(mockNotebookEnvironmentMapper.getNotebooksUsingEnvironment(testEnvironmentId)).thenReturn([]); - when(mockNotebookEnvironmentMapper.removeEnvironmentForNotebook(anything())).thenResolve(); - - // Mock open notebooks with kernels - const openNotebook1 = { - uri: Uri.file('/workspace/open-notebook1.deepnote'), - notebookType: 'deepnote', - isClosed: false - } as any; - - const openNotebook2 = { - uri: Uri.file('/workspace/open-notebook2.deepnote'), - notebookType: 'jupyter-notebook', - isClosed: false - } as any; - - const openNotebook3 = { - uri: Uri.file('/workspace/open-notebook3.deepnote'), - notebookType: 'deepnote', - isClosed: false - } as any; - - // Mock workspace.notebookDocuments - when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([ - openNotebook1, - openNotebook2, - openNotebook3 - ]); - - // Mock kernels - const mockKernel1 = { - kernelConnectionMetadata: { - kind: 'startUsingDeepnoteKernel', - serverProviderHandle: { - handle: `deepnote-config-server-${testEnvironmentId}` - } - }, - dispose: sinon.stub().resolves() - }; - - const mockKernel3 = { - kernelConnectionMetadata: { - kind: 'startUsingDeepnoteKernel', - serverProviderHandle: { - handle: 'deepnote-config-server-different-env' - } - }, - dispose: sinon.stub().resolves() - }; - - // Mock kernelProvider.get() - when(mockKernelProvider.get(openNotebook1)).thenReturn(mockKernel1 as any); - when(mockKernelProvider.get(openNotebook2)).thenReturn(undefined); // No kernel for jupyter notebook - when(mockKernelProvider.get(openNotebook3)).thenReturn(mockKernel3 as any); - - // Mock window.withProgress - when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( - (_options: ProgressOptions, callback: Function) => { - const mockProgress = { - report: () => { - // Mock progress reporting - } - }; - const mockToken: CancellationToken = { - isCancellationRequested: false, - onCancellationRequested: () => ({ - dispose: () => { - // Mock disposable - } - }) - }; - return callback(mockProgress, mockToken); - } - ); - - // Mock environment deletion - when(mockConfigManager.deleteEnvironment(testEnvironmentId, anything())).thenResolve(); - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined); - - // Execute the command - await view.deleteEnvironmentCommand(testEnvironmentId); - - // Verify that only kernel1 (using the deleted environment) was disposed - assert.strictEqual(mockKernel1.dispose.callCount, 1, 'Kernel using deleted environment should be disposed'); - assert.strictEqual( - mockKernel3.dispose.callCount, - 0, - 'Kernel using different environment should not be disposed' - ); - }); - }); - - suite('selectEnvironmentForNotebook', () => { - const testInterpreter1: PythonEnvironment = { - id: 'python-1', - uri: Uri.file('/usr/bin/python3.11'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const testInterpreter2: PythonEnvironment = { - id: 'python-2', - uri: Uri.file('/usr/bin/python3.12'), - version: { major: 3, minor: 12, patch: 0, raw: '3.12.0' } - } as PythonEnvironment; - - const currentEnvironment: DeepnoteEnvironment = { - id: 'current-env-id', - name: 'Current Environment', - pythonInterpreter: testInterpreter1, - venvPath: Uri.file('/path/to/current/venv'), - createdAt: new Date(), - lastUsedAt: new Date() - }; - - const newEnvironment: DeepnoteEnvironment = { - id: 'new-env-id', - name: 'New Environment', - pythonInterpreter: testInterpreter2, - venvPath: Uri.file('/path/to/new/venv'), - packages: ['pandas', 'numpy'], - createdAt: new Date(), - lastUsedAt: new Date() - }; - - setup(() => { - resetCalls(mockConfigManager); - resetCalls(mockNotebookEnvironmentMapper); - resetCalls(mockKernelAutoSelector); - resetCalls(mockKernelProvider); - resetCalls(mockedVSCodeNamespaces.window); - }); - - test('should successfully switch to a different environment', async () => { - // Mock active notebook - const notebookUri = Uri.file('/workspace/notebook.deepnote'); - const mockNotebook = { - uri: notebookUri, - notebookType: 'deepnote', - cellCount: 5 - }; - const mockNotebookEditor = { - notebook: mockNotebook - }; - - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(mockNotebookEditor as any); - - // Mock current environment mapping - const baseFileUri = notebookUri.with({ query: '', fragment: '' }); - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri)).thenReturn( - currentEnvironment.id - ); - when(mockConfigManager.getEnvironment(currentEnvironment.id)).thenReturn(currentEnvironment); - - // Mock available environments - when(mockConfigManager.listEnvironments()).thenReturn([currentEnvironment, newEnvironment]); - - // Mock environment status - when(mockConfigManager.getEnvironmentWithStatus(currentEnvironment.id)).thenReturn({ - ...currentEnvironment, - status: EnvironmentStatus.Stopped - }); - when(mockConfigManager.getEnvironmentWithStatus(newEnvironment.id)).thenReturn({ - ...newEnvironment, - status: EnvironmentStatus.Running - }); - - // Mock user selecting the new environment - when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenCall((items: any[]) => { - // Find the item for the new environment - const selectedItem = items.find((item) => item.environmentId === newEnvironment.id); - return Promise.resolve(selectedItem); - }); - - // Mock no executing cells - const mockKernel = { id: 'test-kernel' }; - const mockKernelExecution = { - pendingCells: [] - }; - when(mockKernelProvider.get(mockNotebook as any)).thenReturn(mockKernel as any); - when(mockKernelProvider.getKernelExecution(mockKernel as any)).thenReturn(mockKernelExecution as any); - - // Mock window.withProgress to execute the callback - when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( - (_options: ProgressOptions, callback: Function) => { - return callback(); - } - ); - - // Mock environment mapping update - when(mockNotebookEnvironmentMapper.setEnvironmentForNotebook(baseFileUri, newEnvironment.id)).thenResolve(); - - // Mock controller rebuild - when(mockKernelAutoSelector.rebuildController(mockNotebook as any)).thenResolve(); - - // Mock success message - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined); - - // Execute the command - await view.selectEnvironmentForNotebook(); - - // Verify API calls - verify(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri)).once(); - verify(mockConfigManager.getEnvironment(currentEnvironment.id)).once(); - verify(mockConfigManager.listEnvironments()).once(); - verify(mockConfigManager.getEnvironmentWithStatus(currentEnvironment.id)).once(); - verify(mockConfigManager.getEnvironmentWithStatus(newEnvironment.id)).once(); - verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).once(); - verify(mockKernelProvider.get(mockNotebook as any)).once(); - verify(mockKernelProvider.getKernelExecution(mockKernel as any)).once(); - - // Verify environment switch - verify(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).once(); - verify(mockNotebookEnvironmentMapper.setEnvironmentForNotebook(baseFileUri, newEnvironment.id)).once(); - verify(mockKernelAutoSelector.rebuildController(mockNotebook as any)).once(); - - // Verify success message was shown - verify(mockedVSCodeNamespaces.window.showInformationMessage(anything())).once(); - }); - }); - - suite('Server Management (startServer, stopServer, restartServer)', () => { - const testEnvironmentId = 'test-env-id'; - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const testEnvironment: DeepnoteEnvironment = { - id: testEnvironmentId, - name: 'Test Environment', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - createdAt: new Date(), - lastUsedAt: new Date() - }; - - setup(() => { - resetCalls(mockConfigManager); - resetCalls(mockedVSCodeNamespaces.window); - - // Common mocks for all server management operations - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - - when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( - (_options: ProgressOptions, callback: Function) => { - const mockProgress = { - report: () => { - // Mock progress reporting - } - }; - const mockToken: CancellationToken = { - isCancellationRequested: false, - onCancellationRequested: () => ({ - dispose: () => { - // Mock disposable - } - }) - }; - return callback(mockProgress, mockToken); - } - ); - - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined); - }); - - test('should call environmentManager.startServer', async () => { - when(mockConfigManager.startServer(testEnvironmentId, anything())).thenResolve(); - - await (view as any).startServer(testEnvironmentId); - - verify(mockConfigManager.startServer(testEnvironmentId, anything())).once(); - }); - - test('should call environmentManager.stopServer', async () => { - when(mockConfigManager.stopServer(testEnvironmentId, anything())).thenResolve(); - - await (view as any).stopServer(testEnvironmentId); - - verify(mockConfigManager.stopServer(testEnvironmentId, anything())).once(); - }); - - test('should call environmentManager.restartServer', async () => { - when(mockConfigManager.restartServer(testEnvironmentId, anything())).thenResolve(); - - await (view as any).restartServer(testEnvironmentId); - - verify(mockConfigManager.restartServer(testEnvironmentId, anything())).once(); - }); - }); - - suite('managePackages', () => { - const testEnvironmentId = 'test-env-id'; - const testInterpreter: PythonEnvironment = { - id: 'test-python-id', - uri: Uri.file('/usr/bin/python3'), - version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } - } as PythonEnvironment; - - const testEnvironment: DeepnoteEnvironment = { - id: testEnvironmentId, - name: 'Test Environment', - pythonInterpreter: testInterpreter, - venvPath: Uri.file('/path/to/venv'), - packages: ['numpy', 'pandas'], - createdAt: new Date(), - lastUsedAt: new Date() - }; - - setup(() => { - resetCalls(mockConfigManager); - resetCalls(mockedVSCodeNamespaces.window); - }); - - test('should call environmentManager.updateEnvironment with parsed packages', async () => { - when(mockConfigManager.getEnvironment(testEnvironmentId)).thenReturn(testEnvironment); - when(mockedVSCodeNamespaces.window.showInputBox(anything())).thenReturn( - Promise.resolve('matplotlib, scipy, sklearn') - ); - when(mockConfigManager.updateEnvironment(anything(), anything())).thenResolve(); - - when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( - (_options: ProgressOptions, callback: Function) => { - return callback(); - } - ); - - when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined); - - await (view as any).managePackages(testEnvironmentId); - - verify( - mockConfigManager.updateEnvironment( - testEnvironmentId, - deepEqual({ packages: ['matplotlib', 'scipy', 'sklearn'] }) - ) - ).once(); - }); - }); -}); diff --git a/src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts b/src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts deleted file mode 100644 index 77345abce7..0000000000 --- a/src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { injectable, inject } from 'inversify'; -import { Uri, Memento } from 'vscode'; -import { IExtensionContext } from '../../../platform/common/types'; -import { logger } from '../../../platform/logging'; - -/** - * Manages the mapping between notebooks and their selected environments - * Stores selections in workspace state for persistence across sessions - */ -@injectable() -export class DeepnoteNotebookEnvironmentMapper { - private static readonly STORAGE_KEY = 'deepnote.notebookEnvironmentMappings'; - private readonly workspaceState: Memento; - private mappings: Map; // notebookUri.fsPath -> environmentId - - constructor(@inject(IExtensionContext) context: IExtensionContext) { - this.workspaceState = context.workspaceState; - this.mappings = new Map(); - this.loadMappings(); - } - - /** - * Get the environment ID selected for a notebook - * @param notebookUri The notebook URI (without query/fragment) - * @returns Environment ID, or undefined if not set - */ - public getEnvironmentForNotebook(notebookUri: Uri): string | undefined { - const key = notebookUri.fsPath; - return this.mappings.get(key); - } - - /** - * Set the environment for a notebook - * @param notebookUri The notebook URI (without query/fragment) - * @param environmentId The environment ID - */ - public async setEnvironmentForNotebook(notebookUri: Uri, environmentId: string): Promise { - const key = notebookUri.fsPath; - this.mappings.set(key, environmentId); - await this.saveMappings(); - logger.info(`Mapped notebook ${notebookUri.fsPath} to environment ${environmentId}`); - } - - /** - * Remove the environment mapping for a notebook - * @param notebookUri The notebook URI (without query/fragment) - */ - public async removeEnvironmentForNotebook(notebookUri: Uri): Promise { - const key = notebookUri.fsPath; - this.mappings.delete(key); - await this.saveMappings(); - logger.info(`Removed environment mapping for notebook ${notebookUri.fsPath}`); - } - - /** - * Get all notebooks using a specific environment - * @param environmentId The environment ID - * @returns Array of notebook URIs - */ - public getNotebooksUsingEnvironment(environmentId: string): Uri[] { - const notebooks: Uri[] = []; - for (const [notebookPath, configId] of this.mappings.entries()) { - if (configId === environmentId) { - notebooks.push(Uri.file(notebookPath)); - } - } - return notebooks; - } - - /** - * Load mappings from workspace state - */ - private loadMappings(): void { - const stored = this.workspaceState.get>(DeepnoteNotebookEnvironmentMapper.STORAGE_KEY); - if (stored) { - this.mappings = new Map(Object.entries(stored)); - logger.info(`Loaded ${this.mappings.size} notebook-environment mappings`); - } - } - - /** - * Save mappings to workspace state - */ - private async saveMappings(): Promise { - const obj = Object.fromEntries(this.mappings.entries()); - await this.workspaceState.update(DeepnoteNotebookEnvironmentMapper.STORAGE_KEY, obj); - } -} diff --git a/src/kernels/deepnote/types.ts b/src/kernels/deepnote/types.ts index 307ef5f576..21c366ea08 100644 --- a/src/kernels/deepnote/types.ts +++ b/src/kernels/deepnote/types.ts @@ -7,16 +7,6 @@ import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { JupyterServerProviderHandle } from '../jupyter/types'; import { serializePythonEnvironment } from '../../platform/api/pythonApi'; import { getTelemetrySafeHashedString } from '../../platform/telemetry/helpers'; -import { - CreateDeepnoteEnvironmentOptions, - DeepnoteEnvironment, - DeepnoteEnvironmentWithStatus -} from './environments/deepnoteEnvironment'; - -export interface VenvAndToolkitInstallation { - pythonInterpreter: PythonEnvironment; - toolkitVersion: string; -} /** * Connection metadata for Deepnote Toolkit Kernels. @@ -31,7 +21,6 @@ export class DeepnoteKernelConnectionMetadata { public readonly interpreter?: PythonEnvironment; public readonly serverProviderHandle: JupyterServerProviderHandle; public readonly serverInfo?: DeepnoteServerInfo; // Store server info for connection - public readonly environmentName?: string; // Name of the Deepnote environment for display purposes private constructor(options: { interpreter?: PythonEnvironment; @@ -40,7 +29,6 @@ export class DeepnoteKernelConnectionMetadata { id: string; serverProviderHandle: JupyterServerProviderHandle; serverInfo?: DeepnoteServerInfo; - environmentName?: string; }) { this.interpreter = options.interpreter; this.kernelSpec = options.kernelSpec; @@ -48,7 +36,6 @@ export class DeepnoteKernelConnectionMetadata { this.id = options.id; this.serverProviderHandle = options.serverProviderHandle; this.serverInfo = options.serverInfo; - this.environmentName = options.environmentName; } public static create(options: { @@ -58,7 +45,6 @@ export class DeepnoteKernelConnectionMetadata { id: string; serverProviderHandle: JupyterServerProviderHandle; serverInfo?: DeepnoteServerInfo; - environmentName?: string; }) { return new DeepnoteKernelConnectionMetadata(options); } @@ -83,31 +69,18 @@ export const IDeepnoteToolkitInstaller = Symbol('IDeepnoteToolkitInstaller'); export interface IDeepnoteToolkitInstaller { /** * Ensures deepnote-toolkit is installed in a dedicated virtual environment. - * Environment-based method. * @param baseInterpreter The base Python interpreter to use for creating the venv - * @param venvPath The path where the venv should be created + * @param deepnoteFileUri The URI of the .deepnote file (used to create a unique venv per file) * @param token Cancellation token to cancel the operation - * @returns The Python interpreter from the venv and the toolkit version + * @returns The Python interpreter from the venv * @throws {DeepnoteVenvCreationError} If venv creation fails * @throws {DeepnoteToolkitInstallError} If toolkit installation fails */ - ensureVenvAndToolkit( + ensureInstalled( baseInterpreter: PythonEnvironment, - venvPath: vscode.Uri, + deepnoteFileUri: vscode.Uri, token?: vscode.CancellationToken - ): Promise; - - /** - * Install additional packages in the venv. - * @param venvPath The path to the venv - * @param packages List of package names to install - * @param token Cancellation token to cancel the operation - */ - installAdditionalPackages( - venvPath: vscode.Uri, - packages: string[], - token?: vscode.CancellationToken - ): Promise; + ): Promise; /** * Gets the venv Python interpreter if toolkit is installed, undefined otherwise. @@ -126,27 +99,23 @@ 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 or gets an existing deepnote-toolkit Jupyter server. * @param interpreter The Python interpreter to use - * @param venvPath The path to the venv - * @param environmentId The environment ID (for server management) + * @param deepnoteFileUri The URI of the .deepnote file (for server management per file) * @param token Cancellation token to cancel the operation * @returns Connection information (URL, port, etc.) */ - startServer( + getOrStartServer( interpreter: PythonEnvironment, - venvPath: vscode.Uri, - environmentId: string, + deepnoteFileUri: vscode.Uri, token?: vscode.CancellationToken ): Promise; /** - * Stops the deepnote-toolkit server for a kernel environment. - * @param environmentId The environment ID - * @param token Cancellation token to cancel the operation + * Stops the deepnote-toolkit server if running. + * @param deepnoteFileUri The URI of the .deepnote file */ - stopServer(environmentId: string, token?: vscode.CancellationToken): Promise; + stopServer(deepnoteFileUri: vscode.Uri): Promise; /** * Disposes all server processes and resources. @@ -157,8 +126,7 @@ export interface IDeepnoteServerStarter { export interface DeepnoteServerInfo { url: string; - jupyterPort: number; - lspPort: number; + port: number; token?: string; } @@ -186,150 +154,6 @@ export interface IDeepnoteKernelAutoSelector { * @param token Cancellation token to cancel the operation */ ensureKernelSelected(notebook: vscode.NotebookDocument, token?: vscode.CancellationToken): Promise; - - /** - * Force rebuild the controller for a notebook by clearing cached controller and metadata. - * This is used when switching environments to ensure a new controller is created. - * @param notebook The notebook document - * @param token Cancellation token to cancel the operation - */ - rebuildController(notebook: vscode.NotebookDocument, token?: vscode.CancellationToken): Promise; - - /** - * Clear the controller selection for a notebook using a specific environment. - * This is used when deleting an environment to unselect its controller from any open notebooks. - * @param notebook The notebook document - * @param environmentId The environment ID - */ - clearControllerForEnvironment(notebook: vscode.NotebookDocument, environmentId: string): void; -} - -export const IDeepnoteEnvironmentManager = Symbol('IDeepnoteEnvironmentManager'); -export interface IDeepnoteEnvironmentManager { - /** - * Initialize the manager by loading environments from storage - */ - initialize(): Promise; - - /** - * Wait for initialization to complete - */ - waitForInitialization(): Promise; - - /** - * Create a new kernel environment - * @param options Environment creation options - * @param token Cancellation token to cancel the operation - */ - createEnvironment( - options: CreateDeepnoteEnvironmentOptions, - token?: vscode.CancellationToken - ): Promise; - - /** - * Get all environments - */ - listEnvironments(): DeepnoteEnvironment[]; - - /** - * Get a specific environment by ID - */ - getEnvironment(id: string): DeepnoteEnvironment | undefined; - - /** - * Get environment with status information - */ - getEnvironmentWithStatus(id: string): DeepnoteEnvironmentWithStatus | undefined; - - /** - * Update an environment's metadata - */ - updateEnvironment( - id: string, - updates: Partial> - ): Promise; - - /** - * Delete an environment - * @param id The environment ID - * @param token Cancellation token to cancel the operation - */ - deleteEnvironment(id: string, token?: vscode.CancellationToken): Promise; - - /** - * Start the Jupyter server for an environment - * @param id The environment ID - */ - startServer(id: string, token?: vscode.CancellationToken): Promise; - - /** - * Stop the Jupyter server for an environment - * @param id The environment ID - * @param token Cancellation token to cancel the operation - */ - stopServer(id: string, token?: vscode.CancellationToken): Promise; - - /** - * Restart the Jupyter server for an environment - * @param id The environment ID - * @param token Cancellation token to cancel the operation - */ - restartServer(id: string, token?: vscode.CancellationToken): Promise; - - /** - * Update the last used timestamp for an environment - */ - updateLastUsed(id: string): Promise; - - /** - * Event fired when environments change - */ - onDidChangeEnvironments: vscode.Event; - - /** - * Dispose of all resources - */ - dispose(): void; -} - -export const IDeepnoteEnvironmentPicker = Symbol('IDeepnoteEnvironmentPicker'); -export interface IDeepnoteEnvironmentPicker { - /** - * Show a quick pick to select an environment for a notebook - * @param notebookUri The notebook URI (for context in messages) - * @returns Selected environment, or undefined if cancelled - */ - pickEnvironment(notebookUri: vscode.Uri): Promise; -} - -export const IDeepnoteNotebookEnvironmentMapper = Symbol('IDeepnoteNotebookEnvironmentMapper'); -export interface IDeepnoteNotebookEnvironmentMapper { - /** - * Get the environment ID selected for a notebook - * @param notebookUri The notebook URI (without query/fragment) - * @returns Environment ID, or undefined if not set - */ - getEnvironmentForNotebook(notebookUri: vscode.Uri): string | undefined; - - /** - * Set the environment for a notebook - * @param notebookUri The notebook URI (without query/fragment) - * @param environmentId The environment ID - */ - setEnvironmentForNotebook(notebookUri: vscode.Uri, environmentId: string): Promise; - - /** - * Remove the environment mapping for a notebook - * @param notebookUri The notebook URI (without query/fragment) - */ - removeEnvironmentForNotebook(notebookUri: vscode.Uri): Promise; - - /** - * Get all notebooks using a specific environment - * @param environmentId The environment ID - * @returns Array of notebook URIs - */ - getNotebooksUsingEnvironment(environmentId: string): vscode.Uri[]; } export const DEEPNOTE_TOOLKIT_VERSION = '1.0.0rc2'; diff --git a/src/kernels/helpers.ts b/src/kernels/helpers.ts index 9e79765aa2..78b701561b 100644 --- a/src/kernels/helpers.ts +++ b/src/kernels/helpers.ts @@ -300,14 +300,6 @@ export function getDisplayNameOrNameOfKernelConnection(kernelConnection: KernelC } else { return `Python ${pythonVersion}`.trim(); } - case 'startUsingDeepnoteKernel': { - // For Deepnote kernels, use the environment name if available - if (kernelConnection.environmentName) { - return `Deepnote: ${kernelConnection.environmentName}`; - } - // Fallback to kernelspec display name - return oldDisplayName; - } } return oldDisplayName; } diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index 2a0f4e55f4..4bae4114f2 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -290,47 +290,9 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont this.disposables.push(disposeAnyHandler); } public updateConnection(kernelConnection: KernelConnectionMetadata) { - // Check if the connection actually changed by comparing connection properties - // (not just IDs, since Deepnote uses notebook-based IDs that stay the same) - const oldConnection = this.kernelConnection; - const hasChanged = !areKernelConnectionsEqual(oldConnection, kernelConnection); - - logger.info( - `Updating controller ${this.id} connection. Changed: ${hasChanged}. ` + - `Old interpreter: ${ - oldConnection.interpreter ? getDisplayPath(oldConnection.interpreter.uri) : 'none' - }, ` + - `New interpreter: ${ - kernelConnection.interpreter ? getDisplayPath(kernelConnection.interpreter.uri) : 'none' - }` - ); - - // Update the stored connection metadata - this.kernelConnection = kernelConnection; - - // Update display name if (kernelConnection.kind !== 'connectToLiveRemoteKernel') { this.controller.label = getDisplayNameOrNameOfKernelConnection(kernelConnection); } - - // Only dispose kernels if the connection actually changed - // This avoids unnecessary kernel restarts when just reopening the same notebook - if (hasChanged) { - logger.info(`Connection changed - disposing old kernels to force reconnection`); - - // Dispose any existing kernels using the old connection for all associated notebooks - // This forces a fresh kernel connection when cells are next executed - const notebooksToUpdate = workspace.notebookDocuments.filter((doc) => this.associatedDocuments.has(doc)); - notebooksToUpdate.forEach((notebook) => { - const existingKernel = this.kernelProvider.get(notebook); - if (existingKernel) { - logger.info( - `Disposing old kernel for notebook ${getDisplayPath(notebook.uri)} due to connection update` - ); - existingKernel.dispose().catch(noop); - } - }); - } } public asWebviewUri(localResource: Uri): Uri { return this.controller.asWebviewUri(localResource); diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index a65e71796c..f185156250 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -9,23 +9,22 @@ import { NotebookControllerAffinity, window, ProgressLocation, + notebooks, NotebookController, CancellationTokenSource, Disposable, - Uri, l10n, - env, - commands + env } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; import { IDisposableRegistry } from '../../platform/common/types'; import { logger } from '../../platform/logging'; +import { IInterpreterService } from '../../platform/interpreter/contracts'; import { IDeepnoteKernelAutoSelector, + IDeepnoteServerStarter, + IDeepnoteToolkitInstaller, IDeepnoteServerProvider, - IDeepnoteEnvironmentManager, - IDeepnoteEnvironmentPicker, - IDeepnoteNotebookEnvironmentMapper, DEEPNOTE_NOTEBOOK_TYPE, DeepnoteKernelConnectionMetadata } from '../../kernels/deepnote/types'; @@ -43,9 +42,8 @@ import { IDeepnoteInitNotebookRunner } from './deepnoteInitNotebookRunner.node'; import { IDeepnoteNotebookManager } from '../types'; import { IDeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node'; import { DeepnoteProject } from '../../platform/deepnote/deepnoteTypes'; -import { IKernelProvider, IKernel, IJupyterKernelSpec } from '../../kernels/types'; +import { IKernelProvider, IKernel } from '../../kernels/types'; import { DeepnoteKernelError } from '../../platform/errors/deepnoteKernelErrors'; -import { DeepnoteEnvironment } from '../../kernels/deepnote/environments/deepnoteEnvironment'; import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants'; import { IOutputChannel } from '../../platform/common/types'; @@ -71,6 +69,9 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, constructor( @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IControllerRegistration) private readonly controllerRegistration: IControllerRegistration, + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, + @inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller, + @inject(IDeepnoteServerStarter) private readonly serverStarter: IDeepnoteServerStarter, @inject(IPythonExtensionChecker) private readonly pythonExtensionChecker: IPythonExtensionChecker, @inject(IDeepnoteServerProvider) private readonly serverProvider: IDeepnoteServerProvider, @inject(IJupyterRequestCreator) private readonly requestCreator: IJupyterRequestCreator, @@ -82,10 +83,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, @inject(IDeepnoteRequirementsHelper) private readonly requirementsHelper: IDeepnoteRequirementsHelper, - @inject(IDeepnoteEnvironmentManager) private readonly configurationManager: IDeepnoteEnvironmentManager, - @inject(IDeepnoteEnvironmentPicker) private readonly configurationPicker: IDeepnoteEnvironmentPicker, - @inject(IDeepnoteNotebookEnvironmentMapper) - private readonly notebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel ) {} @@ -122,6 +119,17 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, logger.info(`Deepnote notebook opened: ${getDisplayPath(notebook.uri)}`); + // Check if we already have a controller ready for this notebook + const baseFileUri = notebook.uri.with({ query: '', fragment: '' }); + const notebookKey = baseFileUri.fsPath; + const hasExistingController = this.notebookControllers.has(notebookKey); + + // If no existing controller, create a temporary "Loading" controller immediately + // This prevents the kernel selector from appearing when user clicks Run All + if (!hasExistingController) { + this.createLoadingController(notebook, notebookKey); + } + // Always try to ensure kernel is selected (this will reuse existing controllers) // Don't await - let it happen in background so notebook opens quickly void this.ensureKernelSelected(notebook).catch((error) => { @@ -247,65 +255,21 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } } - /** - * Switch controller to use a different environment by updating the existing controller's connection. - * Because we use notebook-based controller IDs (not environment-based), the controller ID stays the same - * and addOrUpdate will call updateConnection() on the existing controller instead of creating a new one. - * This keeps VS Code bound to the same controller object, avoiding DISPOSED errors. - */ - public async rebuildController(notebook: NotebookDocument, token?: CancellationToken): Promise { - const baseFileUri = notebook.uri.with({ query: '', fragment: '' }); - const notebookKey = baseFileUri.fsPath; - - logger.info(`Switching controller environment for ${getDisplayPath(notebook.uri)}`); - - // Check if any cells are executing and log a warning - const kernel = this.kernelProvider.get(notebook); - if (kernel) { - const pendingCells = this.kernelProvider.getKernelExecution(kernel).pendingCells; - if (pendingCells.length > 0) { - logger.warn( - `Switching environments while ${pendingCells.length} cell(s) are executing. Cells may fail.` - ); - } - } - - // Clear cached metadata so ensureKernelSelected creates fresh metadata with new environment - // The controller will stay alive - it will just get updated via updateConnection() - this.notebookConnectionMetadata.delete(notebookKey); - - // Clear old server handle - new environment will register a new handle - const oldServerHandle = this.notebookServerHandles.get(notebookKey); - if (oldServerHandle) { - logger.info(`Clearing old server handle from tracking: ${oldServerHandle}`); - this.notebookServerHandles.delete(notebookKey); - } - - // Update the controller with new environment's metadata - // Because we use notebook-based controller IDs, addOrUpdate will call updateConnection() - // on the existing controller instead of creating a new one - await this.ensureKernelSelected(notebook, token); - - logger.info(`Controller successfully switched to new environment`); - } - public async ensureKernelSelected(notebook: NotebookDocument, _token?: CancellationToken): Promise { - const baseFileUri = notebook.uri.with({ query: '', fragment: '' }); - const notebookKey = baseFileUri.fsPath; - - const kernelSelected = await window.withProgress( + return window.withProgress( { location: ProgressLocation.Notification, title: l10n.t('Loading Deepnote Kernel'), cancellable: true }, - async (progress, progressToken): Promise => { + async (progress, progressToken) => { try { logger.info(`Ensuring Deepnote kernel is selected for ${getDisplayPath(notebook.uri)}`); // Extract the base file URI (without query parameters) // Notebooks from the same .deepnote file have different URIs with ?notebook=id query params - + const baseFileUri = notebook.uri.with({ query: '', fragment: '' }); + const notebookKey = baseFileUri.fsPath; logger.info(`Base Deepnote file: ${getDisplayPath(baseFileUri)}`); // Check if we already have a controller for this notebook file @@ -336,7 +300,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, const selectedController = this.controllerRegistration.getSelected(notebook); if (selectedController && selectedController.id === existingController.id) { logger.info(`Controller already selected for ${getDisplayPath(notebook.uri)}`); - return true; + return; } // Auto-select the existing controller for this notebook @@ -354,331 +318,240 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, logger.info(`Disposed loading controller for ${notebookKey}`); } - return true; + return; } - // No existing controller - check if user has selected a configuration for this notebook - logger.info(`Checking for configuration selection for ${getDisplayPath(baseFileUri)}`); - let selectedConfigId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri); - let selectedConfig = selectedConfigId - ? this.configurationManager.getEnvironment(selectedConfigId) - : undefined; - - // If no configuration selected, or selected config was deleted, show picker - if (!selectedConfig) { - if (selectedConfigId) { - logger.warn( - `Previously selected configuration ${selectedConfigId} not found - showing picker` - ); - } else { - logger.info(`No configuration selected for notebook - showing picker`); - } - - progress.report({ message: 'Select kernel configuration...' }); - selectedConfig = await this.configurationPicker.pickEnvironment(baseFileUri); + // No existing controller, so create a new one + logger.info(`Creating new Deepnote kernel for ${getDisplayPath(notebook.uri)}`); + progress.report({ message: l10n.t('Setting up Deepnote kernel...') }); - if (!selectedConfig) { - logger.info( - `User cancelled configuration selection or no environment found - no kernel will be loaded` - ); - return false; - } + // Check if Python extension is installed + if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { + logger.warn('Python extension is not installed. Prompting user to install it.'); + await this.pythonExtensionChecker.showPythonExtensionInstallRequiredPrompt(); + return; // Exit - user needs to install Python extension first + } - // Save the selection - await this.notebookEnvironmentMapper.setEnvironmentForNotebook(baseFileUri, selectedConfig.id); - logger.info(`Saved configuration selection: ${selectedConfig.name} (${selectedConfig.id})`); - } else { - logger.info(`Using mapped configuration: ${selectedConfig.name} (${selectedConfig.id})`); + // Get active Python interpreter + progress.report({ message: l10n.t('Finding Python interpreter...') }); + const interpreter = await this.interpreterService.getActiveInterpreter(notebook.uri); + if (!interpreter) { + logger.warn( + 'No Python interpreter found for Deepnote notebook. Kernel selection will be manual.' + ); + return; // Exit gracefully - user can select kernel manually } - // Use the selected configuration - await this.ensureKernelSelectedWithConfiguration( - notebook, - selectedConfig, + logger.info(`Using base interpreter: ${getDisplayPath(interpreter.uri)}`); + + // Ensure deepnote-toolkit is installed in a venv and get the venv interpreter + // Will throw typed errors on failure (DeepnoteVenvCreationError or DeepnoteToolkitInstallError) + progress.report({ message: l10n.t('Installing Deepnote toolkit...') }); + const venvInterpreter = await this.toolkitInstaller.ensureInstalled( + interpreter, baseFileUri, - notebookKey, - progress, progressToken ); - return true; - } catch (ex) { - logger.error('Failed to auto-select Deepnote kernel', ex); - throw ex; - } - } - ); + logger.info(`Deepnote toolkit venv ready at: ${getDisplayPath(venvInterpreter.uri)}`); - if (!kernelSelected) { - const createLabel = l10n.t('Create Environment'); - const cancelLabel = l10n.t('Cancel'); + // Start the Deepnote server using the venv interpreter + progress.report({ message: l10n.t('Starting Deepnote server...') }); + const serverInfo = await this.serverStarter.getOrStartServer( + venvInterpreter, + baseFileUri, + progressToken + ); + logger.info(`Deepnote server running at ${serverInfo.url}`); + + // Create server provider handle + const serverProviderHandle: JupyterServerProviderHandle = { + extensionId: JVSC_EXTENSION_ID, + id: 'deepnote-server', + handle: `deepnote-toolkit-server-${baseFileUri.fsPath}` + }; + + // Register the server with the provider so it can be resolved + this.serverProvider.registerServer(serverProviderHandle.handle, serverInfo); + + // Track the server handle for cleanup when notebook is closed + this.notebookServerHandles.set(notebookKey, serverProviderHandle.handle); + + // Connect to the server and get available kernel specs + progress.report({ message: l10n.t('Connecting to kernel...') }); + const connectionInfo = createJupyterConnectionInfo( + serverProviderHandle, + { + baseUrl: serverInfo.url, + token: serverInfo.token || '', + displayName: 'Deepnote Server', + authorizationHeader: {} + }, + this.requestCreator, + this.requestAgentCreator, + this.configService, + baseFileUri + ); - const choice = await window.showInformationMessage( - l10n.t('No environments found. Create one to use with {0}?', getDisplayPath(baseFileUri)), - createLabel, - cancelLabel - ); + const sessionManager = JupyterLabHelper.create(connectionInfo.settings); + let kernelSpec; + try { + const kernelSpecs = await sessionManager.getKernelSpecs(); + logger.info( + `Available kernel specs on Deepnote server: ${kernelSpecs.map((s) => s.name).join(', ')}` + ); - if (choice === createLabel) { - // Trigger the create command - logger.info('Triggering create environment command from picker'); - await commands.executeCommand('deepnote.environments.create'); + // Create expected kernel name based on file path (uses installer's hash logic) + const venvHash = this.toolkitInstaller.getVenvHash(baseFileUri); + const expectedKernelName = `deepnote-venv-${venvHash}`; + logger.info(`Looking for venv kernel spec: ${expectedKernelName}`); - const selectedConfig = await this.configurationPicker.pickEnvironment(baseFileUri); - if (!selectedConfig) { - return; - } + // Prefer the venv kernel spec that uses the venv's Python interpreter + // This ensures packages installed via pip are available to the kernel + kernelSpec = kernelSpecs.find((s) => s.name === expectedKernelName); - const tmpCancellation = new CancellationTokenSource(); - const tmpCancellationToken = tmpCancellation.token; - - // Use the selected configuration - await this.ensureKernelSelectedWithConfiguration( - notebook, - selectedConfig, - baseFileUri, - notebookKey, - { - report: () => { - logger.info('Progress report'); + if (!kernelSpec) { + logger.warn( + l10n.t( + "⚠️ Venv kernel spec '{expectedKernelName}' not found! Falling back to generic Python kernel.", + { expectedKernelName } + ) + ); + logger.warn( + l10n.t( + 'This may cause import errors if packages are installed to the venv but kernel uses system Python.' + ) + ); + kernelSpec = + kernelSpecs.find((s) => s.language === 'python') || + kernelSpecs.find((s) => s.name === 'python3-venv') || + kernelSpecs[0]; } - }, - tmpCancellationToken - ); - } - } - } - - private async ensureKernelSelectedWithConfiguration( - notebook: NotebookDocument, - configuration: DeepnoteEnvironment, - baseFileUri: Uri, - notebookKey: string, - progress: { report(value: { message?: string; increment?: number }): void }, - progressToken: CancellationToken - ): Promise { - logger.info(`Setting up kernel using configuration: ${configuration.name} (${configuration.id})`); - progress.report({ message: `Using ${configuration.name}...` }); - - // Check if Python extension is installed - if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { - logger.warn('Python extension is not installed. Prompting user to install it.'); - await this.pythonExtensionChecker.showPythonExtensionInstallRequiredPrompt(); - return; - } - - // Ensure server is running (startServer is idempotent - returns early if already running) - // Note: startServer() will create the venv if it doesn't exist - // IMPORTANT: Always call this and refresh configuration to get current server info, - // as the configuration object may have stale serverInfo from a previous session - logger.info(`Ensuring server is running for configuration ${configuration.id}`); - progress.report({ message: 'Starting Deepnote server...' }); - await this.configurationManager.startServer(configuration.id, progressToken); - - // ALWAYS refresh configuration to get current serverInfo - // This is critical because the configuration object may have been cached - const updatedConfig = this.configurationManager.getEnvironment(configuration.id); - if (!updatedConfig?.serverInfo) { - throw new Error('Failed to start server for configuration'); - } - configuration = updatedConfig; // Use fresh configuration with current serverInfo - // TypeScript can't infer that serverInfo is non-null after the check above, so we use non-null assertion - const serverInfo = configuration.serverInfo!; - logger.info(`Server running at ${serverInfo.url}`); - - // Update last used timestamp - await this.configurationManager.updateLastUsed(configuration.id); - - // Create server provider handle - const serverProviderHandle: JupyterServerProviderHandle = { - extensionId: JVSC_EXTENSION_ID, - id: 'deepnote-server', - handle: `deepnote-config-server-${configuration.id}` - }; - - // Register the server with the provider - this.serverProvider.registerServer(serverProviderHandle.handle, serverInfo); - this.notebookServerHandles.set(notebookKey, serverProviderHandle.handle); - - // Connect to the server and get available kernel specs - progress.report({ message: 'Connecting to kernel...' }); - const connectionInfo = createJupyterConnectionInfo( - serverProviderHandle, - { - baseUrl: serverInfo.url, - token: serverInfo.token || '', - displayName: `Deepnote: ${configuration.name}`, - authorizationHeader: {} - }, - this.requestCreator, - this.requestAgentCreator, - this.configService, - baseFileUri - ); - - const sessionManager = JupyterLabHelper.create(connectionInfo.settings); - let kernelSpec; - try { - const kernelSpecs = await sessionManager.getKernelSpecs(); - logger.info(`Available kernel specs on Deepnote server: ${kernelSpecs.map((s) => s.name).join(', ')}`); - - // Use the extracted kernel selection logic - kernelSpec = this.selectKernelSpec(kernelSpecs, configuration.id); - logger.info(`✓ Using kernel spec: ${kernelSpec.name} (${kernelSpec.display_name})`); - } finally { - await disposeAsync(sessionManager); - } - - progress.report({ message: 'Finalizing kernel setup...' }); - - // Get the venv Python interpreter (not the base interpreter) - const venvInterpreter = - process.platform === 'win32' - ? Uri.joinPath(configuration.venvPath, 'Scripts', 'python.exe') - : Uri.joinPath(configuration.venvPath, 'bin', 'python'); - - // CRITICAL: Use notebook-based ID instead of environment-based ID - // This ensures that when switching environments, addOrUpdate will call updateConnection() - // on the existing controller instead of creating a new one. This keeps VS Code bound to - // the same controller object, avoiding the DISPOSED error. - const controllerId = `deepnote-notebook-${notebookKey}`; - - const newConnectionMetadata = DeepnoteKernelConnectionMetadata.create({ - interpreter: { uri: venvInterpreter, id: venvInterpreter.fsPath }, - kernelSpec, - baseUrl: serverInfo.url, - id: controllerId, - serverProviderHandle, - serverInfo, - environmentName: configuration.name - }); + if (!kernelSpec) { + throw new Error('No kernel specs available on Deepnote server'); + } - // Store connection metadata for reuse - this.notebookConnectionMetadata.set(notebookKey, newConnectionMetadata); + logger.info(`✓ Using kernel spec: ${kernelSpec.name} (${kernelSpec.display_name})`); + } finally { + await disposeAsync(sessionManager); + } - // Register controller for deepnote notebook type - const controllers = this.controllerRegistration.addOrUpdate(newConnectionMetadata, [DEEPNOTE_NOTEBOOK_TYPE]); + progress.report({ message: l10n.t('Finalizing kernel setup...') }); + const newConnectionMetadata = DeepnoteKernelConnectionMetadata.create({ + interpreter, + kernelSpec, + baseUrl: serverInfo.url, + id: `deepnote-kernel-${interpreter.id}`, + serverProviderHandle, + serverInfo // Pass the server info so we can use it later + }); + + // Store connection metadata for reuse + this.notebookConnectionMetadata.set(notebookKey, newConnectionMetadata); + + // Register controller for deepnote notebook type + const controllers = this.controllerRegistration.addOrUpdate(newConnectionMetadata, [ + DEEPNOTE_NOTEBOOK_TYPE + ]); + + if (controllers.length === 0) { + logger.error('Failed to create Deepnote kernel controller'); + throw new Error('Failed to create Deepnote kernel controller'); + } - if (controllers.length === 0) { - logger.error('Failed to create Deepnote kernel controller'); - throw new Error('Failed to create Deepnote kernel controller'); - } + const controller = controllers[0]; + logger.info(`Created Deepnote kernel controller: ${controller.id}`); - const controller = controllers[0]; - logger.info(`Created Deepnote kernel controller: ${controller.id}`); + // Store the controller for reuse + this.notebookControllers.set(notebookKey, controller); - // Store the controller for reuse - this.notebookControllers.set(notebookKey, controller); + // Prepare init notebook execution for when kernel starts + // This MUST complete before marking controller as preferred to avoid race conditions + const projectId = notebook.metadata?.deepnoteProjectId; + const project = projectId + ? (this.notebookManager.getOriginalProject(projectId) as DeepnoteProject | undefined) + : undefined; - // Prepare init notebook execution - const projectId = notebook.metadata?.deepnoteProjectId; - const project = projectId - ? (this.notebookManager.getOriginalProject(projectId) as DeepnoteProject | undefined) - : undefined; - - if (project) { - progress.report({ message: 'Creating requirements.txt...' }); - await this.requirementsHelper.createRequirementsFile(project, progressToken); - logger.info(`Created requirements.txt for project ${projectId}`); - - if (project.project.initNotebookId && !this.notebookManager.hasInitNotebookBeenRun(projectId!)) { - this.projectsPendingInitNotebook.set(projectId!, { notebook, project }); - logger.info(`Init notebook will run automatically when kernel starts for project ${projectId}`); - } - } + if (project) { + // Create requirements.txt first (needs to be ready for init notebook) + progress.report({ message: l10n.t('Creating requirements.txt...') }); + await this.requirementsHelper.createRequirementsFile(project, progressToken); + logger.info(`Created requirements.txt for project ${projectId}`); + + // Check if project has an init notebook that hasn't been run yet + if ( + project.project.initNotebookId && + !this.notebookManager.hasInitNotebookBeenRun(projectId!) + ) { + // Store for execution when kernel actually starts + // Kernels are created lazily when cells execute, so we can't run init notebook now + this.projectsPendingInitNotebook.set(projectId!, { notebook, project }); + logger.info( + `Init notebook will run automatically when kernel starts for project ${projectId}` + ); + } + } - // Mark controller as protected - this.controllerRegistration.trackActiveInterpreterControllers(controllers); - logger.info(`Marked Deepnote controller as protected from automatic disposal`); - - // Listen to controller disposal - controller.onDidDispose(() => { - logger.info(`Deepnote controller ${controller!.id} disposed, checking if we should remove from tracking`); - // Only remove from map if THIS controller is still the one mapped to this notebookKey - // This prevents old controllers from deleting newer controllers during environment switching - const currentController = this.notebookControllers.get(notebookKey); - if (currentController?.id === controller.id) { - logger.info(`Removing controller ${controller.id} from tracking map`); - this.notebookControllers.delete(notebookKey); - } else { - logger.info( - `Not removing controller ${controller.id} from tracking - a newer controller ${currentController?.id} has replaced it` - ); + // Mark this controller as protected so it won't be automatically disposed + // This is similar to how active interpreter controllers are protected + this.controllerRegistration.trackActiveInterpreterControllers(controllers); + logger.info(`Marked Deepnote controller as protected from automatic disposal`); + + // Listen to controller disposal so we can clean up our tracking + controller.onDidDispose(() => { + logger.info(`Deepnote controller ${controller!.id} disposed, removing from tracking`); + this.notebookControllers.delete(notebookKey); + // Keep connection metadata for quick recreation + // The metadata is still valid and can be used to recreate the controller + }); + + // Auto-select the controller for this notebook using affinity + // Setting NotebookControllerAffinity.Preferred will make VSCode automatically select this controller + // This is done AFTER requirements.txt creation to avoid race conditions + controller.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Preferred); + + logger.info(`Successfully auto-selected Deepnote kernel for ${getDisplayPath(notebook.uri)}`); + progress.report({ message: l10n.t('Kernel ready!') }); + + // Dispose the loading controller once the real one is ready + const loadingController = this.loadingControllers.get(notebookKey); + if (loadingController) { + loadingController.dispose(); + this.loadingControllers.delete(notebookKey); + logger.info(`Disposed loading controller for ${notebookKey}`); + } + } catch (ex) { + logger.error('Failed to auto-select Deepnote kernel', ex); + throw ex; + } } - }); - - // Dispose the loading controller BEFORE selecting the real one - // This ensures VS Code switches directly to our controller - const loadingController = this.loadingControllers.get(notebookKey); - if (loadingController) { - loadingController.dispose(); - this.loadingControllers.delete(notebookKey); - logger.info(`Disposed loading controller for ${notebookKey}`); - } - - // Auto-select the controller - controller.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Preferred); - - logger.info(`Successfully set up kernel with configuration: ${configuration.name}`); - progress.report({ message: 'Kernel ready!' }); + ); } - /** - * Select the appropriate kernel spec for an environment. - * Extracted for testability. - * @param kernelSpecs Available kernel specs from the server - * @param environmentId The environment ID to find a kernel for - * @returns The selected kernel spec - * @throws Error if no suitable kernel spec is found - */ - public selectKernelSpec(kernelSpecs: IJupyterKernelSpec[], environmentId: string): IJupyterKernelSpec { - // Look for environment-specific kernel first - const expectedKernelName = `deepnote-${environmentId}`; - logger.info(`Looking for environment-specific kernel: ${expectedKernelName}`); - - const kernelSpec = kernelSpecs.find((s) => s.name === expectedKernelName); - - if (!kernelSpec) { - logger.warn( - `Environment-specific kernel '${expectedKernelName}' not found! Falling back to generic Python kernel.` - ); - // Fallback to any Python kernel - const fallbackKernel = - kernelSpecs.find((s) => s.language === 'python') || - kernelSpecs.find((s) => s.name === 'python3') || - kernelSpecs[0]; - - if (!fallbackKernel) { - throw new Error('No kernel specs available on Deepnote server'); - } - - return fallbackKernel; - } + private createLoadingController(notebook: NotebookDocument, notebookKey: string): void { + // Create a temporary controller that shows "Loading..." and prevents kernel selection prompt + const loadingController = notebooks.createNotebookController( + `deepnote-loading-${notebookKey}`, + DEEPNOTE_NOTEBOOK_TYPE, + l10n.t('Loading Deepnote Kernel...') + ); - return kernelSpec; - } + // Set it as the preferred controller immediately + loadingController.supportsExecutionOrder = false; + loadingController.supportedLanguages = ['python']; - /** - * Clear the controller selection for a notebook using a specific environment. - * This is used when deleting an environment to unselect its controller from any open notebooks. - */ - public clearControllerForEnvironment(notebook: NotebookDocument, environmentId: string): void { - const selectedController = this.controllerRegistration.getSelected(notebook); - if (!selectedController || selectedController.connection.kind !== 'startUsingDeepnoteKernel') { - return; - } + // Execution handler that does nothing - cells will just sit there until real kernel is ready + loadingController.executeHandler = () => { + // No-op: execution is blocked until the real controller takes over + }; - const deepnoteMetadata = selectedController.connection as DeepnoteKernelConnectionMetadata; - const expectedHandle = `deepnote-config-server-${environmentId}`; + // Select this controller for the notebook + loadingController.updateNotebookAffinity(notebook, NotebookControllerAffinity.Preferred); - if (deepnoteMetadata.serverProviderHandle.handle === expectedHandle) { - // Unselect the controller by setting affinity to Default - selectedController.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Default); - logger.info( - `Cleared controller for notebook ${getDisplayPath(notebook.uri)} (environment ${environmentId})` - ); - } + // Store it so we can dispose it later + this.loadingControllers.set(notebookKey, loadingController); + logger.info(`Created loading controller for ${notebookKey}`); } /** diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts deleted file mode 100644 index 0cfb50b373..0000000000 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ /dev/null @@ -1,821 +0,0 @@ -import { assert } from 'chai'; -import * as sinon from 'sinon'; -import { anything, instance, mock, when } from 'ts-mockito'; -import { DeepnoteKernelAutoSelector } from './deepnoteKernelAutoSelector.node'; -import { - IDeepnoteEnvironmentManager, - IDeepnoteServerProvider, - IDeepnoteEnvironmentPicker, - IDeepnoteNotebookEnvironmentMapper -} from '../../kernels/deepnote/types'; -import { IControllerRegistration, IVSCodeNotebookController } from '../controllers/types'; -import { IDisposableRegistry, IOutputChannel } from '../../platform/common/types'; -import { IPythonExtensionChecker } from '../../platform/api/types'; -import { IJupyterRequestCreator } from '../../kernels/jupyter/types'; -import { IConfigurationService } from '../../platform/common/types'; -import { IDeepnoteInitNotebookRunner } from './deepnoteInitNotebookRunner.node'; -import { IDeepnoteNotebookManager } from '../types'; -import { IKernelProvider, IKernel, IJupyterKernelSpec } from '../../kernels/types'; -import { IDeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node'; -import { NotebookDocument, Uri, NotebookController, CancellationToken } from 'vscode'; -import { DeepnoteEnvironment } from '../../kernels/deepnote/environments/deepnoteEnvironment'; -import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; - -suite('DeepnoteKernelAutoSelector - rebuildController', () => { - let selector: DeepnoteKernelAutoSelector; - let mockDisposableRegistry: IDisposableRegistry; - let mockControllerRegistration: IControllerRegistration; - let mockPythonExtensionChecker: IPythonExtensionChecker; - let mockServerProvider: IDeepnoteServerProvider; - let mockRequestCreator: IJupyterRequestCreator; - let mockConfigService: IConfigurationService; - let mockInitNotebookRunner: IDeepnoteInitNotebookRunner; - let mockNotebookManager: IDeepnoteNotebookManager; - let mockKernelProvider: IKernelProvider; - let mockRequirementsHelper: IDeepnoteRequirementsHelper; - let mockEnvironmentManager: IDeepnoteEnvironmentManager; - let mockEnvironmentPicker: IDeepnoteEnvironmentPicker; - let mockNotebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper; - let mockOutputChannel: IOutputChannel; - - let mockNotebook: NotebookDocument; - let mockController: IVSCodeNotebookController; - let mockNewController: IVSCodeNotebookController; - let sandbox: sinon.SinonSandbox; - - setup(() => { - sandbox = sinon.createSandbox(); - // Create mocks for all dependencies - mockDisposableRegistry = mock(); - mockControllerRegistration = mock(); - mockPythonExtensionChecker = mock(); - mockServerProvider = mock(); - mockRequestCreator = mock(); - mockConfigService = mock(); - mockInitNotebookRunner = mock(); - mockNotebookManager = mock(); - mockKernelProvider = mock(); - mockRequirementsHelper = mock(); - mockEnvironmentManager = mock(); - mockEnvironmentPicker = mock(); - mockNotebookEnvironmentMapper = mock(); - mockOutputChannel = mock(); - - // Create mock notebook - mockNotebook = { - uri: Uri.parse('file:///test/notebook.deepnote?notebook=123'), - notebookType: 'deepnote', - metadata: { deepnoteProjectId: 'project-123' }, - // Add minimal required properties for NotebookDocument - version: 1, - isDirty: false, - isUntitled: false, - isClosed: false, - cellCount: 0, - cellAt: () => { - throw new Error('Not implemented'); - }, - getCells: () => [], - save: async () => true - } as unknown as NotebookDocument; - - // Create mock controllers - mockController = mock(); - when(mockController.id).thenReturn('deepnote-config-kernel-old-env-id'); - when(mockController.controller).thenReturn({} as NotebookController); - - mockNewController = mock(); - when(mockNewController.id).thenReturn('deepnote-config-kernel-new-env-id'); - when(mockNewController.controller).thenReturn({} as NotebookController); - - // Mock disposable registry - push returns the index - when(mockDisposableRegistry.push(anything())).thenReturn(0); - - // Create selector instance - selector = new DeepnoteKernelAutoSelector( - instance(mockDisposableRegistry), - instance(mockControllerRegistration), - instance(mockPythonExtensionChecker), - instance(mockServerProvider), - instance(mockRequestCreator), - undefined, // requestAgentCreator is optional - instance(mockConfigService), - instance(mockInitNotebookRunner), - instance(mockNotebookManager), - instance(mockKernelProvider), - instance(mockRequirementsHelper), - instance(mockEnvironmentManager), - instance(mockEnvironmentPicker), - instance(mockNotebookEnvironmentMapper), - instance(mockOutputChannel) - ); - }); - - teardown(() => { - sandbox.restore(); - }); - - suite('rebuildController', () => { - test('should proceed with environment switch despite pending cells', async () => { - // This test verifies that rebuildController continues with the environment switch - // even when cells are currently executing (pending) - - // Arrange - const mockKernel = mock(); - const mockExecution = { - pendingCells: [{ index: 0 }, { index: 1 }] // 2 cells pending - }; - - when(mockKernelProvider.get(mockNotebook)).thenReturn(instance(mockKernel)); - when(mockKernelProvider.getKernelExecution(instance(mockKernel))).thenReturn(mockExecution as any); - - // Stub ensureKernelSelected to verify it's still called despite pending cells - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelected').resolves(); - - // Act - await selector.rebuildController(mockNotebook); - - // Assert - should proceed despite pending cells - assert.strictEqual( - ensureKernelSelectedStub.calledOnce, - true, - 'ensureKernelSelected should be called even with pending cells' - ); - assert.strictEqual( - ensureKernelSelectedStub.firstCall.args[0], - mockNotebook, - 'ensureKernelSelected should be called with the notebook' - ); - }); - - test('should proceed without error when no kernel is running', async () => { - // This test verifies that rebuildController works correctly when no kernel is active - // (i.e., no cells have been executed yet) - - // Arrange - when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - - // Stub ensureKernelSelected to verify it's called - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelected').resolves(); - - // Act - await selector.rebuildController(mockNotebook); - - // Assert - should proceed normally without a kernel - assert.strictEqual( - ensureKernelSelectedStub.calledOnce, - true, - 'ensureKernelSelected should be called even when no kernel exists' - ); - assert.strictEqual( - ensureKernelSelectedStub.firstCall.args[0], - mockNotebook, - 'ensureKernelSelected should be called with the notebook' - ); - }); - - test('should complete successfully and delegate to ensureKernelSelected', async () => { - // This test verifies that rebuildController completes successfully - // and delegates kernel setup to ensureKernelSelected - // Note: rebuildController does NOT dispose old controllers to prevent - // "notebook controller is DISPOSED" errors for queued cell executions - - // Arrange - when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - - // Stub ensureKernelSelected to verify delegation - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelected').resolves(); - - // Act - await selector.rebuildController(mockNotebook); - - // Assert - method should complete without errors - assert.strictEqual( - ensureKernelSelectedStub.calledOnce, - true, - 'ensureKernelSelected should be called to set up the new environment' - ); - }); - - test('should clear metadata and call ensureKernelSelected to recreate controller', async () => { - // This test verifies that rebuildController: - // 1. Clears cached connection metadata (forces fresh metadata creation) - // 2. Clears old server handle - // 3. Calls ensureKernelSelected to set up controller with new environment - - // Arrange - // Mock kernel provider to return no kernel (no cells executing) - when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - - // Get the notebook key that will be used internally - const baseFileUri = mockNotebook.uri.with({ query: '', fragment: '' }); - const notebookKey = baseFileUri.fsPath; - - // Set up initial metadata and server handle to verify they get cleared - const selectorWithPrivateAccess = selector as any; - const mockMetadata = { id: 'test-metadata' }; - const mockServerHandle = 'test-server-handle'; - selectorWithPrivateAccess.notebookConnectionMetadata.set(notebookKey, mockMetadata); - selectorWithPrivateAccess.notebookServerHandles.set(notebookKey, mockServerHandle); - - // Verify metadata is set before rebuild - assert.strictEqual( - selectorWithPrivateAccess.notebookConnectionMetadata.has(notebookKey), - true, - 'Metadata should be set before rebuildController' - ); - assert.strictEqual( - selectorWithPrivateAccess.notebookServerHandles.has(notebookKey), - true, - 'Server handle should be set before rebuildController' - ); - - // Stub ensureKernelSelected to avoid full execution - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelected').resolves(); - - // Act - await selector.rebuildController(mockNotebook); - - // Assert - verify metadata has been cleared - assert.strictEqual( - selectorWithPrivateAccess.notebookConnectionMetadata.has(notebookKey), - false, - 'Connection metadata should be cleared to force fresh metadata creation' - ); - assert.strictEqual( - selectorWithPrivateAccess.notebookServerHandles.has(notebookKey), - false, - 'Server handle should be cleared' - ); - - // Assert - verify ensureKernelSelected has been called - assert.strictEqual( - ensureKernelSelectedStub.calledOnce, - true, - 'ensureKernelSelected should have been called once' - ); - assert.strictEqual( - ensureKernelSelectedStub.firstCall.args[0], - mockNotebook, - 'ensureKernelSelected should be called with the notebook' - ); - }); - - test('should pass cancellation token to ensureKernelSelected', async () => { - // This test verifies that rebuildController correctly passes the cancellation token - // to ensureKernelSelected, allowing the operation to be cancelled during execution - - // Arrange - const cancellationToken = mock(); - when(cancellationToken.isCancellationRequested).thenReturn(true); - when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - - // Stub ensureKernelSelected to verify it receives the token - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelected').resolves(); - - // Act - await selector.rebuildController(mockNotebook, instance(cancellationToken)); - - // Assert - assert.strictEqual(ensureKernelSelectedStub.calledOnce, true, 'ensureKernelSelected should be called once'); - assert.strictEqual( - ensureKernelSelectedStub.firstCall.args[0], - mockNotebook, - 'ensureKernelSelected should be called with the notebook' - ); - assert.strictEqual( - ensureKernelSelectedStub.firstCall.args[1], - instance(cancellationToken), - 'ensureKernelSelected should be called with the cancellation token' - ); - }); - }); - - suite('environment switching integration', () => { - test('should switch from one environment to another', async () => { - // This test simulates the full flow: - // 1. User has Environment A selected - // 2. User switches to Environment B via the UI - // 3. rebuildController is called - // 4. ensureKernelSelected is invoked to set up new controller with Environment B - - // Arrange - // Mock kernel provider to return no kernel (no cells executing) - when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - - // Stub ensureKernelSelected to track calls without full execution - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelected').resolves(); - - // Act: Call rebuildController to switch environments - await selector.rebuildController(mockNotebook); - - // Assert: Verify ensureKernelSelected was called to set up new controller - assert.strictEqual( - ensureKernelSelectedStub.calledOnce, - true, - 'ensureKernelSelected should have been called once to set up new environment' - ); - assert.strictEqual( - ensureKernelSelectedStub.firstCall.args[0], - mockNotebook, - 'ensureKernelSelected should be called with the notebook' - ); - }); - }); - - // Priority 1 Tests - Critical for environment switching - // UT-4: Configuration Refresh After startServer - suite('Priority 1: Configuration Refresh (UT-4)', () => { - test('Implementation verifies INV-10: config is refreshed after startServer', () => { - // This documents INV-10: Configuration object must be refreshed after startServer() - // to get current serverInfo (not stale/undefined serverInfo) - // - // THE ACTUAL IMPLEMENTATION DOES THIS CORRECTLY: - // See deepnoteKernelAutoSelector.node.ts:450-467: - // - // await this.configurationManager.startServer(configuration.id); - // - // // ALWAYS refresh configuration to get current serverInfo - // const updatedConfig = this.configurationManager.getEnvironment(configuration.id); - // if (!updatedConfig?.serverInfo) { - // throw new Error('Failed to start server for configuration'); - // } - // configuration = updatedConfig; // Use fresh configuration - // - // The environment manager (tested in deepnoteEnvironmentManager.unit.test.ts) - // ensures serverInfo is ALWAYS updated when startServer() is called. - // - // See UT-6 test: "should always call serverStarter.startServer to ensure fresh serverInfo" - // This verifies the environment manager always updates serverInfo. - - assert.ok(true, 'INV-10 is verified by implementation and UT-6 test'); - }); - - test('Implementation verifies error handling for missing serverInfo', () => { - // Documents that the code throws a meaningful error if serverInfo is undefined - // after calling startServer() and refreshing the configuration. - // - // THE ACTUAL IMPLEMENTATION DOES THIS: - // See deepnoteKernelAutoSelector.node.ts:458-461: - // - // const updatedConfig = this.configurationManager.getEnvironment(configuration.id); - // if (!updatedConfig?.serverInfo) { - // throw new Error('Failed to start server for configuration'); - // } - // - // This prevents using stale or undefined serverInfo which would cause connection errors. - - assert.ok(true, 'Error handling for missing serverInfo is implemented correctly'); - }); - }); - - // Priority 1 Integration Tests - Critical for environment switching - suite('Priority 1: Integration Tests (IT-1, IT-8)', () => { - test('IT-1: Full environment switch flow is validated by existing tests', () => { - // IT-1 requires testing the full environment switch flow: - // 1. Notebook mapped to environment B - // 2. New controller for B created and selected - // 3. Old controller for A left alive (not disposed) to handle queued executions - // 4. Can execute cell successfully on B - // - // THIS IS VALIDATED BY EXISTING TESTS: - // - // 1. "should switch from one environment to another" (line 260) - // - Simulates switching from env-a to env-b - // - Validates rebuildController flow with environment change - // - // 2. "should NOT dispose old controller..." (line 178) - // - Validates that old controller is NOT disposed - // - This prevents "DISPOSED" errors for queued cell executions - // - Old controller will be garbage collected naturally - // - // 3. "should clear cached controller and metadata" (line 109) - // - Validates state clearing before rebuild - // - Ensures clean state for new environment - // - // 4. "should unregister old server handle" (line 151) - // - Validates server cleanup during switch - // - // Full integration testing with actual cell execution requires a running VS Code - // instance and is better suited for E2E tests. These unit tests validate all the - // critical invariants that make environment switching work correctly. - - assert.ok(true, 'IT-1 requirements validated by existing rebuildController tests'); - }); - - test('IT-8: Execute cell immediately after switch validated by disposal order tests', () => { - // IT-8 requires: "Execute cell immediately after environment switch" - // Verify: - // 1. Cell executes successfully - // 2. No "controller disposed" error - // 3. Output shows new environment - // - // THIS IS VALIDATED BY THE NON-DISPOSAL APPROACH: - // - // The test on line 178 validates that old controllers are NOT disposed. - // - // This prevents the "controller disposed" error because: - // - VS Code may have queued cell executions that reference the old controller - // - If we disposed the old controller, those executions would fail with "DISPOSED" error - // - By leaving the old controller alive, queued executions complete successfully - // - New cell executions use the new controller (it's now preferred) - // - The old controller will be garbage collected when no longer referenced - // - // The implementation at deepnoteKernelAutoSelector.node.ts:306-315 does this: - // // IMPORTANT: We do NOT dispose the old controller here - // // Reason: VS Code may have queued cell executions that reference the old controller - // // If we dispose it immediately, those queued executions will fail with "DISPOSED" error - // // Instead, we let the old controller stay alive - it will be garbage collected eventually - // - // Full integration testing with actual cell execution requires a running VS Code - // instance with real kernel execution, which is better suited for E2E tests. - - assert.ok(true, 'IT-8 requirements validated by INV-1 and INV-2 controller disposal tests'); - }); - }); - - // Priority 2 Tests - High importance for environment switching - suite('Priority 2: State Management (UT-2)', () => { - test('Implementation verifies INV-9: cached state cleared before rebuild', () => { - // UT-2 requires verifying that rebuildController() clears cached state: - // 1. notebookControllers.delete() called before ensureKernelSelected() - // 2. notebookConnectionMetadata.delete() called before ensureKernelSelected() - // 3. Old server unregistered from provider - // - // THIS IS VALIDATED BY EXISTING TESTS AND IMPLEMENTATION: - // - // 1. "should clear cached controller and metadata" test (line 109) - // - Tests the cache clearing behavior during rebuild - // - Validates INV-9: Connection metadata cache cleared before creating new metadata - // - // 2. "should unregister old server handle" test (line 151) - // - Validates server cleanup during rebuild - // - Ensures old server is unregistered from provider - // - // THE ACTUAL IMPLEMENTATION at deepnoteKernelAutoSelector.node.ts:269-291: - // - // // Clear cached state - // this.notebookControllers.delete(notebookKey); - // this.notebookConnectionMetadata.delete(notebookKey); - // - // // Unregister old server - // const oldServerHandle = this.notebookServerHandles.get(notebookKey); - // if (oldServerHandle) { - // this.serverProvider.unregisterServer(oldServerHandle); - // this.notebookServerHandles.delete(notebookKey); - // } - // - // These operations happen BEFORE calling ensureKernelSelected() to create the new controller, - // ensuring clean state for the environment switch. - - assert.ok(true, 'UT-2 is validated by existing tests and implementation (INV-9)'); - }); - }); - - suite('Priority 2: Server Concurrency (UT-7)', () => { - test('Implementation verifies INV-8: concurrent startServer() calls are serialized', () => { - // UT-7 requires testing that concurrent startServer() calls for the same environment: - // 1. Second call waits for first to complete - // 2. Only one server process started - // 3. Both calls return same serverInfo - // - // THIS BEHAVIOR IS IMPLEMENTED IN deepnoteServerStarter.node.ts:82-91: - // - // // Wait for any pending operations on this environment to complete - // const pendingOp = this.pendingOperations.get(environmentId); - // if (pendingOp) { - // logger.info(`Waiting for pending operation on environment ${environmentId}...`); - // try { - // await pendingOp; - // } catch { - // // Ignore errors from previous operations - // } - // } - // - // And then tracks new operations at lines 103-114: - // - // // Start the operation and track it - // const operation = this.startServerForEnvironment(...); - // this.pendingOperations.set(environmentId, operation); - // - // try { - // const result = await operation; - // return result; - // } finally { - // // Remove from pending operations when done - // if (this.pendingOperations.get(environmentId) === operation) { - // this.pendingOperations.delete(environmentId); - // } - // } - // - // This ensures INV-8: Only one startServer() operation per environmentId can be in - // flight at a time. The second concurrent call will wait for the first to complete, - // then check if the server is already running (line 94-100) and return the existing - // serverInfo, preventing duplicate server processes and port conflicts. - // - // Creating a unit test for this would require complex async mocking and race condition - // simulation. The implementation's use of pendingOperations map provides the guarantee. - - assert.ok(true, 'UT-7 is validated by implementation using pendingOperations map (INV-8)'); - }); - }); - - // Priority 2 Integration Tests - suite('Priority 2: Integration Tests (IT-2, IT-6)', () => { - test('IT-2: Switch while cells executing is handled by warning flow', () => { - // IT-2 requires: "Switch environment while cells are running" - // Verify: - // 1. Warning shown about executing cells - // 2. Switch completes - // 3. Running cell may fail (acceptable) - // 4. New cells execute on new environment - // - // THIS IS VALIDATED BY IMPLEMENTATION: - // - // 1. User warning in deepnoteEnvironmentsView.ts:542-561: - // - Checks kernel.pendingCells before switch - // - Shows warning dialog to user if cells executing - // - User can proceed or cancel - // - // 2. Logging in deepnoteKernelAutoSelector.node.ts:269-276: - // - Checks kernel.pendingCells during rebuildController - // - Logs warning if cells are executing - // - Proceeds with rebuild (non-blocking) - // - // The implementation allows switches during execution (with warnings) because: - // - Blocking would create a poor user experience - // - Running cells may fail, which is acceptable - // - New cells will use the new environment - // - Controller disposal order (INV-2) ensures no "disposed controller" error - // - // Full integration testing would require: - // - Real notebook with executing cells - // - Real kernel execution - // - Timing-sensitive test (start execution, then immediately switch) - // - Better suited for E2E tests - - assert.ok(true, 'IT-2 is validated by warning implementation and INV-2'); - }); - - test('IT-6: Server start failure during switch should show error to user', () => { - // IT-6 requires: "Environment switch fails due to server error" - // Verify: - // 1. Error shown to user - // 2. Notebook still usable (ideally on old environment A) - // 3. No controller leak - // 4. Can retry switch - // - // CURRENT IMPLEMENTATION BEHAVIOR: - // - // 1. If startServer() fails, the error propagates from ensureKernelSelectedWithConfiguration() - // (deepnoteKernelAutoSelector.node.ts:450-467) - // - // 2. The error is caught and shown to user in the UI layer - // - // 3. Controller handling in rebuildController() (lines 306-315): - // - Old controller is stored before rebuild - // - Old controller is NEVER disposed (even on success) - // - This means notebook can still use old controller for queued executions - // - // POTENTIAL IMPROVEMENT (noted in test plan): - // The test plan identifies this as a gap in "Known Gaps and Future Improvements": - // - "No atomic rollback: If switch fails mid-way, state may be inconsistent" - // - Recommended: "Implement rollback mechanism: Restore old controller if switch fails" - // - // Currently, if server start fails: - // - Old controller is NOT disposed (good - notebook still has a controller) - // - Cached state WAS cleared (lines 279-282) - // - So getSelected() may not return the old controller from cache - // - // RECOMMENDED FUTURE IMPROVEMENT: - // Wrap ensureKernelSelected() in try-catch in rebuildController(): - // - On success: dispose old controller as usual - // - On failure: restore cached state for old controller - // - // For now, this test documents the current behavior and the known limitation. - - assert.ok( - true, - 'IT-6 behavior is partially implemented - error shown, but rollback not implemented (known gap)' - ); - }); - }); - - // REAL TDD Tests - These should FAIL if bugs exist - suite('Bug Detection: Kernel Selection', () => { - test('BUG-1: Should prefer environment-specific kernel over .env kernel', () => { - // REAL TEST: This will FAIL if the wrong kernel is selected - // - // The selectKernelSpec method is now extracted and testable! - - const envId = 'env123'; - const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('.env', '.env Python', 'python'), - createMockKernelSpec(`deepnote-${envId}`, 'Deepnote Environment', 'python'), - createMockKernelSpec('python3', 'Python 3', 'python') - ]; - - const selected = selector.selectKernelSpec(kernelSpecs, envId); - - // CRITICAL ASSERTION: Should select environment-specific kernel, NOT .env - assert.strictEqual( - selected?.name, - `deepnote-${envId}`, - `BUG DETECTED: Selected "${selected?.name}" instead of "deepnote-${envId}"! This would use wrong environment.` - ); - }); - - test('BUG-1b: Current implementation falls back to Python kernel (documents expected behavior)', () => { - // This test documents that the current implementation DOES have fallback logic - // - // EXPECTED BEHAVIOR (current): Fall back to generic Python kernel when env-specific kernel not found - // This is a design decision - we don't want to block users if the environment-specific kernel isn't ready yet - - const envId = 'env123'; - const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('.env', '.env Python', 'python'), - createMockKernelSpec('python3', 'Python 3', 'python') - ]; - - // Should fall back to a Python kernel (this is the current behavior) - const selected = selector.selectKernelSpec(kernelSpecs, envId); - - // Should have selected a fallback kernel (either .env or python3) - assert.ok(selected, 'Should select a fallback kernel'); - assert.strictEqual(selected.language, 'python', 'Fallback should be a Python kernel'); - }); - - test('Kernel selection: Should find environment-specific kernel when it exists', () => { - const envId = 'my-env'; - const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('python3', 'Python 3', 'python'), - createMockKernelSpec(`deepnote-${envId}`, 'My Environment', 'python') - ]; - - const selected = selector.selectKernelSpec(kernelSpecs, envId); - - assert.strictEqual(selected?.name, `deepnote-${envId}`); - }); - - test('Kernel selection: Should fall back to python3 when env kernel missing', () => { - // Documents current fallback behavior - falls back to python3 when env kernel missing - const envId = 'my-env'; - const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('python3', 'Python 3', 'python'), - createMockKernelSpec('javascript', 'JavaScript', 'javascript') - ]; - - // Should fall back to python3 (current behavior) - const selected = selector.selectKernelSpec(kernelSpecs, envId); - - assert.strictEqual(selected.name, 'python3', 'Should fall back to python3'); - }); - }); - - suite('Bug Detection: Controller Disposal', () => { - test('BUG-2: Old controller is NOT disposed to prevent queued execution errors', async () => { - // This test documents the fix for the DISPOSED error - // - // SCENARIO: User switches environments and has queued cell executions - // - // THE FIX: We do NOT dispose the old controller at all (lines 306-315) - // - Line 281: notebookControllers.delete(notebookKey) removes controller from cache - // - Lines 306-315: Old controller is left alive (NOT disposed) - // - VS Code may have queued cell executions that reference the old controller - // - Those executions will complete successfully using the old controller - // - New executions will use the new controller (it's now preferred) - // - The old controller will be garbage collected when no longer referenced - // - // This prevents the "notebook controller is DISPOSED" error that happened when: - // 1. User queues cell execution (references old controller) - // 2. User switches environments (creates new controller, disposes old one) - // 3. Queued execution tries to run (BOOM - old controller is disposed) - - assert.ok(true, 'Old controller is never disposed - prevents DISPOSED errors for queued executions'); - }); - - test.skip('BUG-2b: Old controller should only be disposed AFTER new controller is in cache', async () => { - // This test is skipped because _testOnly_setController method doesn't exist in the implementation - // REAL TEST: This will FAIL if disposal happens too early - // - // Setup: Create a scenario where we have an old controller and create a new one - const baseFileUri = mockNotebook.uri.with({ query: '', fragment: '' }); - // const notebookKey = baseFileUri.fsPath; - const newEnv = createMockEnvironment('env-new', 'New Environment', true); - - // Track call order - const callOrder: string[] = []; - - // Setup old controller that tracks when dispose() is called - const oldController = mock(); - when(oldController.id).thenReturn('deepnote-config-kernel-env-old'); - when(oldController.controller).thenReturn({} as any); - when(oldController.dispose()).thenCall(() => { - callOrder.push('OLD_CONTROLLER_DISPOSED'); - return undefined; - }); - - // CRITICAL: Use test helper to set up initial controller in cache - // This simulates the state where a controller already exists before environment switch - // selector._testOnly_setController(notebookKey, instance(oldController)); - - // Setup new controller - const newController = mock(); - when(newController.id).thenReturn('deepnote-config-kernel-env-new'); - when(newController.controller).thenReturn({} as any); - - // Setup mocks - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri)).thenReturn('env-new'); - when(mockEnvironmentManager.getEnvironment('env-new')).thenReturn(newEnv); - when(mockPythonExtensionChecker.isPythonExtensionInstalled).thenReturn(true); - - // Mock controller registration to track when new controller is added - when(mockControllerRegistration.addOrUpdate(anything(), anything())).thenCall(() => { - callOrder.push('NEW_CONTROLLER_ADDED_TO_REGISTRATION'); - return [instance(newController)]; - }); - - // CRITICAL TEST: We need to verify that within rebuildController: - // 1. ensureKernelSelected creates and caches new controller (NEW_CONTROLLER_ADDED_TO_REGISTRATION) - // 2. Only THEN is old controller disposed (OLD_CONTROLLER_DISPOSED) - // - // If OLD_CONTROLLER_DISPOSED happens before NEW_CONTROLLER_ADDED_TO_REGISTRATION, - // then there's a window where no valid controller exists! - - try { - await selector.rebuildController(mockNotebook); - } catch { - // Expected to fail due to mocking complexity - } - - // ASSERTION: If implementation is correct, call order should be: - // 1. NEW_CONTROLLER_ADDED_TO_REGISTRATION (from ensureKernelSelected) - // 2. OLD_CONTROLLER_DISPOSED (from rebuildController after new controller is ready) - // - // This test will FAIL if: - // - dispose() is called before new controller is registered - // - or if dispose() is never called - - if (callOrder.length > 0) { - const newControllerIndex = callOrder.indexOf('NEW_CONTROLLER_ADDED_TO_REGISTRATION'); - const oldDisposeIndex = callOrder.indexOf('OLD_CONTROLLER_DISPOSED'); - - if (newControllerIndex !== -1 && oldDisposeIndex !== -1) { - assert.ok( - newControllerIndex < oldDisposeIndex, - `BUG DETECTED: Old controller disposed before new controller was registered! Order: ${callOrder.join( - ' -> ' - )}` - ); - } else { - // This is OK - test might not have reached disposal due to mocking limitations - assert.ok(true, 'Test did not reach disposal phase due to mocking complexity'); - } - } else { - assert.ok(true, 'Test did not capture call order due to mocking complexity'); - } - }); - }); -}); - -/** - * Helper function to create mock environments - */ -function createMockEnvironment(id: string, name: string, hasServer: boolean = false): DeepnoteEnvironment { - const mockPythonInterpreter: PythonEnvironment = { - id: `/usr/bin/python3`, - uri: Uri.parse(`/usr/bin/python3`) - }; - - return { - id, - name, - description: `Test environment ${name}`, - pythonInterpreter: mockPythonInterpreter, - venvPath: Uri.file(`/test/venvs/${id}`), - packages: [], - createdAt: new Date(), - lastUsedAt: new Date(), - serverInfo: hasServer - ? { - url: `http://localhost:8888`, - jupyterPort: 8888, - lspPort: 8889, - token: 'test-token' - } - : undefined - }; -} - -/** - * Helper function to create mock kernel specs - */ -function createMockKernelSpec(name: string, displayName: string, language: string): IJupyterKernelSpec { - return { - name, - display_name: displayName, - language, - executable: '/usr/bin/python3', - argv: ['python3', '-m', 'ipykernel_launcher', '-f', '{connection_file}'] - }; -} diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index d448ec6fda..bf9ba6186b 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -58,10 +58,7 @@ import { IDeepnoteToolkitInstaller, IDeepnoteServerStarter, IDeepnoteKernelAutoSelector, - IDeepnoteServerProvider, - IDeepnoteEnvironmentManager, - IDeepnoteEnvironmentPicker, - IDeepnoteNotebookEnvironmentMapper + IDeepnoteServerProvider } from '../kernels/deepnote/types'; import { DeepnoteToolkitInstaller } from '../kernels/deepnote/deepnoteToolkitInstaller.node'; import { DeepnoteServerStarter } from '../kernels/deepnote/deepnoteServerStarter.node'; @@ -69,17 +66,10 @@ import { DeepnoteKernelAutoSelector } from './deepnote/deepnoteKernelAutoSelecto import { DeepnoteServerProvider } from '../kernels/deepnote/deepnoteServerProvider.node'; import { DeepnoteInitNotebookRunner, IDeepnoteInitNotebookRunner } from './deepnote/deepnoteInitNotebookRunner.node'; import { DeepnoteRequirementsHelper, IDeepnoteRequirementsHelper } from './deepnote/deepnoteRequirementsHelper.node'; -import { DeepnoteEnvironmentManager } from '../kernels/deepnote/environments/deepnoteEnvironmentManager.node'; -import { DeepnoteEnvironmentStorage } from '../kernels/deepnote/environments/deepnoteEnvironmentStorage.node'; -import { DeepnoteEnvironmentsView } from '../kernels/deepnote/environments/deepnoteEnvironmentsView.node'; -import { DeepnoteEnvironmentsActivationService } from '../kernels/deepnote/environments/deepnoteEnvironmentsActivationService'; -import { DeepnoteEnvironmentPicker } from '../kernels/deepnote/environments/deepnoteEnvironmentPicker'; -import { DeepnoteNotebookEnvironmentMapper } from '../kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node'; import { DeepnoteNotebookCommandListener } from './deepnote/deepnoteNotebookCommandListener'; import { DeepnoteInputBlockCellStatusBarItemProvider } from './deepnote/deepnoteInputBlockCellStatusBarProvider'; import { SqlIntegrationStartupCodeProvider } from './deepnote/integrations/sqlIntegrationStartupCodeProvider'; import { DeepnoteCellCopyHandler } from './deepnote/deepnoteCellCopyHandler'; -import { DeepnoteEnvironmentTreeDataProvider } from '../kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node'; import { OpenInDeepnoteHandler } from './deepnote/openInDeepnoteHandler.node'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { @@ -194,28 +184,6 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea DeepnoteInputBlockCellStatusBarItemProvider ); - // Deepnote configuration services - serviceManager.addSingleton(DeepnoteEnvironmentStorage, DeepnoteEnvironmentStorage); - serviceManager.addSingleton(IDeepnoteEnvironmentManager, DeepnoteEnvironmentManager); - serviceManager.addSingleton( - DeepnoteEnvironmentTreeDataProvider, - DeepnoteEnvironmentTreeDataProvider - ); - - // Deepnote configuration view - serviceManager.addSingleton(DeepnoteEnvironmentsView, DeepnoteEnvironmentsView); - serviceManager.addSingleton( - IExtensionSyncActivationService, - DeepnoteEnvironmentsActivationService - ); - - // Deepnote configuration selection - serviceManager.addSingleton(IDeepnoteEnvironmentPicker, DeepnoteEnvironmentPicker); - serviceManager.addSingleton( - IDeepnoteNotebookEnvironmentMapper, - DeepnoteNotebookEnvironmentMapper - ); - // File export/import serviceManager.addSingleton(IFileConverter, FileConverter); serviceManager.addSingleton(ExportInterpreterFinder, ExportInterpreterFinder); diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts index 6bc6fa537c..0e65f409bd 100644 --- a/src/platform/common/utils/localize.ts +++ b/src/platform/common/utils/localize.ts @@ -39,7 +39,7 @@ export namespace Experiments { export const inGroup = (groupName: string) => l10n.t("User belongs to experiment group '{0}'", groupName); } export namespace OutputChannelNames { - export const jupyter = l10n.t('Deepnote'); + export const jupyter = l10n.t('Jupyter'); } export namespace Logging { @@ -706,7 +706,7 @@ export namespace DataScience { export const cellAtFormat = (filePath: string, lineNumber: number) => l10n.t('{0} Cell {1}', filePath, lineNumber); - export const jupyterServerConsoleOutputChannel = l10n.t(`Deepnote Server Console`); + export const jupyterServerConsoleOutputChannel = l10n.t(`Jupyter Server Console`); export const kernelConsoleOutputChannel = (kernelName: string) => l10n.t(`{0} Kernel Console Output`, kernelName); export const webNotSupported = l10n.t(`Operation not supported in web version of Jupyter Extension.`); diff --git a/src/platform/logging/index.ts b/src/platform/logging/index.ts index 4b34aedb07..2107613080 100644 --- a/src/platform/logging/index.ts +++ b/src/platform/logging/index.ts @@ -64,7 +64,6 @@ export function initializeLoggers(options: { }) ); const standardOutputChannel = window.createOutputChannel(OutputChannelNames.jupyter, 'log'); - standardOutputChannel.show(true); // Show by default without stealing focus registerLogger(new OutputChannelLogger(standardOutputChannel, options?.homePathRegEx, options?.userNameRegEx)); if (options.addConsoleLogger) {