Skip to content

Commit 0dafe04

Browse files
committed
refactor: Update kernel selection and environment handling in Deepnote
- Modified the `ensureKernelSelected` method to include progress reporting and cancellation token parameters for improved user feedback during kernel selection. - Enhanced the `selectEnvironmentForNotebook` method to accept a notebook parameter, ensuring the correct notebook context is used when selecting environments. - Updated the environment selection command to handle active notebooks more robustly, providing warnings when no Deepnote notebook is found. - Refactored logging to provide clearer insights into notebook state changes and environment selection processes. These changes improve the user experience by providing better feedback and ensuring that the correct notebook context is maintained during operations. Signed-off-by: Tomas Kislan <[email protected]>
1 parent c4d241b commit 0dafe04

File tree

6 files changed

+170
-101
lines changed

6 files changed

+170
-101
lines changed

src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export class DeepnoteEnvironmentManager implements IExtensionSyncActivationServi
189189

190190
config.lastUsedAt = new Date();
191191
await this.persistEnvironments();
192+
this._onDidChangeEnvironments.fire();
192193
}
193194

194195
/**

src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
import { inject, injectable } from 'inversify';
2-
import { commands, Disposable, l10n, ProgressLocation, QuickPickItem, TreeView, window, workspace } from 'vscode';
2+
import {
3+
commands,
4+
Disposable,
5+
l10n,
6+
NotebookDocument,
7+
ProgressLocation,
8+
QuickPickItem,
9+
TreeView,
10+
window,
11+
workspace
12+
} from 'vscode';
313
import { IDisposableRegistry } from '../../../platform/common/types';
414
import { logger } from '../../../platform/logging';
515
import { IPythonApiProvider } from '../../../platform/api/types';
@@ -243,9 +253,19 @@ export class DeepnoteEnvironmentsView implements Disposable {
243253

244254
// Switch environment for notebook command
245255
this.disposables.push(
246-
commands.registerCommand('deepnote.environments.selectForNotebook', async () => {
247-
await this.selectEnvironmentForNotebook();
248-
})
256+
commands.registerCommand(
257+
'deepnote.environments.selectForNotebook',
258+
async (options?: { notebook?: NotebookDocument }) => {
259+
// Get the active notebook
260+
const activeNotebook = options?.notebook ?? window.activeNotebookEditor?.notebook;
261+
if (!activeNotebook || activeNotebook.notebookType !== 'deepnote') {
262+
void window.showWarningMessage(l10n.t('No active Deepnote notebook found'));
263+
return;
264+
}
265+
266+
await this.selectEnvironmentForNotebook({ notebook: activeNotebook });
267+
}
268+
)
249269
);
250270
}
251271

@@ -344,16 +364,11 @@ export class DeepnoteEnvironmentsView implements Disposable {
344364
}
345365
}
346366

347-
public async selectEnvironmentForNotebook(): Promise<void> {
348-
// Get the active notebook
349-
const activeNotebook = window.activeNotebookEditor?.notebook;
350-
if (!activeNotebook || activeNotebook.notebookType !== 'deepnote') {
351-
void window.showWarningMessage(l10n.t('No active Deepnote notebook found'));
352-
return;
353-
}
367+
public async selectEnvironmentForNotebook({ notebook }: { notebook: NotebookDocument }): Promise<void> {
368+
logger.info('Selecting environment for notebook:', notebook);
354369

355370
// Get base file URI (without query/fragment)
356-
const baseFileUri = activeNotebook.uri.with({ query: '', fragment: '' });
371+
const baseFileUri = notebook.uri.with({ query: '', fragment: '' });
357372

358373
// Get current environment selection
359374
const currentEnvironmentId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri);
@@ -421,7 +436,7 @@ export class DeepnoteEnvironmentsView implements Disposable {
421436

422437
// Check if any cells are currently executing using the kernel execution state
423438
// This is more reliable than checking executionSummary
424-
const kernel = this.kernelProvider.get(activeNotebook);
439+
const kernel = this.kernelProvider.get(notebook);
425440
const hasExecutingCells = kernel
426441
? this.kernelProvider.getKernelExecution(kernel).pendingCells.length > 0
427442
: false;
@@ -443,7 +458,7 @@ export class DeepnoteEnvironmentsView implements Disposable {
443458
}
444459

445460
// User selected a different environment - switch to it
446-
logger.info(`Switching notebook ${getDisplayPath(activeNotebook.uri)} to environment ${selectedEnvironmentId}`);
461+
logger.info(`Switching notebook ${getDisplayPath(notebook.uri)} to environment ${selectedEnvironmentId}`);
447462

448463
try {
449464
await window.withProgress(
@@ -459,7 +474,7 @@ export class DeepnoteEnvironmentsView implements Disposable {
459474
// Force rebuild the controller with the new environment
460475
// This clears cached metadata and creates a fresh controller.
461476
// await this.kernelAutoSelector.ensureKernelSelected(activeNotebook);
462-
await this.kernelAutoSelector.rebuildController(activeNotebook);
477+
await this.kernelAutoSelector.rebuildController(notebook);
463478

464479
logger.info(`Successfully switched to environment ${selectedEnvironmentId}`);
465480
}

src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { assert } from 'chai';
22
import * as sinon from 'sinon';
33
import { anything, capture, instance, mock, when, verify, deepEqual, resetCalls } from 'ts-mockito';
4-
import { CancellationToken, Disposable, ProgressOptions, Uri } from 'vscode';
4+
import { CancellationToken, Disposable, NotebookDocument, ProgressOptions, Uri } from 'vscode';
55
import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node';
66
import { IDeepnoteEnvironmentManager, IDeepnoteKernelAutoSelector, IDeepnoteNotebookEnvironmentMapper } from '../types';
77
import { IPythonApiProvider } from '../../../platform/api/types';
@@ -660,7 +660,7 @@ suite('DeepnoteEnvironmentsView', () => {
660660
when(mockedVSCodeNamespaces.window.showInformationMessage(anything())).thenResolve(undefined);
661661

662662
// Execute the command
663-
await view.selectEnvironmentForNotebook();
663+
await view.selectEnvironmentForNotebook({ notebook: mockNotebook as NotebookDocument });
664664

665665
// Verify API calls
666666
verify(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri)).once();

src/kernels/deepnote/types.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,11 @@ export interface IDeepnoteKernelAutoSelector {
189189
* @param notebook The notebook document
190190
* @param token Cancellation token to cancel the operation
191191
*/
192-
ensureKernelSelected(notebook: vscode.NotebookDocument, token?: vscode.CancellationToken): Promise<boolean>;
192+
ensureKernelSelected(
193+
notebook: vscode.NotebookDocument,
194+
progress: { report(value: { message?: string; increment?: number }): void },
195+
token: vscode.CancellationToken
196+
): Promise<boolean>;
193197

194198
/**
195199
* Force rebuild the controller for a notebook by clearing cached controller and metadata.

0 commit comments

Comments
 (0)