Skip to content
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

Remove content mgmt methods form Jupyter Session (4) #13579

Closed
wants to merge 4 commits into from
Closed

Remove content mgmt methods form Jupyter Session (4) #13579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c37e3c0
Remove serverId and URIs to identify servers
DonJayamanne May 29, 2023
d997812
Refactor RemoteURI storage and related
DonJayamanne May 30, 2023
28103c3
Remove cookies and passwords and hub
DonJayamanne May 30, 2023
3f44409
Refactor export
DonJayamanne May 30, 2023
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
10 changes: 0 additions & 10 deletions src/gdpr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,16 +967,6 @@
"${include}": [
"${F1}"

]
}
*/
//Telemetry.SetJupyterURIToUserSpecified
/* __GDPR__
"DATASCIENCE.SET_JUPYTER_URI_USER_SPECIFIED" : {
"azure": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"Was the URI set to an Azure uri.","owner":"donjayamanne"},
"${include}": [
"${F1}"

]
}
*/
Expand Down
6 changes: 2 additions & 4 deletions src/kernels/errors/invalidRemoteJupyterServerUriHandleError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ import { BaseError } from '../../platform/errors/types';
*/
export class InvalidRemoteJupyterServerUriHandleError extends BaseError {
constructor(
public readonly providerId: string,
public readonly handle: string,
public readonly extensionId: string,
public readonly serverId: string
public readonly serverHandle: { extensionId: string; id: string; handle: string },
public readonly extensionId: string
) {
super('invalidremotejupyterserverurihandle', 'Server handle not in list of known handles');
}
Expand Down
14 changes: 0 additions & 14 deletions src/kernels/errors/jupyterInvalidPassword.ts
View file
Open in desktop

This file was deleted.

35 changes: 19 additions & 16 deletions src/kernels/errors/kernelErrorHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,10 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle
error: RemoteJupyterServerUriProviderError,
errorContext?: KernelAction
) {
const server = await this.serverUriStorage.get(error.serverId);
const server = await this.serverUriStorage.get(error.serverHandle);
const message = error.originalError?.message || error.message;
const displayName = server?.displayName;
const idAndHandle = `${error.providerId}:${error.handle}`;
const idAndHandle = `${error.serverHandle.id}:${error.serverHandle.handle}`;
const serverName = displayName || idAndHandle;

return getUserFriendlyErrorMessage(
Expand All @@ -249,37 +249,38 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle
error: RemoteJupyterServerConnectionError,
errorContext?: KernelAction
) {
const info = await this.serverUriStorage.get(error.serverId);
const info = await this.serverUriStorage.get(error.serverHandle);
const message = error.originalError.message || '';

if (info?.provider) {
const serverName = info.displayName ?? error.url;
if (info?.serverHandle) {
const serverName = info.displayName ?? error.baseUrl;
return getUserFriendlyErrorMessage(
DataScience.remoteJupyterConnectionFailedWithServerWithError(serverName, message),
errorContext
);
}

const baseUrl = error.baseUrl;
const serverName =
info?.displayName && baseUrl ? `${info.displayName} (${baseUrl})` : info?.displayName || baseUrl;
info?.displayName && error.baseUrl
? `${info.displayName} (${error.baseUrl})`
: info?.displayName || error.baseUrl;

return getUserFriendlyErrorMessage(
DataScience.remoteJupyterConnectionFailedWithServerWithError(serverName, message),
errorContext
);
}
private async handleJupyterServerProviderConnectionError(info: IJupyterServerUriEntry) {
const provider = await this.jupyterUriProviderRegistration.getProvider(info.serverId);
const provider = await this.jupyterUriProviderRegistration.getProvider(info.serverHandle.id);
if (!provider || !provider.getHandles) {
return false;
}

try {
const handles = await provider.getHandles();

if (!handles.includes(info.provider.handle)) {
await this.serverUriStorage.remove(info.serverId);
if (!handles.includes(info.serverHandle.handle)) {
await this.serverUriStorage.remove(info.serverHandle);
}
return true;
} catch (_ex) {
Expand Down Expand Up @@ -346,12 +347,14 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle
: err instanceof RemoteJupyterServerConnectionError
? err.originalError.message || ''
: err.originalError?.message || err.message;
const serverId = err instanceof RemoteJupyterServerConnectionError ? err.serverId : err.serverId;
const server = await this.serverUriStorage.get(serverId);
const provider = err.serverHandle;
const server = await this.serverUriStorage.get(provider);
const displayName = server?.displayName;
const baseUrl = err instanceof RemoteJupyterServerConnectionError ? err.baseUrl : '';
const idAndHandle =
err instanceof RemoteJupyterServerUriProviderError ? `${err.providerId}:${err.handle}` : '';
err instanceof RemoteJupyterServerUriProviderError
? `${err.serverHandle.id}:${err.serverHandle.handle}`
: '';
const serverName =
displayName && baseUrl ? `${displayName} (${baseUrl})` : displayName || baseUrl || idAndHandle;
const extensionName =
Expand All @@ -375,9 +378,9 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle
switch (selection) {
case DataScience.removeRemoteJupyterConnectionButtonText: {
// Remove this uri if already found (going to add again with a new time)
const item = await this.serverUriStorage.get(serverId);
if (item) {
await this.serverUriStorage.remove(item.serverId);
const item = provider ? await this.serverUriStorage.get(provider) : undefined;
if (item && provider) {
await this.serverUriStorage.remove(provider);
}
// Wait until all of the remote controllers associated with this server have been removed.
return KernelInterpreterDependencyResponse.cancel;
Expand Down
89 changes: 49 additions & 40 deletions src/kernels/errors/kernelErrorHandler.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ import {
IJupyterInterpreterDependencyManager,
IJupyterServerUriStorage,
IJupyterUriProviderRegistration,
JupyterInterpreterDependencyResponse
JupyterInterpreterDependencyResponse,
JupyterServerProviderHandle
} from '../jupyter/types';
import { getDisplayNameOrNameOfKernelConnection } from '../helpers';
import { getOSType, OSType } from '../../platform/common/utils/platform';
import { RemoteJupyterServerConnectionError } from '../../platform/errors/remoteJupyterServerConnectionError';
import { computeServerId, generateUriFromRemoteProvider } from '../jupyter/jupyterUtils';
import { jupyterServerHandleToString } from '../jupyter/jupyterUtils';
import { RemoteJupyterServerUriProviderError } from './remoteJupyterServerUriProviderError';
import { IReservedPythonNamedProvider } from '../../platform/interpreter/types';
import { DataScienceErrorHandlerNode } from './kernelErrorHandler.node';
Expand Down Expand Up @@ -151,15 +152,11 @@ suite('Error Handler Unit Tests', () => {

suite('Kernel startup errors', () => {
let kernelConnection: KernelConnectionMetadata;
const uri = generateUriFromRemoteProvider('1', 'a');
let serverId: string;
suiteSetup(async () => {
serverId = await computeServerId(uri);
});
const serverHandle: JupyterServerProviderHandle = { extensionId: 'ext', id: '1', handle: 'a' };
const serverHandleId = jupyterServerHandleToString(serverHandle);
setup(() => {
when(applicationShell.showErrorMessage(anything(), Common.learnMore)).thenResolve(Common.learnMore as any);
kernelConnection = PythonKernelConnectionMetadata.create({
id: '',
interpreter: {
uri: Uri.file('Hello There'),
id: Uri.file('Hello There').fsPath,
Expand Down Expand Up @@ -785,17 +782,20 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d
);
});
test('Display error when connection to remote jupyter server fails', async () => {
const error = new RemoteJupyterServerConnectionError(uri, serverId, new Error('ECONNRESET error'));
const connection = RemoteKernelSpecConnectionMetadata.create({
const error = new RemoteJupyterServerConnectionError(
serverHandleId,
serverHandle,
new Error('ECONNRESET error')
);
const connection = await RemoteKernelSpecConnectionMetadata.create({
baseUrl: 'http://hello:1234/',
id: '1',
kernelSpec: {
argv: [],
display_name: '',
name: '',
executable: ''
},
serverId
serverHandle
});
when(
applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything())
Expand All @@ -818,27 +818,24 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d
DataScience.selectDifferentKernel
)
).once();
verify(uriStorage.remove(serverId)).never();
verify(uriStorage.remove(anything())).never();
});
test('Display error when connection to remote jupyter server fails due to 3rd party extension', async () => {
const error = new RemoteJupyterServerUriProviderError('1', 'a', new Error('invalid handle'), serverId);
const connection = RemoteKernelSpecConnectionMetadata.create({
const error = new RemoteJupyterServerUriProviderError(serverHandle, new Error('invalid handle'));
const connection = await RemoteKernelSpecConnectionMetadata.create({
baseUrl: 'http://hello:1234/',
id: '1',
kernelSpec: {
argv: [],
display_name: '',
name: '',
executable: ''
},
serverId
serverHandle: serverHandle
});
when(uriStorage.get(serverId)).thenResolve({
when(uriStorage.get(anything())).thenResolve({
time: 1,
uri,
serverId,
displayName: 'Hello Server',
provider: { id: '1', handle: 'a' }
serverHandle: serverHandle
});
when(
applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything())
Expand All @@ -861,27 +858,33 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d
DataScience.selectDifferentKernel
)
).once();
verify(uriStorage.remove(serverId)).never();
verify(uriStorage.get(serverId)).atLeast(1);
verify(uriStorage.remove(anything())).never();
verify(uriStorage.get(anything())).atLeast(1);
});
test('Remove remote Uri if user choses to do so, when connection to remote jupyter server fails', async () => {
const error = new RemoteJupyterServerConnectionError(uri, serverId, new Error('ECONNRESET error'));
const connection = RemoteKernelSpecConnectionMetadata.create({
const error = new RemoteJupyterServerConnectionError(
serverHandleId,
serverHandle,
new Error('ECONNRESET error')
);
const connection = await RemoteKernelSpecConnectionMetadata.create({
baseUrl: 'http://hello:1234/',
id: '1',
kernelSpec: {
argv: [],
display_name: '',
name: '',
executable: '' // Send nothing for argv[0]
},
serverId
serverHandle
});
when(
applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything())
).thenResolve(DataScience.removeRemoteJupyterConnectionButtonText as any);
when(uriStorage.remove(anything())).thenResolve();
when(uriStorage.get(serverId)).thenResolve({ uri, serverId, time: 2, provider: { id: '1', handle: 'a' } });
when(uriStorage.get(anything())).thenResolve({
time: 2,
serverHandle
});
const result = await dataScienceErrorHandler.handleKernelError(
error,
'start',
Expand All @@ -890,21 +893,24 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d
'jupyterExtension'
);
assert.strictEqual(result, KernelInterpreterDependencyResponse.cancel);
verify(uriStorage.remove(serverId)).once();
verify(uriStorage.get(serverId)).atLeast(1);
verify(uriStorage.remove(anything())).once();
verify(uriStorage.get(anything())).atLeast(1);
});
test('Change remote Uri if user choses to do so, when connection to remote jupyter server fails', async () => {
const error = new RemoteJupyterServerConnectionError(uri, serverId, new Error('ECONNRESET error'));
const connection = RemoteKernelSpecConnectionMetadata.create({
const error = new RemoteJupyterServerConnectionError(
serverHandleId,
serverHandle,
new Error('ECONNRESET error')
);
const connection = await RemoteKernelSpecConnectionMetadata.create({
baseUrl: 'http://hello:1234/',
id: '1',
kernelSpec: {
argv: [],
display_name: '',
name: '',
executable: ''
},
serverId
serverHandle
});
when(
applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything())
Expand All @@ -918,20 +924,23 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d
'jupyterExtension'
);
assert.strictEqual(result, KernelInterpreterDependencyResponse.cancel);
verify(uriStorage.remove(serverId)).never();
verify(uriStorage.remove(anything())).never();
});
test('Select different kernel user choses to do so, when connection to remote jupyter server fails', async () => {
const error = new RemoteJupyterServerConnectionError(uri, serverId, new Error('ECONNRESET error'));
const connection = RemoteKernelSpecConnectionMetadata.create({
const error = new RemoteJupyterServerConnectionError(
serverHandleId,
serverHandle,
new Error('ECONNRESET error')
);
const connection = await RemoteKernelSpecConnectionMetadata.create({
baseUrl: 'http://hello:1234/',
id: '1',
kernelSpec: {
argv: [],
display_name: '',
name: '',
executable: ''
},
serverId
serverHandle
});
when(
applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything())
Expand All @@ -944,7 +953,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d
'jupyterExtension'
);
assert.strictEqual(result, KernelInterpreterDependencyResponse.selectDifferentKernel);
verify(uriStorage.remove(serverId)).never();
verify(uriStorage.remove(anything())).never();
});
function verifyErrorMessage(message: string, linkInfo?: string) {
message = message.includes('command:jupyter.viewOutput')
Expand Down
6 changes: 2 additions & 4 deletions src/kernels/errors/remoteJupyterServerUriProviderError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ import { BaseError } from '../../platform/errors/types';
*/
export class RemoteJupyterServerUriProviderError extends BaseError {
constructor(
public readonly providerId: string,
public readonly handle: string,
public readonly originalError: Error,
public serverId: string
public readonly serverHandle: { extensionId: string; id: string; handle: string },
public readonly originalError: Error
) {
super('remotejupyterserveruriprovider', originalError.message || originalError.toString());
}
Expand Down
5 changes: 0 additions & 5 deletions src/kernels/execution/helpers.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ suite(`UpdateNotebookMetadata`, () => {
test('Update Language', async () => {
const notebookMetadata = { orig_nbformat: 4, language_info: { name: 'JUNK' } };
const kernelConnection = PythonKernelConnectionMetadata.create({
id: 'python36',
interpreter: python36Global,
kernelSpec: pythonDefaultKernelSpec
});
Expand All @@ -68,7 +67,6 @@ suite(`UpdateNotebookMetadata`, () => {
test('Update Python Version', async () => {
const notebookMetadata = { orig_nbformat: 4, language_info: { name: 'python', version: '3.6.0' } };
const kernelConnection = PythonKernelConnectionMetadata.create({
id: 'python36',
interpreter: python37Global,
kernelSpec: pythonDefaultKernelSpec
});
Expand All @@ -90,7 +88,6 @@ suite(`UpdateNotebookMetadata`, () => {
language_info: { name: 'python', version: '3.6.0' }
};
const kernelConnection = PythonKernelConnectionMetadata.create({
id: 'python36',
interpreter: python36Global,
kernelSpec: pythonDefaultKernelSpec
});
Expand Down Expand Up @@ -122,7 +119,6 @@ suite(`UpdateNotebookMetadata`, () => {
language_info: { name: 'python', version: '3.6.0' }
};
const kernelConnection = PythonKernelConnectionMetadata.create({
id: 'python36',
interpreter: python36Global,
kernelSpec: pythonDefaultKernelSpec
});
Expand Down Expand Up @@ -170,7 +166,6 @@ suite(`UpdateNotebookMetadata`, () => {
language_info: { name: 'python', version: '3.6.0' }
};
const kernelConnection = PythonKernelConnectionMetadata.create({
id: 'python36',
interpreter: python36Global,
kernelSpec: pythonDefaultKernelSpec
});
Expand Down
Loading