Skip to content

Commit

Permalink
Remove unused proposed API (#14621)
Browse files Browse the repository at this point in the history 
* Remove API added for AzML

* Remove unused import statement in
jupyterSession.ts

* Remove unnecessary test cases for Jupyter style
names in session factory

* Ignore test

* misc
  • Loading branch information
@DonJayamanne
DonJayamanne authored Oct 30, 2023
1 parent 3511846 commit 226adb0
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 186 deletions.
34 changes: 0 additions & 34 deletions src/api.proposed.mappedRemoteDirectory.d.ts
View file

This file was deleted.

4 changes: 0 additions & 4 deletions src/kernels/jupyter/connection/jupyterServerProviderRegistry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,6 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
displayName: server.label,
token: info.token || '',
authorizationHeader: info.headers,
// eslint-disable-next-line local-rules/dont-use-fspath
mappedRemoteNotebookDir: server.mappedRemoteDirectory?.fsPath,
webSocketProtocols: info.webSocketProtocols,
fetch: info.fetch,
WebSocket: info.WebSocket
Expand All @@ -262,8 +260,6 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
displayName: server.label,
token: info.token || '',
authorizationHeader: info.headers,
// eslint-disable-next-line local-rules/dont-use-fspath
mappedRemoteNotebookDir: (result || server).mappedRemoteDirectory?.fsPath,
webSocketProtocols: info.webSocketProtocols,
fetch: info.fetch,
WebSocket: info.WebSocket
Expand Down
3 changes: 0 additions & 3 deletions src/kernels/jupyter/jupyterUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,6 @@ export function createJupyterConnectionInfo(
: getJupyterConnectionDisplayName(token, baseUrl),
dispose: () => toDispose?.dispose(),
rootDirectory,
// Temporarily support workingDirectory as a fallback for old extensions using that (to be removed in the next release).
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mappedRemoteNotebookDir: serverUri?.mappedRemoteNotebookDir || (serverUri as any)?.workingDirectory,
// For remote jupyter servers that are managed by us, we can provide the auth header.
// Its crucial this is set to undefined, else password retrieval will not be attempted.
getAuthHeader,
Expand Down
72 changes: 0 additions & 72 deletions src/kernels/jupyter/session/jupyterKernelSessionFactory.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ suite('New Jupyter Kernel Session Factory', () => {
when(connection.dispose()).thenReturn();
when(connection.getAuthHeader).thenReturn();
when(connection.getWebsocketProtocols).thenReturn();
when(connection.mappedRemoteNotebookDir).thenReturn();
when(connection.providerId).thenReturn('_builtin.something');
when(connection.rootDirectory).thenReturn(Uri.file('someDir'));
when(connection.token).thenReturn('1234');
Expand Down Expand Up @@ -438,43 +437,7 @@ suite('New Jupyter Kernel Session Factory', () => {
when(kernel.status).thenReturn('busy');
assert.strictEqual(wrapperSession.status, 'busy');
});
test('Create Session with Jupyter style names (notebook)', async () => {
when(connection.mappedRemoteNotebookDir).thenReturn('/foo/bar');
const resource = Uri.file('/foo/bar/baz/abc.ipynb');
const options: KernelSessionCreationOptions = {
kernelConnection: remoteKernelSpec,
creator: 'jupyterExtension',
resource,
token: token.token,
ui
};
const { session, kernel } = createSession();
when(sessionManager.startNew(anything(), anything())).thenResolve(resolvableInstance(session));

const wrapperSession = await factory.create(options);

assert.ok(wrapperSession);

verify(kernelService.ensureKernelIsUsable(anything(), anything(), anything(), anything(), false)).never();
verify(jupyterNotebookProvider.getOrStartServer(anything())).never();
verify(workspaceService.computeWorkingDirectory(anything())).never();
verify(sessionManager.startNew(anything(), anything())).once();
verify(jupyterConnection.createConnectionInfo(anything())).once();
assert.strictEqual(capture(sessionManager.startNew).first()[0].type, 'notebook');

when(kernel.status).thenReturn('idle');
assert.strictEqual(wrapperSession.status, 'idle');
when(kernel.status).thenReturn('busy');
assert.strictEqual(wrapperSession.status, 'busy');

const newSessionOptions = capture(sessionManager.startNew).first()[0];
assert.strictEqual(newSessionOptions.name, 'abc.ipynb');
assert.strictEqual(newSessionOptions.path, 'baz/abc.ipynb');
assert.strictEqual(newSessionOptions.type, 'notebook');
assert.deepStrictEqual(newSessionOptions.kernel, { name: 'python3' });
});
test('Create Session with non-Jupyter style names (notebook)', async () => {
when(connection.mappedRemoteNotebookDir).thenReturn();
const resource = Uri.file('/foo/bar/baz/abc.ipynb');
const options: KernelSessionCreationOptions = {
kernelConnection: remoteKernelSpec,
Expand Down Expand Up @@ -523,39 +486,4 @@ suite('New Jupyter Kernel Session Factory', () => {
assert.strictEqual(newSessionOptions.type, 'notebook');
assert.deepStrictEqual(newSessionOptions.kernel, { name: 'python3' });
});
test('Create Session with Jupyter style names (interactive window with Python file)', async () => {
when(connection.mappedRemoteNotebookDir).thenReturn('/foo/bar');
const resource = Uri.file('/foo/bar/baz/abc.py');
const options: KernelSessionCreationOptions = {
kernelConnection: remoteKernelSpec,
creator: 'jupyterExtension',
resource,
token: token.token,
ui
};
const { session, kernel } = createSession();
when(sessionManager.startNew(anything(), anything())).thenResolve(resolvableInstance(session));

const wrapperSession = await factory.create(options);

assert.ok(wrapperSession);

verify(kernelService.ensureKernelIsUsable(anything(), anything(), anything(), anything(), false)).never();
verify(jupyterNotebookProvider.getOrStartServer(anything())).never();
verify(workspaceService.computeWorkingDirectory(anything())).never();
verify(sessionManager.startNew(anything(), anything())).once();
verify(jupyterConnection.createConnectionInfo(anything())).once();
assert.strictEqual(capture(sessionManager.startNew).first()[0].type, 'console');

when(kernel.status).thenReturn('idle');
assert.strictEqual(wrapperSession.status, 'idle');
when(kernel.status).thenReturn('busy');
assert.strictEqual(wrapperSession.status, 'busy');

const newSessionOptions = capture(sessionManager.startNew).first()[0];
assert.strictEqual(newSessionOptions.name, 'abc.py');
assert.strictEqual(newSessionOptions.path, 'baz/abc.py');
assert.strictEqual(newSessionOptions.type, 'console');
assert.deepStrictEqual(newSessionOptions.kernel, { name: 'python3' });
});
});
45 changes: 22 additions & 23 deletions src/kernels/jupyter/session/jupyterSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
import { DisplayOptions } from '../../displayOptions';
import { IJupyterKernelService } from '../types';
import { noop } from '../../../platform/common/utils/misc';
import * as path from '../../../platform/vscode-path/resources';
import { getResourceType } from '../../../platform/common/utils';
import { waitForIdleOnSession } from '../../common/helpers';
import { BaseJupyterSessionConnection } from '../../common/baseJupyterSessionConnection';
Expand Down Expand Up @@ -196,27 +195,27 @@ export class JupyterSessionWrapper
}

export function getRemoteSessionOptions(
remoteConnection: IJupyterConnection,
resource?: Uri
_remoteConnection: IJupyterConnection,
_resource?: Uri
): Pick<Session.ISessionOptions, 'path' | 'name'> | undefined | void {
if (!resource || resource.scheme === 'untitled' || !remoteConnection.mappedRemoteNotebookDir) {
return;
}
// Get Uris of both, local and remote files.
// Convert Uris to strings to Uri again, as its possible the Uris are not always compatible.
// E.g. one could be dealing with custom file system providers.
const filePath = Uri.file(resource.path);
const mappedLocalPath = Uri.file(remoteConnection.mappedRemoteNotebookDir);
if (!path.isEqualOrParent(filePath, mappedLocalPath)) {
return;
}
const sessionPath = path.relativePath(mappedLocalPath, filePath);
// If we have mapped the local dir to the remote dir, then we need to use the name of the file.
const sessionName = path.basename(resource);
if (sessionName && sessionPath) {
return {
path: sessionPath,
name: sessionName
};
}
// if (!resource || resource.scheme === 'untitled' || !remoteConnection.mappedRemoteNotebookDir) {
// return;
// }
// // Get Uris of both, local and remote files.
// // Convert Uris to strings to Uri again, as its possible the Uris are not always compatible.
// // E.g. one could be dealing with custom file system providers.
// const filePath = Uri.file(resource.path);
// const mappedLocalPath = Uri.file(remoteConnection.mappedRemoteNotebookDir);
// if (!path.isEqualOrParent(filePath, mappedLocalPath)) {
// return;
// }
// const sessionPath = path.relativePath(mappedLocalPath, filePath);
// // If we have mapped the local dir to the remote dir, then we need to use the name of the file.
// const sessionName = path.basename(resource);
// if (sessionName && sessionPath) {
// return {
// path: sessionPath,
// name: sessionName
// };
// }
}
1 change: 0 additions & 1 deletion src/kernels/jupyter/session/jupyterSession.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ suite('JupyterSession', () => {
let kernelService: JupyterKernelService;
function createJupyterSession(resource: Resource = undefined, kernelConnectionMetadata: KernelConnectionMetadata) {
connection = mock<IJupyterConnection>();
when(connection.mappedRemoteNotebookDir).thenReturn(undefined);
token = new CancellationTokenSource();
disposables.push(token);

Expand Down
5 changes: 0 additions & 5 deletions src/kernels/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,11 +558,6 @@ export interface IJupyterConnection extends Disposable {
* Returns the sub-protocols to be used. See details of `protocols` here https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/WebSocket
*/
getWebsocketProtocols?(): string[];
/**
* Maps to IJupyterServerUri.mappedRemoteNotebookDir
* @see IJupyterServerUri
*/
readonly mappedRemoteNotebookDir?: string;
}

export enum InterruptResult {
Expand Down
86 changes: 42 additions & 44 deletions src/notebooks/controllers/preferredKernelConnectionService.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { assert } from 'chai';
import * as sinon from 'sinon';
import { anything, instance, mock, when } from 'ts-mockito';
import { CancellationTokenSource, NotebookDocument, Disposable, EventEmitter, Uri } from 'vscode';
import { CancellationTokenSource, NotebookDocument, Disposable, EventEmitter } from 'vscode';
import { ContributedKernelFinderKind, IContributedKernelFinder } from '../../kernels/internalTypes';
import { PreferredRemoteKernelIdProvider } from '../../kernels/jupyter/connection/preferredRemoteKernelIdProvider';
import {
Expand Down Expand Up @@ -71,26 +71,26 @@ suite('Preferred Kernel Connection', () => {
kernelModel: instance(mock<LiveKernelModel>()),
serverProviderHandle: serverProviderHandle2
});
const remoteLiveJavaKernelConnection = LiveRemoteKernelConnectionMetadata.create({
baseUrl: '',
id: 'liveRemoteJava',
kernelModel: {
lastActivityTime: new Date(),
model: {
id: 'xyz',
kernel: {
name: 'java',
id: 'xyz'
},
path: 'baz/sample.ipynb',
name: 'sample.ipynb',
type: 'notebook'
},
name: 'java',
numberOfConnections: 1
},
serverProviderHandle: serverProviderHandle2
});
// const remoteLiveJavaKernelConnection = LiveRemoteKernelConnectionMetadata.create({
// baseUrl: '',
// id: 'liveRemoteJava',
// kernelModel: {
// lastActivityTime: new Date(),
// model: {
// id: 'xyz',
// kernel: {
// name: 'java',
// id: 'xyz'
// },
// path: 'baz/sample.ipynb',
// name: 'sample.ipynb',
// type: 'notebook'
// },
// name: 'java',
// numberOfConnections: 1
// },
// serverProviderHandle: serverProviderHandle2
// });
const remoteJavaKernelSpec = RemoteKernelSpecConnectionMetadata.create({
baseUrl: '',
id: 'remoteJavaKernelSpec',
Expand Down Expand Up @@ -178,7 +178,6 @@ suite('Preferred Kernel Connection', () => {
instance(localPythonEnvFinder)
]);
(instance(connection) as any).then = undefined;
when(connection.mappedRemoteNotebookDir).thenReturn(undefined);
when(jupyterConnection.createConnectionInfo(anything())).thenResolve(instance(connection));
preferredService = new PreferredKernelConnectionService(instance(jupyterConnection));
disposables.push(preferredService);
Expand Down Expand Up @@ -217,31 +216,30 @@ suite('Preferred Kernel Connection', () => {

assert.strictEqual(preferredKernel, remoteJavaKernelSpec);
});
test('Find existing session if there is an exact match for the notebook', async () => {
when(connection.mappedRemoteNotebookDir).thenReturn('/foo/bar/');
notebook = new TestNotebookDocument(Uri.file('/foo/bar/baz/sample.ipynb'), 'jupyter-notebook', {
custom: { metadata: notebookMetadata }
});
// test('Find existing session if there is an exact match for the notebook', async () => {
// notebook = new TestNotebookDocument(Uri.file('/foo/bar/baz/sample.ipynb'), 'jupyter-notebook', {
// custom: { metadata: notebookMetadata }
// });

when(preferredRemoteKernelProvider.getPreferredRemoteKernelId(uriEquals(notebook.uri))).thenResolve(
undefined
);
when(remoteKernelFinder.status).thenReturn('idle');
when(remoteKernelFinder.kernels).thenReturn([
remoteLiveKernelConnection1,
remoteLiveJavaKernelConnection,
remoteJavaKernelSpec
]);
notebookMetadata.language_info!.name = remoteJavaKernelSpec.kernelSpec.language!;
// when(preferredRemoteKernelProvider.getPreferredRemoteKernelId(uriEquals(notebook.uri))).thenResolve(
// undefined
// );
// when(remoteKernelFinder.status).thenReturn('idle');
// when(remoteKernelFinder.kernels).thenReturn([
// remoteLiveKernelConnection1,
// remoteLiveJavaKernelConnection,
// remoteJavaKernelSpec
// ]);
// notebookMetadata.language_info!.name = remoteJavaKernelSpec.kernelSpec.language!;

const preferredKernel = await preferredService.findPreferredRemoteKernelConnection(
notebook,
instance(remoteKernelFinder),
cancellation.token
);
// const preferredKernel = await preferredService.findPreferredRemoteKernelConnection(
// notebook,
// instance(remoteKernelFinder),
// cancellation.token
// );

assert.strictEqual(preferredKernel, remoteLiveJavaKernelConnection);
});
// assert.strictEqual(preferredKernel, remoteLiveJavaKernelConnection);
// });
});
suite('Local Kernel Specs (preferred match)', () => {
test('No match for notebook when there are no kernels', async () => {
Expand Down

0 comments on commit 226adb0

Please sign in to comment.