Skip to content

Commit

Permalink
Stop using version from old Python API (#15096)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored Jan 31, 2024
1 parent 3fc4a02 commit b0065ba
Show file tree
Hide file tree
Showing 13 changed files with 244 additions and 46 deletions.
4 changes: 4 additions & 0 deletions src/kernels/errors/kernelErrorHandler.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock
import { dispose } from '../../platform/common/utils/lifecycle';
import { PythonExtension } from '@vscode/python-extension';
import { resolvableInstance } from '../../test/datascience/helpers';
import { setPythonApi } from '../../platform/interpreter/helpers';

suite('Error Handler Unit Tests', () => {
let dataScienceErrorHandler: DataScienceErrorHandler;
Expand Down Expand Up @@ -108,6 +109,9 @@ suite('Error Handler Unit Tests', () => {
disposables.push({ dispose: () => sinon.restore() });
environments = mock<PythonExtension['environments']>();
when(mockedApi.environments).thenReturn(instance(environments));
when(environments.known).thenReturn([]);
setPythonApi(instance(mockedApi));
disposables.push({ dispose: () => setPythonApi(undefined as any) });
when(environments.resolveEnvironment(jupyterInterpreter.id)).thenResolve({
executable: { sysPrefix: '' }
} as any);
Expand Down
8 changes: 4 additions & 4 deletions src/kernels/execution/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
} from '../helpers';
import { StopWatch } from '../../platform/common/utils/stopWatch';
import { getExtensionSpecifcStack } from '../../platform/errors/errors';
import { getVersion } from '../../platform/interpreter/helpers';

export enum CellOutputMimeTypes {
error = 'application/vnd.code.notebook.error',
Expand Down Expand Up @@ -706,12 +707,11 @@ export async function updateNotebookMetadata(
const interpreter = isPythonConnection
? getInterpreterFromKernelConnectionMetadata(kernelConnection)
: undefined;
const version = interpreter?.version
? `${interpreter.version.major}.${interpreter.version.minor}.${interpreter.version.patch}`
: '';
const versionInfo = await getVersion(interpreter);
const version = versionInfo ? `${versionInfo.major}.${versionInfo.minor}.${versionInfo.micro}` : '';
if (
interpreter &&
interpreter.version &&
versionInfo &&
metadata &&
metadata.language_info &&
metadata.language_info.version !== version
Expand Down
34 changes: 32 additions & 2 deletions src/kernels/execution/helpers.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import * as sinon from 'sinon';
import type * as nbformat from '@jupyterlab/nbformat';
import { assert } from 'chai';
import { Uri } from 'vscode';
import { updateNotebookMetadata } from './helpers';
import { IJupyterKernelSpec, PythonKernelConnectionMetadata } from '../types';
import { EnvironmentType, PythonEnvironment } from '../../platform/pythonEnvironments/info';
import { PythonExtension } from '@vscode/python-extension';
import { instance, mock, when } from 'ts-mockito';
import { resolvableInstance } from '../../test/datascience/helpers';
import { setPythonApi } from '../../platform/interpreter/helpers';
import { dispose } from '../../platform/common/utils/lifecycle';

// Function return type
// type updateNotebookMetadataReturn = { changed: boolean; kernelId: string | undefined };
Expand All @@ -27,14 +33,38 @@ suite(`UpdateNotebookMetadata`, () => {
executable: 'python'
};
const python37Global: PythonEnvironment = {
uri: Uri.file('/usr/bin/python36'),
id: Uri.file('/usr/bin/python36').fsPath,
uri: Uri.file('/usr/bin/python37'),
id: Uri.file('/usr/bin/python37').fsPath,
sysPrefix: '/usr',
displayName: 'Python 3.7',
envType: EnvironmentType.Unknown,
sysVersion: '3.7.0',
version: { major: 3, minor: 7, patch: 0, raw: '3.7.0' }
};
let environments: PythonExtension['environments'];
let disposables: { dispose: () => void }[] = [];
setup(() => {
const mockedApi = mock<PythonExtension>();
sinon.stub(PythonExtension, 'api').resolves(resolvableInstance(mockedApi));
disposables.push({ dispose: () => sinon.restore() });
environments = mock<PythonExtension['environments']>();
when(mockedApi.environments).thenReturn(instance(environments));
when(environments.known).thenReturn([
{
id: python36Global.id,
version: { major: 3, minor: 6, micro: 0, sysVersion: '3.6.0' }
} as any,
{
id: python37Global.id,
version: { major: 3, minor: 7, micro: 0, sysVersion: '3.7.0' }
} as any
]);
setPythonApi(instance(mockedApi));
disposables.push({ dispose: () => setPythonApi(undefined as any) });
});
teardown(() => {
disposables = dispose(disposables);
});
test('Empty call does not change anything', async () => {
const value = await updateNotebookMetadata();
assert.strictEqual(value.changed, false);
Expand Down
6 changes: 3 additions & 3 deletions src/kernels/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { JupyterKernelSpec } from './jupyter/jupyterKernelSpec';
import { sendTelemetryEvent } from '../telemetry';
import { IPlatformService } from '../platform/common/platform/types';
import { splitLines } from '../platform/common/helpers';
import { getPythonEnvironmentName } from '../platform/interpreter/helpers';
import { getCachedVersion, getPythonEnvironmentName } from '../platform/interpreter/helpers';
import { cellOutputToVSCCellOutput } from './execution/helpers';
import { handleTensorBoardDisplayDataOutput } from './execution/executionHelpers';
import { once } from '../platform/common/utils/functional';
Expand Down Expand Up @@ -261,7 +261,7 @@ export function getDisplayNameOrNameOfKernelConnection(kernelConnection: KernelC
const envName = getPythonEnvironmentName(kernelConnection.interpreter);
if (kernelConnection.kernelSpec.language === PYTHON_LANGUAGE) {
const pythonVersion = `Python ${
getTelemetrySafeVersion(kernelConnection.interpreter.version?.raw || '') || ''
getTelemetrySafeVersion(getCachedVersion(kernelConnection.interpreter)) || ''
}`.trim();
return kernelConnection.interpreter.envName
? `${oldDisplayName} (${pythonVersion})`
Expand All @@ -276,7 +276,7 @@ export function getDisplayNameOrNameOfKernelConnection(kernelConnection: KernelC
}
case 'startUsingPythonInterpreter':
const pythonVersion = (
getTelemetrySafeVersion(kernelConnection.interpreter.version?.raw || '') || ''
getTelemetrySafeVersion(getCachedVersion(kernelConnection.interpreter)) || ''
).trim();
if (
kernelConnection.interpreter.envType &&
Expand Down
105 changes: 100 additions & 5 deletions src/kernels/helpers.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import * as sinon from 'sinon';
import { assert } from 'chai';
import { when, instance, mock } from 'ts-mockito';
import { Uri } from 'vscode';
Expand All @@ -12,8 +13,27 @@ import {
PythonKernelConnectionMetadata
} from './types';
import { EnvironmentType, PythonEnvironment } from '../platform/pythonEnvironments/info';
import { PythonExtension } from '@vscode/python-extension';
import { resolvableInstance } from '../test/datascience/helpers';
import { dispose } from '../platform/common/utils/lifecycle';
import { setPythonApi } from '../platform/interpreter/helpers';

suite('Kernel Connection Helpers', () => {
let environments: PythonExtension['environments'];
let disposables: { dispose: () => void }[] = [];
setup(() => {
const mockedApi = mock<PythonExtension>();
sinon.stub(PythonExtension, 'api').resolves(resolvableInstance(mockedApi));
disposables.push({ dispose: () => sinon.restore() });
environments = mock<PythonExtension['environments']>();
when(mockedApi.environments).thenReturn(instance(environments));
when(environments.known).thenReturn([]);
setPythonApi(instance(mockedApi));
disposables.push({ dispose: () => setPythonApi(undefined as any) });
});
teardown(() => {
disposables = dispose(disposables);
});
test('Live kernels should display the name`', () => {
const name = getDisplayNameOrNameOfKernelConnection(
LiveRemoteKernelConnectionMetadata.create({
Expand Down Expand Up @@ -257,7 +277,7 @@ suite('Kernel Connection Helpers', () => {
test('Display name if kernel is associated with a non-global Python environment', () => {
const name = getDisplayNameOrNameOfKernelConnection(
LocalKernelSpecConnectionMetadata.create({
id: '',
id: '1',
kernelSpec: {
argv: [],
display_name: 'kspecname',
Expand Down Expand Up @@ -331,6 +351,18 @@ suite('Kernel Connection Helpers', () => {
assert.strictEqual(name, 'kspecname');
});
test('Prefixed with `<env name>` kernel is associated with a non-global Python environment', () => {
when(environments.known).thenReturn([
{
id: Uri.file('pyPath').fsPath,
version: {
major: 9,
minor: 8,
micro: 7,
release: undefined,
sysVersion: '9.8.7.6-pre'
}
} as any
]);
const name = getDisplayNameOrNameOfKernelConnection(
LocalKernelSpecConnectionMetadata.create({
id: '',
Expand All @@ -350,7 +382,7 @@ suite('Kernel Connection Helpers', () => {
version: {
major: 9,
minor: 8,
patch: 1,
patch: 7,
raw: '9.8.7.6-pre'
},
envType: EnvironmentType.Conda
Expand All @@ -360,6 +392,18 @@ suite('Kernel Connection Helpers', () => {
assert.strictEqual(name, 'kspecname (Python 9.8.7)');
});
test('Prefixed with `<env name>` kernel is associated with a non-global 64-bit Python environment', () => {
when(environments.known).thenReturn([
{
id: Uri.file('pyPath').fsPath,
version: {
major: 9,
minor: 8,
micro: 7,
release: undefined,
sysVersion: '9.8.7'
}
} as any
]);
const name = getDisplayNameOrNameOfKernelConnection(
LocalKernelSpecConnectionMetadata.create({
id: '',
Expand All @@ -379,7 +423,7 @@ suite('Kernel Connection Helpers', () => {
version: {
major: 9,
minor: 8,
patch: 1,
patch: 7,
raw: '9.8.7.6-pre'
},
envType: EnvironmentType.Conda
Expand Down Expand Up @@ -436,6 +480,18 @@ suite('Kernel Connection Helpers', () => {
assert.strictEqual(name, 'Python');
});
test('Return Python Version for global python environment with a version', () => {
when(environments.known).thenReturn([
{
id: Uri.file('pyPath').fsPath,
version: {
major: 1,
minor: 2,
micro: 3,
release: undefined,
sysVersion: '1.2.3'
}
} as any
]);
const name = getDisplayNameOrNameOfKernelConnection(
PythonKernelConnectionMetadata.create({
id: '',
Expand Down Expand Up @@ -509,6 +565,7 @@ suite('Kernel Connection Helpers', () => {
const kernelSpec = mock<IJupyterKernelSpec>();
const interpreter = mock<PythonEnvironment>();
when(kernelSpec.language).thenReturn('python');
when(interpreter.id).thenReturn('xyz');
when(interpreter.envName).thenReturn('');
when(interpreter.version).thenReturn({
major: 9,
Expand All @@ -518,6 +575,18 @@ suite('Kernel Connection Helpers', () => {
});
when(interpreter.displayName).thenReturn('Something 64-bit');
when(interpreter.envType).thenReturn(EnvironmentType.Pipenv);
when(environments.known).thenReturn([
{
id: instance(interpreter).id,
version: {
major: 9,
minor: 8,
micro: 7,
release: undefined,
sysVersion: '9.8.7.6-pre'
}
} as any
]);

const name = getDisplayNameOrNameOfKernelConnection(
PythonKernelConnectionMetadata.create({
Expand All @@ -532,15 +601,28 @@ suite('Kernel Connection Helpers', () => {
const kernelSpec = mock<IJupyterKernelSpec>();
const interpreter = mock<PythonEnvironment>();
when(kernelSpec.language).thenReturn('python');
when(interpreter.id).thenReturn('xyz');
when(interpreter.envName).thenReturn('.env');
when(interpreter.version).thenReturn({
major: 9,
minor: 8,
patch: 1,
patch: 7,
raw: '9.8.7.6-pre'
});
when(interpreter.displayName).thenReturn('Something');
when(interpreter.envType).thenReturn(EnvironmentType.Conda);
when(environments.known).thenReturn([
{
id: instance(interpreter).id,
version: {
major: 9,
minor: 8,
micro: 7,
release: undefined,
sysVersion: '9.8.7.6-pre'
}
} as any
]);

const name = getDisplayNameOrNameOfKernelConnection(
PythonKernelConnectionMetadata.create({
Expand All @@ -555,15 +637,28 @@ suite('Kernel Connection Helpers', () => {
const kernelSpec = mock<IJupyterKernelSpec>();
const interpreter = mock<PythonEnvironment>();
when(kernelSpec.language).thenReturn('python');
when(interpreter.id).thenReturn('xyz');
when(interpreter.envName).thenReturn('.env');
when(interpreter.version).thenReturn({
major: 9,
minor: 8,
patch: 1,
patch: 7,
raw: '9.8.7.6-pre'
});
when(interpreter.displayName).thenReturn('Something 64-bit');
when(interpreter.envType).thenReturn(EnvironmentType.Conda);
when(environments.known).thenReturn([
{
id: instance(interpreter).id,
version: {
major: 9,
minor: 8,
micro: 7,
release: undefined,
sysVersion: '9.8.7.6-pre'
}
} as any
]);

const name = getDisplayNameOrNameOfKernelConnection(
PythonKernelConnectionMetadata.create({
Expand Down
6 changes: 4 additions & 2 deletions src/kernels/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import type { IAnyMessageArgs } from '@jupyterlab/services/lib/kernel/kernel';
import { getKernelInfo } from './kernelInfo';
import { KernelInterruptTimeoutError } from './errors/kernelInterruptTimeoutError';
import { dispose } from '../platform/common/utils/lifecycle';
import { getCachedVersion } from '../platform/interpreter/helpers';

const widgetVersionOutPrefix = 'e976ee50-99ed-4aba-9b6b-9dcd5634d07d:IPyWidgets:';
/**
Expand Down Expand Up @@ -585,8 +586,9 @@ abstract class BaseKernel implements IBaseKernel {
info.push(`Python Path: ${getDisplayPath(interpreter.uri)}`);
info.push(interpreter.envType || '');
info.push(interpreter.envName || '');
if (interpreter.version) {
info.push(`${interpreter.version.major}.${interpreter.version.minor}.${interpreter.version.patch}`);
const version = getCachedVersion(interpreter);
if (version) {
info.push(`${version.major}.${version.minor}.${version.micro}`);
}
pythonInfo = ` (${info.filter((s) => s).join(', ')})`;
}
Expand Down
12 changes: 10 additions & 2 deletions src/kernels/raw/finder/pythonKernelInterruptDaemon.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { EOL } from 'os';
import { swallowExceptions } from '../../../platform/common/utils/misc';
import { splitLines } from '../../../platform/common/helpers';
import { IPythonExecutionFactory } from '../../../platform/interpreter/types.node';
import { getCachedVersion } from '../../../platform/interpreter/helpers';
function isBestPythonInterpreterForAnInterruptDaemon(interpreter: PythonEnvironment) {
// Give preference to globally installed python environments.
// The assumption is that users are more likely to uninstall/delete local python environments
Expand All @@ -33,11 +34,18 @@ function isBestPythonInterpreterForAnInterruptDaemon(interpreter: PythonEnvironm
return false;
}
function isSupportedPythonVersion(interpreter: PythonEnvironment) {
let major = interpreter?.version?.major ?? 3;
let minor = interpreter?.version?.minor ?? 6;
const version = getCachedVersion(interpreter);
if (version) {
major = version.major || major;
minor = version.minor || minor;
}
if (
(interpreter?.version?.major ?? 3) >= 3 &&
major >= 3 &&
// Even thought 3.6 is no longer supported, we know this works well enough for what we want.
// This way we don't need to update this every time the supported version changes.
(interpreter?.version?.minor ?? 6) >= 6
minor >= 6
) {
return true;
}
Expand Down
Loading

0 comments on commit b0065ba

Please sign in to comment.