Skip to content

Route python interpreter into kernel launcher #11486

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.

Sign up for GitHub

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

Merged
IanMatthewHuff merged 2 commits into from
Apr 29, 2020
Merged

Route python interpreter into kernel launcher #11486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49f4164
basic routing working
Apr 28, 2020
78194ac
typo fix
Apr 28, 2020
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
19 changes: 14 additions & 5 deletions src/client/datascience/baseJupyterSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { traceError, traceInfo, traceWarning } from '../common/logger';
import { waitForPromise } from '../common/utils/async';
import * as localize from '../common/utils/localize';
import { noop } from '../common/utils/misc';
import { PythonInterpreter } from '../interpreter/contracts';
import { sendTelemetryEvent } from '../telemetry';
import { Telemetry } from './constants';
import { JupyterKernelPromiseFailedError } from './jupyter/kernels/jupyterKernelPromiseFailedError';
Expand Down Expand Up @@ -41,6 +42,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
return this._session;
}
protected kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined;
protected interpreter: PythonInterpreter | undefined;
public get kernelSocket(): Observable<KernelSocketInformation | undefined> {
return this._kernelSocket;
}
Expand Down Expand Up @@ -118,24 +120,29 @@ export abstract class BaseJupyterSession implements IJupyterSession {
}
}

public async changeKernel(kernel: IJupyterKernelSpec | LiveKernelModel, timeoutMS: number): Promise<void> {
public async changeKernel(
kernel: IJupyterKernelSpec | LiveKernelModel,
timeoutMS: number,
interpreter?: PythonInterpreter
): Promise<void> {
let newSession: ISessionWithSocket | undefined;

// If we are already using this kernel in an active session just return back
if (this.kernelSpec?.name === kernel.name && this.session) {
return;
}

newSession = await this.createNewKernelSession(kernel, timeoutMS);
newSession = await this.createNewKernelSession(kernel, timeoutMS, interpreter);

// This is just like doing a restart, kill the old session (and the old restart session), and start new ones
if (this.session) {
this.shutdownSession(this.session, this.statusHandler).ignoreErrors();
this.restartSessionPromise?.then((r) => this.shutdownSession(r, undefined)).ignoreErrors(); // NOSONAR
}

// Update our kernel spec
// Update our kernel spec and interpreter
this.kernelSpec = kernel;
this.interpreter = interpreter;

// Save the new session
this.setSession(newSession);
Expand All @@ -144,7 +151,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
this.session?.statusChanged.connect(this.statusHandler); // NOSONAR

// Start the restart session promise too.
this.restartSessionPromise = this.createRestartSession(kernel, newSession);
this.restartSessionPromise = this.createRestartSession(kernel, newSession, interpreter);
}

public async restart(_timeout: number): Promise<void> {
Expand Down Expand Up @@ -298,13 +305,15 @@ export abstract class BaseJupyterSession implements IJupyterSession {
protected abstract async createRestartSession(
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined,
session: ISessionWithSocket,
interpreter?: PythonInterpreter,
cancelToken?: CancellationToken
): Promise<ISessionWithSocket>;

// Sub classes need to implement their own kernel change specific code
protected abstract createNewKernelSession(
kernel: IJupyterKernelSpec | LiveKernelModel,
timeoutMS: number
timeoutMS: number,
interpreter?: PythonInterpreter
): Promise<ISessionWithSocket>;

// Changes the current session.
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/jupyter/jupyterNotebook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ export class JupyterNotebookBase implements INotebook {
this.ranInitialSetup = false;

// Change the kernel on the session
await this.session.changeKernel(spec, timeoutMS);
await this.session.changeKernel(spec, timeoutMS, interpreter);

// Change our own kernel spec
// Only after session was successfully created.
Expand Down
2 changes: 2 additions & 0 deletions src/client/datascience/jupyter/jupyterSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { IOutputChannel } from '../../common/types';
import { sleep } from '../../common/utils/async';
import * as localize from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { PythonInterpreter } from '../../interpreter/contracts';
import { captureTelemetry } from '../../telemetry';
import { BaseJupyterSession, JupyterSessionStartError } from '../baseJupyterSession';
import { Identifiers, Telemetry } from '../constants';
Expand Down Expand Up @@ -131,6 +132,7 @@ export class JupyterSession extends BaseJupyterSession {
protected async createRestartSession(
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined,
session: ISessionWithSocket,
_interpreter?: PythonInterpreter,
cancelToken?: CancellationToken
): Promise<ISessionWithSocket> {
// We need all of the above to create a restart session
Expand Down
7 changes: 6 additions & 1 deletion src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as localize from '../../../common/utils/localize';
import { IServiceContainer } from '../../../ioc/types';
import { Identifiers, LiveShare, Settings } from '../../constants';
import { KernelSelector } from '../../jupyter/kernels/kernelSelector';
import { KernelService } from '../../jupyter/kernels/kernelService';
import { HostJupyterNotebook } from '../../jupyter/liveshare/hostJupyterNotebook';
import { LiveShareParticipantHost } from '../../jupyter/liveshare/liveShareParticipantMixin';
import { IRoleBasedObject } from '../../jupyter/liveshare/roleBasedFactory';
Expand Down Expand Up @@ -58,6 +59,7 @@ export class HostRawNotebookProvider
private kernelLauncher: IKernelLauncher,
private kernelFinder: IKernelFinder,
private kernelSelector: KernelSelector,
private kernelService: KernelService,
private progressReporter: ProgressReporter,
private outputChannel: IOutputChannel
) {
Expand Down Expand Up @@ -117,7 +119,10 @@ export class HostRawNotebookProvider
cancelToken
);

await rawSession.connect(kernelSpec, launchTimeout, cancelToken);
// Locate the interpreter that matches our kernelspec
const interpreter = await this.kernelService.findMatchingInterpreter(kernelSpec);

await rawSession.connect(kernelSpec, launchTimeout, interpreter, cancelToken);

// Get the execution info for our notebook
const info = await this.getExecutionInfo(kernelSpec);
Expand Down
21 changes: 15 additions & 6 deletions src/client/datascience/raw-kernel/rawJupyterSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { IDisposable, IOutputChannel, Resource } from '../../common/types';
import { waitForPromise } from '../../common/utils/async';
import * as localize from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { PythonInterpreter } from '../../interpreter/contracts';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { BaseJupyterSession } from '../baseJupyterSession';
import { Identifiers, Telemetry } from '../constants';
Expand Down Expand Up @@ -62,6 +63,7 @@ export class RawJupyterSession extends BaseJupyterSession {
public async connect(
kernelSpec: IJupyterKernelSpec,
timeout: number,
interpreter?: PythonInterpreter,
cancelToken?: CancellationToken
): Promise<IJupyterKernelSpec | undefined> {
// Save the resource that we connect with
Expand All @@ -71,7 +73,7 @@ export class RawJupyterSession extends BaseJupyterSession {
// Notebook Provider level will handle the thrown error
newSession = await waitForPromise(
Promise.race([
this.startRawSession(kernelSpec, cancelToken),
this.startRawSession(kernelSpec, interpreter, cancelToken),
createPromiseFromCancellation({
cancelAction: 'reject',
defaultValue: new CancellationError(),
Expand All @@ -94,7 +96,11 @@ export class RawJupyterSession extends BaseJupyterSession {
sendTelemetryEvent(Telemetry.RawKernelSessionStartSuccess);
traceInfo('Raw session started and connected');
this.setSession(newSession);

// Update kernelspec and interpreter
this.kernelSpec = newSession.kernelProcess?.kernelSpec;
this.interpreter = interpreter;

this.outputChannel.appendLine(
localize.DataScience.kernelStarted().format(this.kernelSpec.display_name || this.kernelSpec.name)
);
Expand All @@ -115,7 +121,8 @@ export class RawJupyterSession extends BaseJupyterSession {

public async createNewKernelSession(
kernel: IJupyterKernelSpec | LiveKernelModel,
_timeoutMS: number
_timeoutMS: number,
interpreter?: PythonInterpreter
): Promise<ISessionWithSocket> {
if (!kernel || 'session' in kernel) {
// Don't allow for connecting to a LiveKernelModel
Expand All @@ -124,7 +131,7 @@ export class RawJupyterSession extends BaseJupyterSession {

this.outputChannel.appendLine(localize.DataScience.kernelStarted().format(kernel.display_name || kernel.name));

return this.startRawSession(kernel);
return this.startRawSession(kernel, interpreter);
}

protected shutdownSession(
Expand Down Expand Up @@ -161,19 +168,20 @@ export class RawJupyterSession extends BaseJupyterSession {

protected startRestartSession() {
if (!this.restartSessionPromise && this.session) {
this.restartSessionPromise = this.createRestartSession(this.kernelSpec, this.session);
this.restartSessionPromise = this.createRestartSession(this.kernelSpec, this.session, this.interpreter);
}
}
protected async createRestartSession(
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined,
_session: ISessionWithSocket,
interpreter?: PythonInterpreter,
cancelToken?: CancellationToken
): Promise<ISessionWithSocket> {
if (!kernelSpec || 'session' in kernelSpec) {
// Need to have connected before restarting and can't use a LiveKernelModel
throw new Error(localize.DataScience.sessionDisposed());
}
const startPromise = this.startRawSession(kernelSpec, cancelToken);
const startPromise = this.startRawSession(kernelSpec, interpreter, cancelToken);
return startPromise.then((session) => {
this.kernelSelector.addKernelToIgnoreList(session.kernel);
return session;
Expand All @@ -183,6 +191,7 @@ export class RawJupyterSession extends BaseJupyterSession {
@captureTelemetry(Telemetry.RawKernelStartRawSession, undefined, true)
private async startRawSession(
kernelSpec: IJupyterKernelSpec,
interpreter?: PythonInterpreter,
cancelToken?: CancellationToken
): Promise<RawSession> {
const cancellationPromise = createPromiseFromCancellation({
Expand All @@ -193,7 +202,7 @@ export class RawJupyterSession extends BaseJupyterSession {
cancellationPromise.catch(noop);

const process = await Promise.race([
this.kernelLauncher.launch(kernelSpec, this.resource),
this.kernelLauncher.launch(kernelSpec, this.resource, interpreter),
cancellationPromise
]);

Expand Down
4 changes: 4 additions & 0 deletions src/client/datascience/raw-kernel/rawNotebookProviderWrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { IServiceContainer } from '../../ioc/types';
import { JUPYTER_OUTPUT_CHANNEL } from '../constants';
import { KernelSelector } from '../jupyter/kernels/kernelSelector';
import { KernelService } from '../jupyter/kernels/kernelService';
import { IRoleBasedObject, RoleBasedFactory } from '../jupyter/liveshare/roleBasedFactory';
import { ILiveShareHasRole } from '../jupyter/liveshare/types';
import { IKernelFinder, IKernelLauncher } from '../kernel-launcher/types';
Expand All @@ -43,6 +44,7 @@ type RawNotebookProviderClassType = {
kernelLauncher: IKernelLauncher,
kernelFinder: IKernelFinder,
kernelSelector: KernelSelector,
kernelService: KernelService,
progressReporter: ProgressReporter,
outputChannel: IOutputChannel
): IRawNotebookProviderInterface;
Expand All @@ -67,6 +69,7 @@ export class RawNotebookProviderWrapper implements IRawNotebookProvider, ILiveSh
@inject(IKernelLauncher) kernelLauncher: IKernelLauncher,
@inject(IKernelFinder) kernelFinder: IKernelFinder,
@inject(KernelSelector) kernelSelector: KernelSelector,
@inject(KernelService) kernelService: KernelService,
@inject(ProgressReporter) progressReporter: ProgressReporter,
@inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) outputChannel: IOutputChannel
) {
Expand All @@ -87,6 +90,7 @@ export class RawNotebookProviderWrapper implements IRawNotebookProvider, ILiveSh
kernelLauncher,
kernelFinder,
kernelSelector,
kernelService,
progressReporter,
outputChannel
);
Expand Down
6 changes: 5 additions & 1 deletion src/client/datascience/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,11 @@ export interface IJupyterSession extends IAsyncDisposable {
content: KernelMessage.IInspectRequestMsg['content']
): Promise<KernelMessage.IInspectReplyMsg | undefined>;
sendInputReply(content: string): void;
changeKernel(kernel: IJupyterKernelSpec | LiveKernelModel, timeoutMS: number): Promise<void>;
changeKernel(
kernel: IJupyterKernelSpec | LiveKernelModel,
timeoutMS: number,
interpreter?: PythonInterpreter
): Promise<void>;
registerCommTarget(
targetName: string,
callback: (comm: Kernel.IComm, msg: KernelMessage.ICommOpenMsg) => void | PromiseLike<void>
Expand Down