-
Notifications
You must be signed in to change notification settings - Fork 299
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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'; | ||
|
@@ -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>(); | ||
|
||
|
@@ -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); | ||
|
@@ -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); | ||
|
@@ -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); | ||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will work until ipykernel gets to version 10 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OMG, thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'); | ||
|
There was a problem hiding this comment.
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.