Skip to content

Commit

Permalink
Revision to displaying cached remote kernel specs and live kernels (#…
Browse files Browse the repository at this point in the history
…9985)

* Misc

* Misc

* Misc

* Misc

* oops

* Misc

* Add tests

* Misc

* Misc

* Address code review comments

* Misc

* oops

* Fix tests
  • Loading branch information
DonJayamanne authored May 12, 2022
1 parent 3675351 commit 1143125
Show file tree
Hide file tree
Showing 19 changed files with 877 additions and 86 deletions.
1 change: 0 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@
"DataScience.remoteJupyterConnectionFailedWithoutServerWithError": "Connection failure. Verify the server is running and reachable. ({0}).",
"DataScience.jupyterNotebookRemoteConnectFailedWeb": "Failed to connect to remote Jupyter server.\r\nCheck that the Jupyter Server URI can be reached from a browser.\r\n{0}. Click [here](https://aka.ms/vscjremoteweb) for more information.",
"DataScience.jupyterNotebookRemoteConnectSelfCertsFailed": "Failed to connect to remote Jupyter notebook.\r\nSpecified server is using self signed certs. Enable Allow Unauthorized Remote Connection setting to connect anyways\r\n{0}\r\n{1}",
"DataScience.jupyterRemoteConnectFailedModalMessage": "Failed to connect to the remote Jupyter Server. View Jupyter log for further details.",
"DataScience.changeJupyterRemoteConnection": "Change Jupyter Server connection.",
"DataScience.notebookVersionFormat": "Jupyter Notebook Version: {0}",
"DataScience.jupyterKernelSpecNotFound": "Cannot create a Jupyter kernel spec and none are available for use",
Expand Down
100 changes: 100 additions & 0 deletions src/kernels/jupyter/liveRemoteKernelConnectionTracker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { inject, injectable, named } from 'inversify';
import { Memento, Uri } from 'vscode';
import { IExtensionSyncActivationService } from '../../platform/activation/types';
import { GLOBAL_MEMENTO, IDisposableRegistry, IMemento } from '../../platform/common/types';
import { noop } from '../../platform/common/utils/misc';
import { LiveRemoteKernelConnectionMetadata } from '../types';
import { computeServerId } from './jupyterUtils';
import { IJupyterServerUriStorage, ILiveRemoteKernelConnectionUsageTracker } from './types';

export const mementoKeyToTrackRemoveKernelUrisAndSessionsUsedByResources = 'removeKernelUrisAndSessionsUsedByResources';

type ServerId = string;
type KernelId = string;
type UriString = string;
type UriSessionUsedByResources = Record<ServerId, Record<KernelId, UriString[]>>;

/**
* Class to track the remote kernels that have been used by notebooks.
*/
@injectable()
export class LiveRemoteKernelConnectionUsageTracker
implements IExtensionSyncActivationService, ILiveRemoteKernelConnectionUsageTracker
{
private usedRemoteKernelServerIdsAndSessions: UriSessionUsedByResources = {};
constructor(
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IJupyterServerUriStorage) private readonly uriStorage: IJupyterServerUriStorage,
@inject(IMemento) @named(GLOBAL_MEMENTO) private readonly memento: Memento
) {}
public activate(): void {
this.usedRemoteKernelServerIdsAndSessions = this.memento.get<UriSessionUsedByResources>(
mementoKeyToTrackRemoveKernelUrisAndSessionsUsedByResources,
{}
);
this.uriStorage.onDidRemoveUri(this.onDidRemoveUri, this, this.disposables);
}

public wasKernelUsed(connection: LiveRemoteKernelConnectionMetadata) {
return (
connection.serverId in this.usedRemoteKernelServerIdsAndSessions &&
typeof connection.kernelModel.id === 'string' &&
connection.kernelModel.id in this.usedRemoteKernelServerIdsAndSessions[connection.serverId]
);
}
public trackKernelIdAsUsed(resource: Uri, serverId: string, kernelId: string) {
this.usedRemoteKernelServerIdsAndSessions[serverId] = this.usedRemoteKernelServerIdsAndSessions[serverId] || {};
this.usedRemoteKernelServerIdsAndSessions[serverId][kernelId] =
this.usedRemoteKernelServerIdsAndSessions[serverId][kernelId] || [];
const uris = this.usedRemoteKernelServerIdsAndSessions[serverId][kernelId];
if (uris.includes(resource.toString())) {
return;
}
uris.push(resource.toString());
this.memento
.update(
mementoKeyToTrackRemoveKernelUrisAndSessionsUsedByResources,
this.usedRemoteKernelServerIdsAndSessions
)
.then(noop, noop);
}
public trackKernelIdAsNotUsed(resource: Uri, serverId: string, kernelId: string) {
if (!(serverId in this.usedRemoteKernelServerIdsAndSessions)) {
return;
}
if (!(kernelId in this.usedRemoteKernelServerIdsAndSessions[serverId])) {
return;
}
const uris = this.usedRemoteKernelServerIdsAndSessions[serverId][kernelId];
if (!Array.isArray(uris) || !uris.includes(resource.toString())) {
return;
}
uris.splice(uris.indexOf(resource.toString()), 1);
if (uris.length === 0) {
delete this.usedRemoteKernelServerIdsAndSessions[serverId][kernelId];
}
if (Object.keys(this.usedRemoteKernelServerIdsAndSessions[serverId]).length === 0) {
delete this.usedRemoteKernelServerIdsAndSessions[serverId];
}

this.memento
.update(
mementoKeyToTrackRemoveKernelUrisAndSessionsUsedByResources,
this.usedRemoteKernelServerIdsAndSessions
)
.then(noop, noop);
}
private onDidRemoveUri(uri: string) {
const serverId = computeServerId(uri);
delete this.usedRemoteKernelServerIdsAndSessions[serverId];
this.memento
.update(
mementoKeyToTrackRemoveKernelUrisAndSessionsUsedByResources,
this.usedRemoteKernelServerIdsAndSessions
)
.then(noop, noop);
}
}
2 changes: 1 addition & 1 deletion src/kernels/jupyter/preferredRemoteKernelIdProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class PreferredRemoteKernelIdProvider {
public async storePreferredRemoteKernelId(uri: Uri, id: string): Promise<void> {
await this.updatePreferredRemoteKernelIdInternal(uri, id);
}
public async updatePreferredRemoteKernelIdInternal(uri: Uri, id?: string): Promise<void> {
private async updatePreferredRemoteKernelIdInternal(uri: Uri, id?: string): Promise<void> {
let requiresUpdate = false;

// Don't update in memory representation.
Expand Down
81 changes: 81 additions & 0 deletions src/kernels/jupyter/remoteKernelConnectionHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { Disposable, NotebookDocument } from 'vscode';
import { IVSCodeNotebookController } from '../../notebooks/controllers/types';
import { INotebookControllerManager } from '../../notebooks/types';
import { IExtensionSyncActivationService } from '../../platform/activation/types';
import { IDisposableRegistry } from '../../platform/common/types';
import { noop } from '../../platform/common/utils/misc';
import { traceInfo } from '../../platform/logging';
import { IKernel, IKernelProvider, isLocalConnection } from '../types';
import { PreferredRemoteKernelIdProvider } from './preferredRemoteKernelIdProvider';
import { ILiveRemoteKernelConnectionUsageTracker } from './types';

@injectable()
export class RemoteKernelConnectionHandler implements IExtensionSyncActivationService {
constructor(
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IKernelProvider) private readonly kernelProvider: IKernelProvider,
@inject(INotebookControllerManager) private readonly controllers: INotebookControllerManager,
@inject(ILiveRemoteKernelConnectionUsageTracker)
private readonly liveKernelTracker: ILiveRemoteKernelConnectionUsageTracker,
@inject(PreferredRemoteKernelIdProvider)
private readonly preferredRemoteKernelIdProvider: PreferredRemoteKernelIdProvider
) {}
activate(): void {
this.kernelProvider.onDidStartKernel(this.onDidStartKernel, this, this.disposables);
this.controllers.onNotebookControllerSelectionChanged(
this.onNotebookControllerSelectionChanged,
this,
this.disposables
);
}
private onNotebookControllerSelectionChanged({
selected,
notebook,
controller
}: {
selected: boolean;
notebook: NotebookDocument;
controller: IVSCodeNotebookController;
}) {
if (controller.connection.kind === 'connectToLiveRemoteKernel' && controller.connection.kernelModel.id) {
if (selected) {
this.liveKernelTracker.trackKernelIdAsUsed(
notebook.uri,
controller.connection.serverId,
controller.connection.kernelModel.id
);
} else {
this.liveKernelTracker.trackKernelIdAsNotUsed(
notebook.uri,
controller.connection.serverId,
controller.connection.kernelModel.id
);
}
}
if (isLocalConnection(controller.connection)) {
this.preferredRemoteKernelIdProvider.clearPreferredRemoteKernelId(notebook.uri).catch(noop);
}
}
private onDidStartKernel(kernel: IKernel) {
if (kernel.creator !== 'jupyterExtension' || !kernel.resourceUri) {
return;
}
const resource = kernel.resourceUri;
if (kernel.kernelConnectionMetadata.kind === 'startUsingRemoteKernelSpec') {
const serverId = kernel.kernelConnectionMetadata.serverId;
const subscription = kernel.kernelSocket.subscribe((info) => {
const kernelId = info?.options.id;
if (!kernel.disposed && !kernel.disposing && kernelId) {
traceInfo(`Updating preferred kernel for remote notebook ${kernelId}`);
this.preferredRemoteKernelIdProvider.storePreferredRemoteKernelId(resource, kernelId).catch(noop);
this.liveKernelTracker.trackKernelIdAsUsed(resource, serverId, kernelId);
}
});
this.disposables.push(new Disposable(() => subscription.unsubscribe()));
}
}
}
18 changes: 15 additions & 3 deletions src/kernels/jupyter/serviceRegistry.node.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { IExtensionSingleActivationService } from '../../platform/activation/types';
import { IExtensionSingleActivationService, IExtensionSyncActivationService } from '../../platform/activation/types';
import { IServiceManager } from '../../platform/ioc/types';
import { IRemoteKernelFinder } from '../raw/types';
import { INotebookProvider } from '../types';
Expand Down Expand Up @@ -37,6 +37,8 @@ import { NotebookStarter } from './launcher/notebookStarter.node';
import { ServerConnectionType } from './launcher/serverConnectionType';
import { ServerPreload } from './launcher/serverPreload.node';
import { JupyterServerUriStorage } from './launcher/serverUriStorage';
import { LiveRemoteKernelConnectionUsageTracker } from './liveRemoteKernelConnectionTracker';
import { RemoteKernelConnectionHandler } from './remoteKernelConnectionHandler';
import { RemoteKernelFinder } from './remoteKernelFinder';
import { JupyterServerSelector } from './serverSelector';
import { BackingFileCreator } from './session/backingFileCreator.node';
Expand All @@ -61,7 +63,8 @@ import {
INotebookStarter,
IJupyterRequestCreator,
IJupyterRequestAgentCreator,
INotebookServerFactory
INotebookServerFactory,
ILiveRemoteKernelConnectionUsageTracker
} from './types';
import { IJupyterCommandFactory, IJupyterSubCommandExecutionService } from './types.node';

Expand Down Expand Up @@ -146,5 +149,14 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole
serviceManager.addSingleton<IJupyterRequestAgentCreator>(IJupyterRequestAgentCreator, RequestAgentCreator);
serviceManager.addSingleton<ServerConnectionType>(ServerConnectionType, ServerConnectionType);
serviceManager.addSingleton<JupyterConnection>(JupyterConnection, JupyterConnection);
serviceManager.addBinding(JupyterConnection, IExtensionSingleActivationService);
serviceManager.addBinding(JupyterConnection, IExtensionSyncActivationService);
serviceManager.addSingleton<ILiveRemoteKernelConnectionUsageTracker>(
ILiveRemoteKernelConnectionUsageTracker,
LiveRemoteKernelConnectionUsageTracker
);
serviceManager.addBinding(ILiveRemoteKernelConnectionUsageTracker, IExtensionSyncActivationService);
serviceManager.addSingleton<IExtensionSyncActivationService>(
IExtensionSyncActivationService,
RemoteKernelConnectionHandler
);
}
18 changes: 15 additions & 3 deletions src/kernels/jupyter/serviceRegistry.web.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { IExtensionSingleActivationService } from '../../platform/activation/types';
import { IExtensionSingleActivationService, IExtensionSyncActivationService } from '../../platform/activation/types';
import { IServiceManager } from '../../platform/ioc/types';
import { IRemoteKernelFinder } from '../raw/types';
import { INotebookProvider } from '../types';
Expand All @@ -19,6 +19,8 @@ import { NotebookProvider } from './launcher/notebookProvider';
import { NotebookServerProvider } from './launcher/notebookServerProvider';
import { ServerConnectionType } from './launcher/serverConnectionType';
import { JupyterServerUriStorage } from './launcher/serverUriStorage';
import { LiveRemoteKernelConnectionUsageTracker } from './liveRemoteKernelConnectionTracker';
import { RemoteKernelConnectionHandler } from './remoteKernelConnectionHandler';
import { RemoteKernelFinder } from './remoteKernelFinder';
import { JupyterServerSelector } from './serverSelector';
import { BackingFileCreator } from './session/backingFileCreator.web';
Expand All @@ -35,7 +37,8 @@ import {
IJupyterServerProvider,
IJupyterExecution,
IJupyterRequestCreator,
INotebookServerFactory
INotebookServerFactory,
ILiveRemoteKernelConnectionUsageTracker
} from './types';

export function registerTypes(serviceManager: IServiceManager, _isDevMode: boolean) {
Expand Down Expand Up @@ -71,5 +74,14 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole
serviceManager.addSingleton<IJupyterRequestCreator>(IJupyterRequestCreator, JupyterRequestCreator);
serviceManager.addSingleton<ServerConnectionType>(ServerConnectionType, ServerConnectionType);
serviceManager.addSingleton<JupyterConnection>(JupyterConnection, JupyterConnection);
serviceManager.addBinding(JupyterConnection, IExtensionSingleActivationService);
serviceManager.addBinding(JupyterConnection, IExtensionSyncActivationService);
serviceManager.addSingleton<ILiveRemoteKernelConnectionUsageTracker>(
ILiveRemoteKernelConnectionUsageTracker,
LiveRemoteKernelConnectionUsageTracker
);
serviceManager.addBinding(ILiveRemoteKernelConnectionUsageTracker, IExtensionSyncActivationService);
serviceManager.addSingleton<IExtensionSyncActivationService>(
IExtensionSyncActivationService,
RemoteKernelConnectionHandler
);
}
19 changes: 18 additions & 1 deletion src/kernels/jupyter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import {
IJupyterKernelSpec,
GetServerOptions,
IKernelSocket,
KernelActionSource
KernelActionSource,
LiveRemoteKernelConnectionMetadata
} from '../types';
import { ClassType } from '../../platform/ioc/types';

Expand Down Expand Up @@ -294,3 +295,19 @@ export interface INotebookStarter extends IDisposable {
cancelToken: CancellationToken
): Promise<IJupyterConnection>;
}

export const ILiveRemoteKernelConnectionUsageTracker = Symbol('ILiveRemoteKernelConnectionUsageTracker');
export interface ILiveRemoteKernelConnectionUsageTracker {
/**
* Whether the provided remote kernel was ever used by any notebook within the extension.
*/
wasKernelUsed(connection: LiveRemoteKernelConnectionMetadata): boolean;
/**
* Tracks the fact that the provided remote kernel for a given server was used by a notebook defined by the uri.
*/
trackKernelIdAsUsed(resource: Uri, serverId: string, kernelId: string): void;
/**
* Tracks the fact that the provided remote kernel for a given server is no longer used by a notebook defined by the uri.
*/
trackKernelIdAsNotUsed(resource: Uri, serverId: string, kernelId: string): void;
}
15 changes: 13 additions & 2 deletions src/kernels/kernelFinder.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Memento } from 'vscode';
import { IFileSystem } from '../platform/common/platform/types.node';
import { GLOBAL_MEMENTO, IMemento } from '../platform/common/types';
import { ServerConnectionType } from './jupyter/launcher/serverConnectionType';
import { IJupyterServerUriStorage } from './jupyter/types';
import { IJupyterServerUriStorage, ILiveRemoteKernelConnectionUsageTracker } from './jupyter/types';
import { BaseKernelFinder } from './kernelFinder.base';
import { PreferredRemoteKernelIdProvider } from './jupyter/preferredRemoteKernelIdProvider';
import { ILocalKernelFinder, IRemoteKernelFinder } from './raw/types';
Expand All @@ -21,7 +21,9 @@ export class KernelFinder extends BaseKernelFinder {
@inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento,
@inject(IFileSystem) private readonly fs: IFileSystem,
@inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage,
@inject(ServerConnectionType) serverConnectionType: ServerConnectionType
@inject(ServerConnectionType) serverConnectionType: ServerConnectionType,
@inject(ILiveRemoteKernelConnectionUsageTracker)
private readonly liveKernelConnectionTracker: ILiveRemoteKernelConnectionUsageTracker
) {
super(
preferredRemoteFinder,
Expand Down Expand Up @@ -50,6 +52,15 @@ export class KernelFinder extends BaseKernelFinder {
? this.fs.localFileExists(kernel.interpreter.uri.fsPath)
: Promise.resolve(true);
});
case 'startUsingRemoteKernelSpec':
// Always fetch the latest kernels from remotes, no need to display cached remote kernels.
return false;
case 'connectToLiveRemoteKernel':
// Only list live kernels that was used by the user,
// Even if such a kernel no longer exists on the sever.
// This way things don't just disappear from the list &
// user will get notified when they attempt to re-use this kernel.
return this.liveKernelConnectionTracker.wasKernelUsed(kernel);
}

return true;
Expand Down
Loading

0 comments on commit 1143125

Please sign in to comment.