Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the kernel to check for ipykernel 6 version, instead of the python installer #7689

Merged
merged 1 commit into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/7576.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use the kernel to check for ipykernel 6 version, instead of the python installer. This works for both local and remote.
4 changes: 2 additions & 2 deletions src/client/datascience/jupyter/kernels/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,12 @@ export class Kernel implements IKernel {
await promise;
return promise;
}
public async executeHidden(code: string) {
public async executeHidden(code: string): Promise<nbformat.IOutput[]> {
const stopWatch = new StopWatch();
const notebookPromise = this.startNotebook();
const promise = notebookPromise.then((nb) => executeSilently(nb.session, code));
this.trackNotebookCellPerceivedColdTime(stopWatch, notebookPromise, promise).catch(noop);
await promise;
return promise;
}
public async start(options: { disableUI?: boolean } = {}): Promise<void> {
await this.startNotebook(options);
Expand Down
3 changes: 2 additions & 1 deletion src/client/datascience/jupyter/kernels/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
InterruptResult,
KernelSocketInformation
} from '../../types';
import type { nbformat } from '@jupyterlab/coreutils';

export type LiveKernelModel = IJupyterKernel & Partial<IJupyterKernelSpec> & { session: Session.IModel };

Expand Down Expand Up @@ -146,7 +147,7 @@ export interface IKernel extends IAsyncDisposable {
interrupt(): Promise<InterruptResult>;
restart(): Promise<void>;
executeCell(cell: NotebookCell): Promise<NotebookCellRunState>;
executeHidden(code: string): Promise<void>;
executeHidden(code: string): Promise<nbformat.IOutput[]>;
}

export type KernelOptions = {
Expand Down
62 changes: 28 additions & 34 deletions src/client/debugger/jupyter/debuggingManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@ import {
DebugSession,
NotebookCell,
DebugSessionOptions,
ProgressLocation,
DebugAdapterDescriptor,
Event,
EventEmitter,
NotebookEditor
} from 'vscode';
import * as path from 'path';
import { IKernel, IKernelProvider } from '../../datascience/jupyter/kernels/types';
import { IConfigurationService, IDisposable, Product, ProductInstallStatus } from '../../common/types';
import { IConfigurationService, IDisposable } from '../../common/types';
import { KernelDebugAdapter } from './kernelDebugAdapter';
import { INotebookProvider } from '../../datascience/types';
import { IExtensionSingleActivationService } from '../../activation/types';
Expand All @@ -35,9 +34,7 @@ import { Commands as DSCommands } from '../../datascience/constants';
import { IFileSystem } from '../../common/platform/types';
import { IDebuggingManager, IKernelDebugAdapterConfig, KernelDebugMode } from '../types';
import { DebuggingTelemetry, pythonKernelDebugAdapter } from '../constants';
import { IPythonInstaller } from '../../api/types';
import { sendTelemetryEvent } from '../../telemetry';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { DebugCellController, RunByLineController } from './debugControllers';
import { assertIsDebugConfig } from './helper';
import { Debugger } from './debugger';
Expand All @@ -53,7 +50,6 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb
private notebookToDebugAdapter = new Map<NotebookDocument, KernelDebugAdapter>();
private notebookToRunByLineController = new Map<NotebookDocument, RunByLineController>();
private notebookInProgress = new Set<NotebookDocument>();
private cache = new Map<PythonEnvironment, boolean>();
private readonly disposables: IDisposable[] = [];
private _doneDebugging = new EventEmitter<void>();

Expand All @@ -65,7 +61,6 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook,
@inject(IFileSystem) private fs: IFileSystem,
@inject(IPythonInstaller) private pythonInstaller: IPythonInstaller,
@inject(IConfigurationService) private settings: IConfigurationService
) {
this.debuggingInProgress = new ContextKey(EditorContexts.DebuggingInProgress, this.commandManager);
Expand Down Expand Up @@ -245,12 +240,7 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb

try {
this.notebookInProgress.add(editor.document);
if (
await this.checkForIpykernel6(
editor.document,
mode === KernelDebugMode.RunByLine ? DataScience.startingRunByLine() : undefined
)
) {
if (await this.checkForIpykernel6(editor.document)) {
switch (mode) {
case KernelDebugMode.Everything:
await this.startDebugging(editor.document);
Expand Down Expand Up @@ -421,31 +411,35 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb
return kernel;
}

private async checkForIpykernel6(doc: NotebookDocument, waitingMessage?: string): Promise<boolean> {
private async checkForIpykernel6(doc: NotebookDocument): Promise<boolean> {
try {
const controller = this.notebookControllerManager.getSelectedNotebookController(doc);
const interpreter = controller?.connection.interpreter;
if (interpreter) {
const cacheResult = this.cache.get(interpreter);
if (cacheResult === true) {
return true;
let kernel = this.kernelProvider.get(doc);

if (!kernel) {
const controller = this.notebookControllerManager.getSelectedNotebookController(doc);

if (controller) {
kernel = this.kernelProvider.getOrCreate(doc, {
metadata: controller.connection,
controller: controller?.controller,
resourceUri: doc.uri
});
}
}

const checkCompatible = () =>
this.pythonInstaller.isProductVersionCompatible(Product.ipykernel, '>=6.0.0', interpreter);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for caching or the python installer anymore, this is just faster.

const status = waitingMessage
? await this.appShell.withProgress(
{ location: ProgressLocation.Notification, title: waitingMessage },
checkCompatible
)
: await checkCompatible();
const result = status === ProductInstallStatus.Installed;

sendTelemetryEvent(DebuggingTelemetry.ipykernel6Status, undefined, {
status: result ? 'installed' : 'notInstalled'
});
this.cache.set(interpreter, result);
return result;
if (kernel) {
const code = 'import ipykernel\nprint(ipykernel.__version__)';
const output = await kernel.executeHidden(code);

if (output[0].text) {
const majorVersion = Number(output[0].text.toString().charAt(0));
const result = majorVersion >= 6;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will work until ipykernel gets to version 10

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG, thanks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in #7695


sendTelemetryEvent(DebuggingTelemetry.ipykernel6Status, undefined, {
status: result ? 'installed' : 'notInstalled'
});
return result;
}
}
} catch {
traceError('Debugging: Could not check for ipykernel 6');
Expand Down