Skip to content

Commit

Permalink
Pass resource to track Kernel Metadata against a resource (#4760)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored Feb 11, 2021
1 parent 93d6475 commit 4a0eefa
Show file tree
Hide file tree
Showing 34 changed files with 145 additions and 100 deletions.
15 changes: 1 addition & 14 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,7 @@ module.exports = {
'src/test/datascience/mockCode2ProtocolConverter.ts',
'src/test/datascience/mockFileSystem.ts',
'src/test/datascience/interactive-common/trustService.unit.test.ts',
'src/test/datascience/interactive-common/notebookProvider.unit.test.ts',
'src/test/datascience/interactive-common/notebookServerProvider.unit.test.ts',
'src/test/datascience/interactive-common/',
'src/test/datascience/interactive-common/trustCommandHandler.unit.test.ts',
'src/test/datascience/mockStatusProvider.ts',
'src/test/datascience/extensionapi/exampleextension/ms-toolsai-test/webpack.config.js',
Expand All @@ -253,8 +252,6 @@ module.exports = {
'src/test/datascience/preWarmVariables.unit.test.ts',
'src/test/datascience/remoteTestHelpers.ts',
'src/test/datascience/mockWorkspaceFolder.ts',
'src/test/datascience/mockJupyterSession.ts',
'src/test/datascience/jupyterUriProviderRegistration.functional.test.ts',
'src/test/datascience/mockJupyterRequest.ts',
'src/test/datascience/inputHistory.unit.test.ts',
'src/test/datascience/jupyterHelpers.ts',
Expand All @@ -271,12 +268,10 @@ module.exports = {
'src/test/datascience/mockProtocol2CodeConverter.ts',
'src/test/datascience/editor-integration/helpers.ts',
'src/test/datascience/editor-integration/cellhashprovider.unit.test.ts',
'src/test/datascience/editor-integration/gotocell.functional.test.ts',
'src/test/datascience/editor-integration/codelensprovider.unit.test.ts',
'src/test/datascience/editor-integration/codewatcher.unit.test.ts',
'src/test/datascience/jupyterPasswordConnect.unit.test.ts',
'src/test/datascience/testHelpers.tsx',
'src/test/datascience/notebook.functional.test.ts',
'src/test/datascience/mockLanguageClient.ts',
'src/test/datascience/errorHandler.functional.test.tsx',
'src/test/datascience/notebook/notebookStorage.unit.test.ts',
Expand All @@ -300,7 +295,6 @@ module.exports = {
'src/test/datascience/intellisense.unit.test.ts',
'src/test/datascience/markdownManipulation.unit.test.ts',
'src/test/datascience/interactivePanel.functional.test.tsx',
'src/test/datascience/variableTestHelpers.ts',
'src/test/datascience/testPersistentStateFactory.ts',
'src/test/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts',
'src/test/datascience/jupyter/interpreter/jupyterInterpreterDependencyService.unit.test.ts',
Expand Down Expand Up @@ -901,12 +895,10 @@ module.exports = {
'src/client/datascience/kernel-launcher/kernelDaemonPreWarmer.ts',
'src/client/datascience/ipywidgets/localWidgetScriptSourceProvider.ts',
'src/client/datascience/ipywidgets/ipyWidgetScriptSourceProvider.ts',
'src/client/datascience/ipywidgets/ipyWidgetScriptSource.ts',
'src/client/datascience/ipywidgets/cdnWidgetScriptSourceProvider.ts',
'src/client/datascience/ipywidgets/types.ts',
'src/client/datascience/ipywidgets/remoteWidgetScriptSourceProvider.ts',
'src/client/datascience/ipywidgets/constants.ts',
'src/client/datascience/ipywidgets/ipyWidgetMessageDispatcher.ts',
'src/client/datascience/ipywidgets/ipywidgetHandler.ts',
'src/client/datascience/ipywidgets/ipyWidgetMessageDispatcherFactory.ts',
'src/client/datascience/themeFinder.ts',
Expand Down Expand Up @@ -951,7 +943,6 @@ module.exports = {
'src/client/datascience/notebookStorage/nativeEditorStorage.ts',
'src/client/datascience/notebookStorage/factory.ts',
'src/client/datascience/notebookStorage/types.ts',
'src/client/datascience/notebookStorage/nativeEditorProvider.ts',
'src/client/datascience/debugLocationTracker.ts',
'src/client/datascience/plotting/plotViewerMessageListener.ts',
'src/client/datascience/plotting/types.ts',
Expand All @@ -972,7 +963,6 @@ module.exports = {
'src/client/datascience/editor-integration/codelensprovider.ts',
'src/client/datascience/editor-integration/cellhashprovider.ts',
'src/client/datascience/commands/commandLineSelector.ts',
'src/client/datascience/commands/notebookCommands.ts',
'src/client/datascience/commands/exportCommands.ts',
'src/client/datascience/cellFactory.ts',
'src/client/datascience/notebook/notebookEditorCompatibilitySupport.ts',
Expand Down Expand Up @@ -1003,7 +993,6 @@ module.exports = {
'src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts',
'src/client/datascience/jupyter/kernels/jupyterKernelSpec.ts',
'src/client/datascience/jupyter/jupyterExecutionFactory.ts',
'src/client/datascience/jupyter/serverPreload.ts',
'src/client/datascience/jupyter/jupyterRequest.ts',
'src/client/datascience/jupyter/commandLineSelector.ts',
'src/client/datascience/jupyter/jupyterDebugger.ts',
Expand All @@ -1014,8 +1003,6 @@ module.exports = {
'src/client/datascience/jupyter/liveshare/utils.ts',
'src/client/datascience/jupyter/liveshare/guestJupyterSessionManagerFactory.ts',
'src/client/datascience/jupyter/liveshare/types.ts',
'src/client/datascience/jupyter/liveshare/serverCache.ts',
'src/client/datascience/jupyter/liveshare/guestJupyterExecution.ts',
'src/client/datascience/jupyter/debuggerVariableRegistration.ts',
'src/client/datascience/jupyter/jupyterConnection.ts',
'src/client/datascience/jupyter/jupyterPasswordConnect.ts',
Expand Down
12 changes: 9 additions & 3 deletions src/client/datascience/baseJupyterSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { CancellationToken } from 'vscode-jsonrpc';
import { ServerStatus } from '../../datascience-ui/interactive-common/mainState';
import { WrappedError } from '../common/errors/types';
import { traceError, traceInfo, traceWarning } from '../common/logger';
import { Resource } from '../common/types';
import { sleep, waitForPromise } from '../common/utils/async';
import * as localize from '../common/utils/localize';
import { noop } from '../common/utils/misc';
Expand All @@ -22,6 +23,7 @@ import { kernelConnectionMetadataHasKernelSpec } from './jupyter/kernels/helpers
import { JupyterKernelPromiseFailedError } from './jupyter/kernels/jupyterKernelPromiseFailedError';
import { getKernelConnectionId, KernelConnectionMetadata } from './jupyter/kernels/types';
import { suppressShutdownErrors } from './raw-kernel/rawKernel';
import { trackKernelResourceInformation } from './telemetry/telemetry';
import { IJupyterSession, ISessionWithSocket, KernelSocketInformation } from './types';

/**
Expand All @@ -34,7 +36,7 @@ import { IJupyterSession, ISessionWithSocket, KernelSocketInformation } from './
export class JupyterSessionStartError extends WrappedError {
constructor(originalException: Error) {
super(originalException.message, originalException);
sendTelemetryEvent(Telemetry.StartSessionFailedJupyter);
sendTelemetryEvent(Telemetry.StartSessionFailedJupyter, undefined, undefined, originalException, true);
}
}

Expand Down Expand Up @@ -145,9 +147,12 @@ export abstract class BaseJupyterSession implements IJupyterSession {
}
return this.session.kernel.requestKernelInfo();
}
public async changeKernel(kernelConnection: KernelConnectionMetadata, timeoutMS: number): Promise<void> {
public async changeKernel(
resource: Resource,
kernelConnection: KernelConnectionMetadata,
timeoutMS: number
): Promise<void> {
let newSession: ISessionWithSocket | undefined;

// If we are already using this kernel in an active session just return back
const currentKernelSpec =
this.kernelConnectionMetadata && kernelConnectionMetadataHasKernelSpec(this.kernelConnectionMetadata)
Expand All @@ -163,6 +168,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
}
}

trackKernelResourceInformation(resource, { kernelConnection });
newSession = await this.createNewKernelSession(kernelConnection, timeoutMS);

// This is just like doing a restart, kill the old session (and the old restart session), and start new ones
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/commands/activeEditorContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
private updateContextOfActiveNotebookKernel(activeEditor?: INotebookEditor) {
if (activeEditor) {
this.notebookProvider
.getOrCreateNotebook({ identity: activeEditor.file, getOnly: true })
.getOrCreateNotebook({ identity: activeEditor.file, resource: activeEditor.file, getOnly: true })
.then((nb) => {
if (activeEditor === this.notebookEditorProvider.activeEditor) {
const canStart = nb && nb.status !== ServerStatus.NotStarted && activeEditor.model?.isTrusted;
Expand Down
8 changes: 6 additions & 2 deletions src/client/datascience/commands/notebookCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,15 @@ export class NotebookCommands implements IDisposable {
}
if (options.identity) {
// Make sure we have a connection or we can't get remote kernels.
const connection = await this.notebookProvider.connect({ getOnly: false, disableUI: false });
const connection = await this.notebookProvider.connect({
getOnly: false,
disableUI: false,
resource: options.resource
});

// Select a new kernel using the connection information
const kernel = await this.kernelSelector.selectJupyterKernel(
options.identity,
options.resource,
connection,
connection?.type || this.notebookProvider.type,
options.currentKernelDisplayName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,12 @@ export class IntellisenseProvider implements IInteractiveWindowListener {

private async getNotebook(token: CancellationToken): Promise<INotebook | undefined> {
return this.notebookIdentity
? this.notebookProvider.getOrCreateNotebook({ identity: this.notebookIdentity, getOnly: true, token })
? this.notebookProvider.getOrCreateNotebook({
identity: this.notebookIdentity,
resource: this.resource,
getOnly: true,
token
})
: undefined;
}

Expand Down
13 changes: 10 additions & 3 deletions src/client/datascience/interactive-common/interactiveBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,10 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo

protected async createNotebookIfProviderConnectionExists(): Promise<void> {
// Check to see if we are already connected to our provider
const providerConnection = await this.notebookProvider.connect({ getOnly: true });
const providerConnection = await this.notebookProvider.connect({
getOnly: true,
resource: this.owningResource
});

if (providerConnection) {
try {
Expand Down Expand Up @@ -931,7 +934,7 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
serverUri !== Settings.JupyterServerLocalLaunch &&
!this.configService.getSettings(this.owningResource).disableJupyterAutoStart
) {
serverConnection = await this.notebookProvider.connect({ disableUI: true });
serverConnection = await this.notebookProvider.connect({ disableUI: true, resource: this.owningResource });
}
let displayName =
serverConnection?.displayName ||
Expand Down Expand Up @@ -985,7 +988,11 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
// Make sure we're loaded first.
try {
traceInfo('Waiting for jupyter server and web panel ...');
const serverConnection = await this.notebookProvider.connect({ getOnly: false, disableUI: false });
const serverConnection = await this.notebookProvider.connect({
getOnly: false,
disableUI: false,
resource: this.owningResource
});
if (serverConnection) {
await this.ensureNotebook(serverConnection);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { CancellationToken, ConfigurationTarget, EventEmitter, Uri } from 'vscod
import { IApplicationShell } from '../../common/application/types';
import { CancellationError, wrapCancellationTokens } from '../../common/cancellation';
import { traceInfo } from '../../common/logger';
import { IConfigurationService } from '../../common/types';
import { IConfigurationService, Resource } from '../../common/types';
import * as localize from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { IInterpreterService } from '../../interpreter/contracts';
Expand Down Expand Up @@ -95,6 +95,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {

private async startServer(
options: {
resource: Resource;
metadata?: nbformat.INotebookMetadata;
kernelConnection?: KernelConnectionMetadata;
},
Expand Down Expand Up @@ -215,6 +216,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {
}

private async getNotebookServerOptions(options: {
resource: Resource;
metadata?: nbformat.INotebookMetadata;
kernelConnection?: KernelConnectionMetadata;
}): Promise<INotebookServerOptions> {
Expand All @@ -230,6 +232,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {

return {
uri: serverURI,
resource: options.resource,
skipUsingDefaultConfig: !useDefaultConfig,
purpose: Identifiers.HistoryPurpose,
kernelConnection: options.kernelConnection,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,10 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
try {
const settings = this.configuration.getSettings(document.uri);
// Create a new notebook
notebook = await this.notebookProvider.getOrCreateNotebook({ identity: createExportInteractiveIdentity() });
notebook = await this.notebookProvider.getOrCreateNotebook({
identity: createExportInteractiveIdentity(),
resource: file
});
// If that works, then execute all of the cells.
const cells = Array.prototype.concat(
...(await Promise.all(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
if (this.notebookIdentity && !this.notebook) {
this.notebook = await this.notebookProvider.getOrCreateNotebook({
identity: this.notebookIdentity,
resource: this.notebookIdentity,
getOnly: true
});
}
Expand Down
1 change: 1 addition & 0 deletions src/client/datascience/ipywidgets/ipyWidgetScriptSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export class IPyWidgetScriptSource implements ILocalResourceUriConverter {
if (!this.notebook) {
this.notebook = await this.notebookProvider.getOrCreateNotebook({
identity: this.identity,
resource: this.identity,
disableUI: true,
getOnly: true
});
Expand Down
15 changes: 12 additions & 3 deletions src/client/datascience/jupyter/jupyterExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { PythonEnvironment } from '../../pythonEnvironments/info';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { JupyterSessionStartError } from '../baseJupyterSession';
import { Commands, Identifiers, Telemetry } from '../constants';
import { trackKernelResourceInformation } from '../telemetry/telemetry';
import {
IJupyterConnection,
IJupyterExecution,
Expand Down Expand Up @@ -192,7 +193,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
const sessionManager = await sessionManagerFactory.create(connection);
try {
kernelConnectionMetadata = await this.kernelSelector.getPreferredKernelForRemoteConnection(
undefined,
options?.resource,
sessionManager,
options?.metadata,
cancelToken
Expand All @@ -211,7 +212,12 @@ export class JupyterExecutionBase implements IJupyterExecution {
purpose: options ? options.purpose : uuid(),
disableUI: !allowUI
};

// If we were not provided a kernel connection, this means we changed the connection here.
if (!options?.kernelConnection) {
trackKernelResourceInformation(options?.resource, {
kernelConnection: launchInfo.kernelConnectionMetadata
});
}
// eslint-disable-next-line no-constant-condition
while (true) {
try {
Expand All @@ -238,14 +244,17 @@ export class JupyterExecutionBase implements IJupyterExecution {
const selection = await this.appShell.showErrorMessage(message, selectKernel, cancel);
if (selection === selectKernel) {
const kernelInterpreter = await this.kernelSelector.selectLocalKernel(
undefined,
options?.resource,
'jupyter',
new StopWatch(),
cancelToken,
getDisplayNameOrNameOfKernelConnection(launchInfo.kernelConnectionMetadata)
);
if (kernelInterpreter) {
launchInfo.kernelConnectionMetadata = kernelInterpreter;
trackKernelResourceInformation(options?.resource, {
kernelConnection: launchInfo.kernelConnectionMetadata
});
continue;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/jupyter/jupyterNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ export class JupyterNotebookBase implements INotebook {
this.ranInitialSetup = false;

// Change the kernel on the session
await this.session.changeKernel(connectionMetadata, timeoutMS);
await this.session.changeKernel(this.resource, connectionMetadata, timeoutMS);

// Change our own kernel spec
// Only after session was successfully created.
Expand Down
2 changes: 2 additions & 0 deletions src/client/datascience/jupyter/jupyterNotebookProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class JupyterNotebookProvider implements IJupyterNotebookProvider {
const server = await this.serverProvider.getOrCreateServer({
getOnly: options.getOnly,
disableUI: options.disableUI,
resource: options.resource,
token: options.token
});

Expand All @@ -56,6 +57,7 @@ export class JupyterNotebookProvider implements IJupyterNotebookProvider {
getOnly: options.getOnly,
disableUI: options.disableUI,
token: options.token,
resource: options.resource,
metadata: options.metadata,
kernelConnection: options.kernelConnection
});
Expand Down
Loading

0 comments on commit 4a0eefa

Please sign in to comment.