Skip to content

Commit dee6066

Browse files
hc2ptkislancoderabbitai[bot]Artmann
authored
feat: Deepnote kernel management view (#52)
* feat: implement Phase 1 of Deepnote kernel configuration management Add core infrastructure for managing Deepnote kernel configurations, enabling future user-controlled kernel lifecycle management. ## What's New ### Core Models & Storage - Create type definitions for kernel configurations with UUID-based identity - Implement persistent storage layer using VS Code globalState - Build configuration manager with full CRUD operations - Add event system for configuration change notifications ### Components Added - `deepnoteKernelConfiguration.ts`: Type definitions and interfaces - `deepnoteConfigurationStorage.ts`: Serialization and persistence - `deepnoteConfigurationManager.ts`: Business logic and lifecycle management ### API Updates - Extend `IDeepnoteToolkitInstaller` with configuration-based methods - Extend `IDeepnoteServerStarter` with configuration-based methods - Maintain backward compatibility with file-based APIs ### Service Registration - Register DeepnoteConfigurationStorage as singleton - Register DeepnoteConfigurationManager with auto-activation - Integrate with existing dependency injection system ## Testing - Add comprehensive unit tests for storage layer (11 tests, all passing) - Add unit tests for configuration manager (29 tests, 22 passing) - 7 tests intentionally failing pending Phase 2 service refactoring ## Documentation - Create KERNEL_MANAGEMENT_VIEW_IMPLEMENTATION.md with complete architecture - Document 8-phase implementation plan - Define migration strategy from file-based to configuration-based system ## Dependencies - Add uuid package for configuration ID generation ## Status Phase 1 complete. Ready for Phase 2 (refactoring existing services). Related: #4913 * feat: add configuration-based APIs to DeepnoteToolkitInstaller and DeepnoteServerStarter Refactor DeepnoteToolkitInstaller and DeepnoteServerStarter to support configuration-based kernel management alongside existing file-based workflow. DeepnoteToolkitInstaller changes: - Add ensureVenvAndToolkit() method for direct venv path management - Add installAdditionalPackages() for installing packages in existing venvs - Add getVenvInterpreterByPath() helper for venv path-based interpreter lookup - Add getKernelSpecName() and getKernelDisplayName() for venv-based naming - Refactor installImpl() to installVenvAndToolkit() using venv paths - Update kernel spec installation to use venv directory names instead of file hashes - Mark ensureInstalled() as deprecated, now delegates to ensureVenvAndToolkit() DeepnoteServerStarter changes: - Add startServer() method accepting venv path and configuration ID - Add stopServer() method accepting configuration ID instead of file URI - Add startServerForConfiguration() implementation for config-based lifecycle - Add stopServerForConfiguration() implementation for config-based cleanup - Mark getOrStartServer() as deprecated for backward compatibility - Update server process tracking to work with configuration IDs as keys These changes enable the kernel configuration manager to create and manage isolated Python environments without requiring .deepnote file associations. Legacy file-based methods are preserved for backward compatibility. Part of Phase 2: Refactoring Existing Services * feat: implement Phase 3 - Tree View UI for kernel configurations Add VS Code tree view interface for managing Deepnote kernel configurations. Changes: - Created DeepnoteConfigurationTreeItem with status-based icons and context values - Created DeepnoteConfigurationTreeDataProvider implementing TreeDataProvider - Created DeepnoteConfigurationsView handling all UI commands: * create: Multi-step wizard for new configurations * start/stop/restart: Server lifecycle management * delete: Configuration removal with confirmation * editName: Rename configurations * managePackages: Update package lists * refresh: Manual tree refresh - Created DeepnoteConfigurationsActivationService for initialization - Registered all services in serviceRegistry.node.ts - Added deepnoteKernelConfigurations view to package.json - Added 8 commands with icons and context menus - Added view/title and view/item/context menu contributions Technical details: - Uses Python API to enumerate available interpreters - Implements progress notifications for long-running operations - Provides input validation for names and package lists - Shows status indicators (Running/Starting/Stopped) with appropriate colors - Displays configuration details (Python path, venv, packages, timestamps) Fixes: - Made DeepnoteConfigurationManager.initialize() public to match interface - Removed unused getDisplayName() method from DeepnoteToolkitInstaller - Added type annotations to all lambda parameters * feat: implement Phase 3 - Tree View UI for kernel configurations Add VS Code tree view interface for managing Deepnote kernel configurations. Changes: - Created DeepnoteConfigurationTreeItem with status-based icons and context values - Created DeepnoteConfigurationTreeDataProvider implementing TreeDataProvider - Created DeepnoteConfigurationsView handling all UI commands: * create: Multi-step wizard for new configurations * start/stop/restart: Server lifecycle management * delete: Configuration removal with confirmation * editName: Rename configurations * managePackages: Update package lists * refresh: Manual tree refresh - Created DeepnoteConfigurationsActivationService for initialization - Registered all services in serviceRegistry.node.ts - Added deepnoteKernelConfigurations view to package.json - Added 8 commands with icons and context menus - Added view/title and view/item/context menu contributions Technical details: - Uses Python API to enumerate available interpreters - Implements progress notifications for long-running operations - Provides input validation for names and package lists - Shows status indicators (Running/Starting/Stopped) with appropriate colors - Displays configuration details (Python path, venv, packages, timestamps) Fixes: - Made DeepnoteConfigurationManager.initialize() public to match interface - Removed unused getDisplayName() method from DeepnoteToolkitInstaller - Added type annotations to all lambda parameters * feat: add Phase 7 Part 1 - configuration picker infrastructure Add infrastructure for notebook-to-configuration mapping and selection UI. Changes: - Created DeepnoteConfigurationPicker for showing configuration selection UI - Created DeepnoteNotebookConfigurationMapper for tracking notebook→config mappings - Added interfaces to types.ts for new services - Registered new services in serviceRegistry.node.ts - Updated KERNEL_MANAGEMENT_VIEW_IMPLEMENTATION.md with: * New components documentation (picker and mapper) * Updated file structure with implementation status * Comprehensive implementation status section Technical details: - Picker shows configurations with status indicators (running/stopped) - Picker includes "Create new" option - Mapper stores selections in workspace state (per-workspace persistence) - Mapper provides bidirectional lookups (notebook→config and config→notebooks) Next steps: - Integrate picker and mapper with DeepnoteKernelAutoSelector - Show picker when notebook opens without selected configuration - Use selected configuration's venv and server instead of auto-creating * refactor: rename "Configuration" to "Environment" for clarity This refactoring improves terminology clarity across the Deepnote kernel management system by renaming "Kernel Configuration" to "Environment". Rationale: - "Configuration" implies kernel settings, but the system manages Python virtual environments (venv + packages + Jupyter server) - "Environment" is more accurate and familiar to developers - Reduces confusion between "configuration" (settings) and "environment" (Python venv) Changes: - Renamed directory: configurations/ → environments/ - Renamed 15 files (classes, interfaces, tests) - Updated types.ts: 6 interface names, 6 symbol constants - Updated package.json: 9 commands, 1 view ID, titles/labels - Updated all import paths and references across codebase - Updated documentation in KERNEL_MANAGEMENT_VIEW_IMPLEMENTATION.md Terminology mapping: - Kernel Configuration → Environment - IDeepnoteConfigurationManager → IDeepnoteEnvironmentManager - DeepnoteConfigurationsView → DeepnoteEnvironmentsView - deepnote.configurations.* → deepnote.environments.* - deepnoteKernelConfigurations view → deepnoteEnvironments view All tests pass, TypeScript compilation successful. * Fixed tests * Ensure propsed APIs are loaded properly this was leading to many error logs in the "Jupyter Outputs" * Improve DX by auto showing "Deepnote" outputs * feat: add environment selection UI for notebooks Added comprehensive UI for selecting and switching environments for Deepnote notebooks, making it easy for users to choose which kernel environment to use. New Features: - Added "Select Environment for Notebook" command in notebook toolbar that shows a quick pick with all available environments - Environment picker now directly triggers the create environment dialog when "Create New" is selected, then automatically re-shows the picker - Environment names are now displayed in kernel connection labels for better visibility (e.g., "Deepnote: Python 3.10") - Shows current environment selection with checkmark in the picker - Displays environment status (Running/Stopped) with icons in picker - Warns users before switching if cells are currently executing Improvements: - Environment picker integration: Clicking "Create New" now launches the full creation dialog instead of just showing an info message - Added rebuildController() to IDeepnoteKernelAutoSelector interface for switching environments - Updated test mocks to include new dependencies UI/UX: - Added notebook toolbar button with server-environment icon - Quick pick shows environment details: interpreter path, packages, status - Graceful handling of edge cases (no environments, same environment selected) - Clear progress notifications during environment switching * fix: resolve environment switching issues with stale server connections Fixed three critical bugs that prevented proper environment switching: 1. Controller disposal race condition: Old controller was disposed before the new controller was ready, causing "DISPOSED" errors during cell execution. Fixed by deferring disposal until after new controller is fully registered. 2. Stale configuration caching: Configuration object wasn't refreshed after startServer(), so we connected with outdated serverInfo. Fixed by always calling getEnvironment() after startServer() to get current server info. 3. Environment manager early return: startServer() had an early return when config.serverInfo was set, preventing verification that the server was actually running. This caused connections to wrong/stale servers when switching TO a previously-used environment. Fixed by always calling serverStarter.startServer() (which is idempotent) to ensure current info. Additional improvements: - Made kernel spec installation idempotent and ensured it runs when reusing existing venvs - Removed legacy auto-create fallback path that's no longer needed - Added proper TypeScript non-null assertions after server info validation * WIP: Environment switching with controller disposal workaround This commit implements environment switching but with a known limitation regarding controller disposal and queued cell executions. ## Changes ### Port Allocation Refactoring - Refactored DeepnoteServerProvider to handle both jupyterPort and lspPort - Updated DeepnoteServerStarter to allocate both ports - Updated DeepnoteEnvironmentManager to store complete serverInfo - All port-related code now consistently uses the dual-port model ### Kernel Selection Logic Extraction - Extracted kernel selection logic into public `selectKernelSpec()` method - Added 4 unit tests for kernel selection (all passing) - Method is now testable and validates environment-specific kernel preference - Falls back to generic Python kernels when env-specific kernel not found ### Environment Switching Implementation - Added environment switching via tree view context menu - Switching calls `rebuildController()` to create new controller - Shows warning dialog if cells are currently executing - Updates notebook-to-environment mapping on switch ## Known Issue: Controller Disposal Problem When switching environments, we encountered a critical "notebook controller is DISPOSED" error. The issue occurs because: 1. User queues cell execution (VS Code references current controller) 2. User switches environments 3. New controller created and marked as preferred 4. Old controller disposed 5. Queued execution tries to run 5+ seconds later → DISPOSED error ### Current Workaround (Implemented) We do NOT dispose old controllers at all. Instead: - Old controllers stay alive to handle queued executions - New controller is marked as "Preferred" for new executions - Garbage collection cleans up eventually ### Why This Is Not Ideal - Potential memory leak with many switches - No guarantee new executions use new controller - Users might execute on wrong environment after switch - VS Code controller selection not fully deterministic ### What We Tried 1. Adding delay before disposal → Failed (timing unpredictable) 2. Disposing after setting preference → Failed 3. Never disposing → Prevents error but suboptimal ### Proper Solution Needed May require VS Code API changes to: - Force controller selection immediately - Cancel/migrate queued executions - Query if controller has pending executions See TODO.md for full analysis and discussion. ## Testing - ✅ All 40+ unit tests passing - ✅ Kernel selection tests passing (4 new tests) - ✅ Port allocation working correctly - ⚠️ Environment switching works but with limitations above Related files: - src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts:306-315 - src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts:599-635 - src/kernels/deepnote/environments/deepnoteEnvironmentsView.ts:542-561 * WIP trying to get env switching working * refactor: remove legacy ensureInstalled method and clean up environment management This commit removes the deprecated `ensureInstalled` method from the `DeepnoteToolkitInstaller` class, streamlining the installation process by encouraging the use of `ensureVenvAndToolkit`. Additionally, the `logs.txt` file has been deleted as it is no longer necessary for the current implementation. Changes include: - Removal of the legacy method to enhance code clarity and maintainability. - Deletion of the logs file to reduce clutter in the repository. These changes contribute to a cleaner codebase and improved user experience when managing Deepnote environments. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: update Deepnote environment structure and improve path handling This commit refines the Deepnote environment management by updating the structure of the `DeepnoteEnvironmentState` to encapsulate the `pythonInterpreterPath` as an object with `id` and `uri`. Additionally, the `venvBinDir` in `DeepnoteServerStarter` is now derived using `path.dirname()` for better path handling. Changes include: - Refactored `DeepnoteEnvironmentState` to use an object for `pythonInterpreterPath`. - Improved path handling for virtual environment binaries in `DeepnoteServerStarter`. - Updated related tests to reflect the new structure and ensure correctness. These changes enhance code clarity and maintainability while ensuring accurate environment configurations. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * Fix test Signed-off-by: Tomas Kislan <tomas@kislan.sk> * Fix test Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Reformat file Signed-off-by: Tomas Kislan <tomas@kislan.sk> * chore: Update spell check config Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Update deepnote toolkit installation to return toolkit version and improve logging * feat: Localize Deepnote environment command titles and improve logging messages Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Logging improvements, added cancelation token support Signed-off-by: Tomas Kislan <tomas@kislan.sk> * feat: Localize user messages in Deepnote environments view for improved accessibility - Updated error, information, and prompt messages to use localization functions for better internationalization support. - Enhanced user experience by ensuring all relevant messages are translated, including environment creation, deletion, and package management prompts. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * test: Enhance unit tests for DeepnoteEnvironmentsView with editEnvironmentName functionality - Added comprehensive tests for the editEnvironmentName method, covering scenarios such as early returns for non-existent environments, user cancellations, and input validation. - Implemented checks for successful renaming of environments, ensuring existing configurations are preserved. - Updated the environment status icon in the UI to reflect error states accurately. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * chore: Improve localization, improve error handling, other cleanup - Removed outdated `@types/uuid` dependency from `package.json` and `package-lock.json`. - Localized command titles and error messages in the Deepnote environment management components for improved accessibility. - Enhanced error handling in the environment manager and server starter to provide clearer feedback to users. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * feat: Add cancellation token support to startServer method and enhance environment manager, fix some tests - Updated the `startServer` method in `IDeepnoteEnvironmentManager` to accept an optional cancellation token for better control over server startup processes. - Implemented the cancellation token in the `DeepnoteEnvironmentsView` to handle user cancellations during server operations. - Refactored tests to ensure proper handling of dependencies and cancellation scenarios. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Simplify cancellation handling in Deepnote environment manager and improve localization - Replaced manual cancellation checks in the `stopServer` method with a centralized `Cancellation.throwIfCanceled` method for cleaner code. - Enhanced localization in the `DeepnoteEnvironmentTreeItem` class to provide more accurate time-related messages. - Removed unnecessary binding of `IDeepnoteEnvironmentManager` to `IExtensionSyncActivationService` in the service registry. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Enhance Deepnote environment manager with output channel integration and streamlined cancellation handling - Added `IOutputChannel` integration to the `DeepnoteEnvironmentManager` for improved logging of activation errors. - Replaced manual cancellation checks with `Cancellation.throwIfCanceled` for cleaner and more consistent cancellation handling across methods. - Ensured proper disposal of the output channel during resource cleanup. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Update iconPath in DeepnoteEnvironmentTreeItem to use ThemeColor Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Inject DeepnoteEnvironmentTreeDataProvider through DI Signed-off-by: Tomas Kislan <tomas@kislan.sk> * test: Refactor DeepnoteKernelAutoSelector tests Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Enhance error messages in Deepnote environments view and improve cancellation handling Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Minor changes Signed-off-by: Tomas Kislan <tomas@kislan.sk> * Remove unused import Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Fix test Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Fix test Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Remove duplicate import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Update environment status handling in DeepnoteEnvironmentTreeItem Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Update DeepnoteEnvironmentTreeItem to use consistent IDs for info items - Modified createInfoItem method to accept an ID parameter for better identification of info items. - Updated instances of createInfoItem across the DeepnoteEnvironmentTreeDataProvider to use the new ID format. - Enhanced unit tests to reflect changes in info item creation and ensure proper functionality. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * test: Add DeepnoteEnvironmentsView tests Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Enhance DeepnoteEnvironmentTreeDataProvider to include environment ID in info items - Updated the creation of info items in DeepnoteEnvironmentTreeDataProvider to include the environment ID for better identification. - Modified the createInfoItem method in DeepnoteEnvironmentTreeItem to accept an environment ID, ensuring consistent ID formatting. - Adjusted unit tests to validate the changes in info item creation and ensure proper functionality. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * feat: Add clearControllerForEnvironment method and disposeKernelsUsingEnvironment logic - Introduced clearControllerForEnvironment method in IDeepnoteKernelAutoSelector to unselect the controller for a notebook when an environment is deleted. - Implemented disposeKernelsUsingEnvironment method in DeepnoteEnvironmentsView to dispose of kernels from open notebooks using the deleted environment. - Enhanced unit tests for DeepnoteEnvironmentsView to verify kernel disposal behavior when an environment is deleted. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Merge package.json extension menus Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Pass cancellation token to startServer Signed-off-by: Tomas Kislan <tomas@kislan.sk> * Remove unnecessary check Signed-off-by: Tomas Kislan <tomas@kislan.sk> * Reorder toolbar commands Signed-off-by: Tomas Kislan <tomas@kislan.sk> * test: Add more simple tests, to increase coverage of new code Signed-off-by: Tomas Kislan <tomas@kislan.sk> * Update spell check config Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Consolidate server management tests in DeepnoteEnvironmentsView Signed-off-by: Tomas Kislan <tomas@kislan.sk> * refactor: Simplify environment selection logic in DeepnoteEnvironmentPicker and DeepnoteEnvironmentsView - Removed redundant checks for existing environments and streamlined the user prompts for creating new environments. - Updated the createEnvironmentCommand to return the created environment for better handling in the environment selection process. - Enhanced logging for environment switching and user interactions. This refactor improves the clarity and efficiency of the environment management workflow. Signed-off-by: Tomas Kislan <tomas@kislan.sk> * fix: Fix tensorflow typo Signed-off-by: Tomas Kislan <tomas@kislan.sk> --------- Signed-off-by: Tomas Kislan <tomas@kislan.sk> Co-authored-by: Tomas Kislan <tomas@kislan.sk> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Christoffer Artmann <Artgaard@gmail.com>
1 parent b7065eb commit dee6066

36 files changed

+6548
-664
lines changed

.vscode/launch.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"runtimeExecutable": "${execPath}",
1010
"args": [
1111
"--extensionDevelopmentPath=${workspaceFolder}",
12-
"--enable-proposed-api"
12+
"--enable-proposed-api=Deepnote.vscode-deepnote"
1313
],
1414
"smartStep": true,
1515
"sourceMaps": true,

CLAUDE.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,34 @@
11
## Code Style & Organization
2+
23
- Order method, fields and properties, first by accessibility and then by alphabetical order.
34
- Don't add the Microsoft copyright header to new files.
45
- 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 `/`)
6+
- Follow established patterns, especially when importing new packages (e.g. instead of importing uuid directly, use the helper `import { generateUuid } from '../platform/common/uuid';`)
7+
8+
9+
## Code conventions
10+
11+
- Always run `npx prettier` before committing
512

613
## Testing
14+
715
- Unit tests use Mocha/Chai framework with `.unit.test.ts` extension
816
- Test files should be placed alongside the source files they test
917
- Run all tests: `npm test` or `npm run test:unittests`
1018
- Run single test file: `npx mocha --config ./build/.mocha.unittests.js.json ./out/path/to/file.unit.test.js`
1119
- Tests run against compiled JavaScript files in `out/` directory
1220
- Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties
1321

22+
1423
## Project Structure
24+
1525
- VSCode extension for Jupyter notebooks
1626
- Uses dependency injection with inversify
1727
- Follows separation of concerns pattern
1828
- TypeScript codebase that compiles to `out/` directory
1929

2030
## Deepnote Integration
31+
2132
- Located in `src/notebooks/deepnote/`
2233
- Refactored architecture:
2334
- `deepnoteTypes.ts` - Type definitions
@@ -28,4 +39,4 @@
2839
- `deepnoteActivationService.ts` - VSCode activation
2940
- Whitespace is good for readability, add a blank line after const groups and before return statements
3041
- Separate third-party and local file imports
31-
- How the extension works is described in @architecture.md
42+
- How the extension works is described in @architecture.md

0 commit comments

Comments
 (0)