Skip to content

Commit

Permalink
Refactor Jupyter Connection
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne committed May 19, 2023
1 parent 8e89c53 commit 327db4f
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 281 deletions.
25 changes: 8 additions & 17 deletions src/kernels/jupyter/connection/jupyterConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { inject, injectable } from 'inversify';
import { noop } from '../../../platform/common/utils/misc';
import { RemoteJupyterServerUriProviderError } from '../../errors/remoteJupyterServerUriProviderError';
import { BaseError } from '../../../platform/errors/types';
import { IJupyterConnection } from '../../types';
import {
computeServerId,
createRemoteConnectionInfo,
Expand Down Expand Up @@ -33,29 +32,16 @@ export class JupyterConnection {
) {}

public async createConnectionInfo(options: { serverId: string } | { uri: string }) {
const uri = 'uri' in options ? options.uri : await this.getUriFromServerId(options.serverId);
const uri = 'uri' in options ? options.uri : (await this.serverUriStorage.get(options.serverId))?.uri;
if (!uri) {
throw new Error('Server Not found');
}
return this.createConnectionInfoFromUri(uri);
}
public async validateRemoteUri(uri: string): Promise<void> {
return this.validateRemoteConnection(await this.createConnectionInfoFromUri(uri));
}

private async getUriFromServerId(serverId: string) {
// Since there's one server per session, don't use a resource to figure out these settings
const savedList = await this.serverUriStorage.getAll();
return savedList.find((item) => item.serverId === serverId)?.uri;
}
private async createConnectionInfoFromUri(uri: string) {
const server = await this.getJupyterServerUri(uri);
const idAndHandle = extractJupyterServerHandleAndId(uri);
return createRemoteConnectionInfo(uri, server, idAndHandle?.id);
}

private async validateRemoteConnection(connection: IJupyterConnection): Promise<void> {
public async validateRemoteUri(uri: string): Promise<void> {
let sessionManager: IJupyterSessionManager | undefined = undefined;
const connection = await this.createConnectionInfoFromUri(uri);
try {
// Attempt to list the running kernels. It will return empty if there are none, but will
// throw if can't connect.
Expand All @@ -69,6 +55,11 @@ export class JupyterConnection {
}
}
}
private async createConnectionInfoFromUri(uri: string) {
const server = await this.getJupyterServerUri(uri);
const idAndHandle = extractJupyterServerHandleAndId(uri);
return createRemoteConnectionInfo(uri, server, idAndHandle?.id);
}

private async getJupyterServerUri(uri: string) {
const idAndHandle = extractJupyterServerHandleAndId(uri);
Expand Down
13 changes: 6 additions & 7 deletions src/kernels/jupyter/connection/jupyterConnection.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { DataScience } from '../../../platform/common/utils/localize';
import { EXTENSION_ROOT_DIR } from '../../../platform/constants.node';
import { ServiceContainer } from '../../../platform/ioc/container';
import { IServiceContainer } from '../../../platform/ioc/types';
import { JupyterConnectionWaiter } from '../launcher/jupyterConnection.node';
import { JupyterConnectionWaiter } from '../launcher/jupyterConnectionWaiter.node';
import { noop } from '../../../test/core';
use(chaiAsPromised);
suite('Jupyter Connection', async () => {
Expand Down Expand Up @@ -167,24 +167,23 @@ suite('JupyterConnection', () => {
when(serviceContainer.get<IConfigurationService>(IConfigurationService)).thenReturn(instance(configService));
});

function createConnectionWaiter(cancelToken?: CancellationToken) {
function createConnectionWaiter() {
return new JupyterConnectionWaiter(
launchResult,
notebookDir,
Uri.file(EXTENSION_ROOT_DIR),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
getServerInfoStub as any,
instance(serviceContainer),
undefined,
cancelToken
undefined
);
}
test('Successfully gets connection info', async () => {
(<any>dsSettings).jupyterLaunchTimeout = 10_000;
const waiter = createConnectionWaiter();
observableOutput.next({ source: 'stderr', out: 'Jupyter listening on http://123.123.123:8888' });

const connection = await waiter.waitForConnection();
const connection = await waiter.ready;

assert.equal(connection.localLaunch, true);
assert.equal(connection.baseUrl, expectedServerInfo.url);
Expand All @@ -195,15 +194,15 @@ suite('JupyterConnection', () => {
(<any>dsSettings).jupyterLaunchTimeout = 10;
const waiter = createConnectionWaiter();

const promise = waiter.waitForConnection();
const promise = waiter.ready;

await assert.isRejected(promise, DataScience.jupyterLaunchTimedOut);
});
test('Throw crashed error', async () => {
const exitCode = 999;
const waiter = createConnectionWaiter();

const promise = waiter.waitForConnection();
const promise = waiter.ready;
childProc.emit('exit', exitCode);
observableOutput.complete();

Expand Down
246 changes: 0 additions & 246 deletions src/kernels/jupyter/launcher/jupyterConnection.node.ts

This file was deleted.

Loading

0 comments on commit 327db4f

Please sign in to comment.