Skip to content

Commit 78923a2

Browse files
authored
fix: Prompt the user to select an environment when running one or more blocks. (#310)
1 parent 8b6c018 commit 78923a2

5 files changed

Lines changed: 489 additions & 96 deletions

File tree

CLAUDE.md

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
- 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 `/`)
66
- Follow established patterns, especially when importing new packages (e.g. instead of importing uuid directly, use the helper `import { generateUuid } from '../platform/common/uuid';`)
77

8-
98
## Code conventions
109

1110
- Always run `npx prettier` before committing
@@ -19,7 +18,6 @@
1918
- Tests run against compiled JavaScript files in `out/` directory
2019
- Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties
2120

22-
2321
## Project Structure
2422

2523
- VSCode extension for Jupyter notebooks
@@ -40,3 +38,42 @@
4038
- Whitespace is good for readability, add a blank line after const groups and before return statements
4139
- Separate third-party and local file imports
4240
- How the extension works is described in @specs/architecture.md
41+
42+
## Best Practices
43+
44+
### Resource Cleanup
45+
46+
- Always dispose `CancellationTokenSource` - never create inline without storing/disposing
47+
- Use try/finally to ensure cleanup:
48+
```typescript
49+
const cts = new CancellationTokenSource();
50+
try {
51+
await fn(cts.token);
52+
} finally {
53+
cts.dispose();
54+
}
55+
```
56+
57+
### DRY Principle
58+
59+
- Extract duplicate logic into helper methods to prevent drift
60+
- When similar logic appears in multiple places (e.g., placeholder controller setup, interpreter validation), consolidate it
61+
62+
### Magic Numbers
63+
64+
- Extract magic numbers (retry counts, delays, timeouts) as named constants near the top of the module
65+
66+
### Error Handling
67+
68+
- Use per-iteration error handling in loops - wrap each iteration in try/catch so one failure doesn't stop the rest
69+
- Handle `withProgress` cancellation gracefully - it throws when user cancels, so wrap in try/catch and return appropriate value
70+
71+
### State Validation
72+
73+
- Verify state after async setup operations - methods can return early without throwing, so check expected state was created
74+
- Validate cached state before early returns - before returning "already configured", verify the state is still valid (e.g., interpreter paths match, controllers aren't stale)
75+
76+
### Cancellation Tokens
77+
78+
- Use real cancellation tokens tied to lifecycle events instead of fake/never-cancelled tokens
79+
- Create `CancellationTokenSource` tied to relevant events (e.g., notebook close, cell cancel)

src/kernels/deepnote/types.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,24 @@ export interface IDeepnoteServerProvider {
210210

211211
export const IDeepnoteKernelAutoSelector = Symbol('IDeepnoteKernelAutoSelector');
212212
export interface IDeepnoteKernelAutoSelector {
213+
/**
214+
* Clear the controller selection for a notebook using a specific environment.
215+
* This is used when deleting an environment to unselect its controller from any open notebooks.
216+
* @param notebook The notebook document
217+
* @param environmentId The environment ID
218+
*/
219+
clearControllerForEnvironment(notebook: vscode.NotebookDocument, environmentId: string): void;
220+
221+
/**
222+
* Ensure an environment is configured for the notebook before execution.
223+
* If not configured, shows picker and sets up the kernel.
224+
* @returns true if environment is ready, false if user cancelled
225+
*/
226+
ensureEnvironmentConfiguredBeforeExecution(
227+
notebook: vscode.NotebookDocument,
228+
token: vscode.CancellationToken
229+
): Promise<boolean>;
230+
213231
/**
214232
* Automatically selects and starts a Deepnote kernel for the given notebook.
215233
* @param notebook The notebook document
@@ -221,6 +239,13 @@ export interface IDeepnoteKernelAutoSelector {
221239
token: vscode.CancellationToken
222240
): Promise<boolean>;
223241

242+
/**
243+
* Handle kernel selection errors with user-friendly messages and actions
244+
* @param error The error to handle
245+
* @param notebook The notebook document associated with the error
246+
*/
247+
handleKernelSelectionError(error: unknown, notebook: vscode.NotebookDocument): Promise<void>;
248+
224249
/**
225250
* Force rebuild the controller for a notebook by clearing cached controller and metadata.
226251
* This is used when switching environments to ensure a new controller is created.
@@ -232,21 +257,6 @@ export interface IDeepnoteKernelAutoSelector {
232257
progress: { report(value: { message?: string; increment?: number }): void },
233258
token: vscode.CancellationToken
234259
): Promise<void>;
235-
236-
/**
237-
* Clear the controller selection for a notebook using a specific environment.
238-
* This is used when deleting an environment to unselect its controller from any open notebooks.
239-
* @param notebook The notebook document
240-
* @param environmentId The environment ID
241-
*/
242-
clearControllerForEnvironment(notebook: vscode.NotebookDocument, environmentId: string): void;
243-
244-
/**
245-
* Handle kernel selection errors with user-friendly messages and actions
246-
* @param error The error to handle
247-
* @param notebook The notebook document associated with the error
248-
*/
249-
handleKernelSelectionError(error: unknown, notebook: vscode.NotebookDocument): Promise<void>;
250260
}
251261

252262
export const IDeepnoteEnvironmentManager = Symbol('IDeepnoteEnvironmentManager');

src/notebooks/controllers/vscodeNotebookController.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { KernelMessage } from '@jupyterlab/services';
66
import type { IAnyMessageArgs } from '@jupyterlab/services/lib/kernel/kernel';
77
import {
88
CancellationError,
9+
CancellationTokenSource,
910
commands,
1011
Disposable,
1112
EventEmitter,
@@ -88,6 +89,7 @@ import { KernelConnector } from './kernelConnector';
8889
import { RemoteKernelReconnectBusyIndicator } from './remoteKernelReconnectBusyIndicator';
8990
import { IConnectionDisplayData, IConnectionDisplayDataProvider, IVSCodeNotebookController } from './types';
9091
import { notebookPathToDeepnoteProjectFilePath } from '../../platform/deepnote/deepnoteProjectUtils';
92+
import { DEEPNOTE_NOTEBOOK_TYPE, IDeepnoteKernelAutoSelector } from '../../kernels/deepnote/types';
9193

9294
/**
9395
* Our implementation of the VSCode Notebook Controller. Called by VS code to execute cells in a notebook. Also displayed
@@ -435,6 +437,32 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont
435437
if (!workspace.isTrusted) {
436438
return;
437439
}
440+
441+
// For Deepnote notebooks, ensure environment is configured before execution
442+
if (notebook.notebookType === DEEPNOTE_NOTEBOOK_TYPE) {
443+
const kernelAutoSelector =
444+
this.serviceContainer.tryGet<IDeepnoteKernelAutoSelector>(IDeepnoteKernelAutoSelector);
445+
446+
if (kernelAutoSelector) {
447+
const cts = new CancellationTokenSource();
448+
449+
try {
450+
const hasEnvironment = await kernelAutoSelector.ensureEnvironmentConfiguredBeforeExecution(
451+
notebook,
452+
cts.token
453+
);
454+
455+
if (!hasEnvironment) {
456+
// User cancelled - do not execute
457+
logger.info(`Execution cancelled: no environment selected for ${getDisplayPath(notebook.uri)}`);
458+
return;
459+
}
460+
} finally {
461+
cts.dispose();
462+
}
463+
}
464+
}
465+
438466
logger.debug(`Handle Execution of Cells ${cells.map((c) => c.index)} for ${getDisplayPath(notebook.uri)}`);
439467
await initializeInteractiveOrNotebookTelemetryBasedOnUserAction(notebook.uri, this.connection);
440468
telemetryTracker?.stop();

0 commit comments

Comments
 (0)