From 35b04ef375d424e43a81e0528d7aa06082534dec Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 27 Feb 2024 16:10:12 +1100 Subject: [PATCH 1/2] Remove PythonEnvironment property isCondaEnvWithoutPython --- src/kernels/errors/kernelErrorHandler.ts | 12 ++++++--- src/kernels/helpers.ts | 10 +++++--- .../finder/localKernelSpecFinderBase.node.ts | 3 ++- .../controllers/connectionDisplayData.node.ts | 6 ++++- .../controllers/connectionDisplayData.ts | 3 ++- .../kernelSourceCommandHandler.ts | 3 ++- ...localPythonEnvKernelSourceSelector.node.ts | 5 +--- src/platform/api/pythonApi.ts | 25 +++++++++---------- .../environmentActivationService.node.ts | 2 +- src/platform/interpreter/helpers.ts | 21 ++++++++++------ .../installer/condaInstaller.node.ts | 11 ++------ .../installer/productInstaller.node.ts | 3 --- .../interpreter/interpreterPackages.node.ts | 3 ++- src/platform/pythonEnvironments/info/index.ts | 3 +-- 14 files changed, 58 insertions(+), 52 deletions(-) diff --git a/src/kernels/errors/kernelErrorHandler.ts b/src/kernels/errors/kernelErrorHandler.ts index b19cba6afc3..f7df79a512e 100644 --- a/src/kernels/errors/kernelErrorHandler.ts +++ b/src/kernels/errors/kernelErrorHandler.ts @@ -59,7 +59,11 @@ import { IInterpreterService } from '../../platform/interpreter/contracts'; import { PackageNotInstalledWindowsLongPathNotEnabledError } from '../../platform/errors/packageNotInstalledWindowsLongPathNotEnabledError'; import { JupyterNotebookNotInstalled } from '../../platform/errors/jupyterNotebookNotInstalled'; import { fileToCommandArgument } from '../../platform/common/helpers'; -import { getPythonEnvDisplayName, getSysPrefix } from '../../platform/interpreter/helpers'; +import { + getPythonEnvDisplayName, + getSysPrefix, + isCondaEnvironmentWithoutPython +} from '../../platform/interpreter/helpers'; import { JupyterServerCollection } from '../../api'; import { getJupyterDisplayName } from '../jupyter/connection/jupyterServerProviderRegistry'; @@ -419,14 +423,14 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle if (details) { sendKernelTelemetryEvent(resource, Telemetry.KernelStartFailureDueToMissingEnv, undefined, { envMissingReason: 'Unknown', - isEmptyCondaEnv: details.isCondaEnvWithoutPython, + isEmptyCondaEnv: isCondaEnvironmentWithoutPython(details), pythonEnvType: details.envType, fileExists }); } else { sendKernelTelemetryEvent(resource, Telemetry.KernelStartFailureDueToMissingEnv, undefined, { envMissingReason: 'EmptyEnvDetailsFromPython', - isEmptyCondaEnv: kernelConnection.interpreter.isCondaEnvWithoutPython, + isEmptyCondaEnv: isCondaEnvironmentWithoutPython(kernelConnection.interpreter), pythonEnvType: kernelConnection.interpreter.envType, fileExists }); @@ -436,7 +440,7 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle const fileExists = await this.fs.exists(kernelConnection.interpreter.uri); sendKernelTelemetryEvent(resource, Telemetry.KernelStartFailureDueToMissingEnv, undefined, { envMissingReason: 'FailedToGetEnvDetailsFromPython', - isEmptyCondaEnv: kernelConnection.interpreter.isCondaEnvWithoutPython, + isEmptyCondaEnv: isCondaEnvironmentWithoutPython(kernelConnection.interpreter), pythonEnvType: kernelConnection.interpreter.envType, fileExists }); diff --git a/src/kernels/helpers.ts b/src/kernels/helpers.ts index 797e4e6386d..9cb2e3b9524 100644 --- a/src/kernels/helpers.ts +++ b/src/kernels/helpers.ts @@ -26,7 +26,11 @@ import { JupyterKernelSpec } from './jupyter/jupyterKernelSpec'; import { sendTelemetryEvent } from '../telemetry'; import { IPlatformService } from '../platform/common/platform/types'; import { splitLines } from '../platform/common/helpers'; -import { getCachedVersion, getPythonEnvironmentName } from '../platform/interpreter/helpers'; +import { + getCachedVersion, + getPythonEnvironmentName, + isCondaEnvironmentWithoutPython +} from '../platform/interpreter/helpers'; import { cellOutputToVSCCellOutput } from './execution/helpers'; import { handleTensorBoardDisplayDataOutput } from './execution/executionHelpers'; import { once } from '../platform/common/utils/functional'; @@ -291,9 +295,7 @@ export function getDisplayNameOrNameOfKernelConnection(kernelConnection: KernelC return kernelConnection.kernelSpec.display_name; } // If this is a conda environment without Python, then don't display `Python` in it. - const isCondaEnvWithoutPython = - kernelConnection.interpreter.envType === EnvironmentType.Conda && - kernelConnection.interpreter.isCondaEnvWithoutPython === true; + const isCondaEnvWithoutPython = isCondaEnvironmentWithoutPython(kernelConnection.interpreter); const pythonDisplayName = pythonVersion.trim() ? `Python ${pythonVersion}` : 'Python'; const envName = getPythonEnvironmentName(kernelConnection.interpreter); if (isCondaEnvWithoutPython && envName) { diff --git a/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts b/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts index 881ab8e4e6d..33a7160f29e 100644 --- a/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts +++ b/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts @@ -26,6 +26,7 @@ import { getComparisonKey } from '../../../platform/vscode-path/resources'; import { PromiseMonitor } from '../../../platform/common/utils/promises'; import { dispose } from '../../../platform/common/utils/lifecycle'; import { JupyterPaths } from './jupyterPaths.node'; +import { isCondaEnvironmentWithoutPython } from '../../../platform/interpreter/helpers'; export type KernelSpecFileWithContainingInterpreter = { interpreter?: PythonEnvironment; kernelSpecFile: Uri }; export const isDefaultPythonKernelSpecSpecName = /python\s\d*.?\d*$/; @@ -386,7 +387,7 @@ export async function loadKernelSpec( // Possible user deleted the underlying interpreter. const interpreterPath = interpreter?.uri.fsPath || kernelJson?.metadata?.interpreter?.path; - const isEmptyCondaEnv = interpreter?.isCondaEnvWithoutPython ? true : false; + const isEmptyCondaEnv = isCondaEnvironmentWithoutPython(interpreter); if (interpreterPath && !isEmptyCondaEnv && !(await fs.exists(Uri.file(interpreterPath)))) { return; } diff --git a/src/notebooks/controllers/connectionDisplayData.node.ts b/src/notebooks/controllers/connectionDisplayData.node.ts index 04ae7ec4219..92977f9611a 100644 --- a/src/notebooks/controllers/connectionDisplayData.node.ts +++ b/src/notebooks/controllers/connectionDisplayData.node.ts @@ -21,6 +21,7 @@ import { } from './connectionDisplayData'; import { DataScience } from '../../platform/common/utils/localize'; import { getJupyterDisplayName } from '../../kernels/jupyter/connection/jupyterServerProviderRegistry'; +import { isCondaEnvironmentWithoutPython } from '../../platform/interpreter/helpers'; @injectable() export class ConnectionDisplayDataProvider implements IConnectionDisplayDataProvider { @@ -57,7 +58,10 @@ export class ConnectionDisplayDataProvider implements IConnectionDisplayDataProv this.details.set(connection.id, newDetails); // If the interpreter information changes, then update the display data. - if (connection.kind === 'startUsingPythonInterpreter' && connection.interpreter.isCondaEnvWithoutPython) { + if ( + connection.kind === 'startUsingPythonInterpreter' && + isCondaEnvironmentWithoutPython(connection.interpreter) + ) { const updateInterpreterInfo = (e: PythonEnvironment[]) => { const changedEnv = e.find((env) => env.id === connection.interpreter?.id); const interpreter = this.interpreters.resolvedEnvironments.find((env) => env.id === changedEnv?.id); diff --git a/src/notebooks/controllers/connectionDisplayData.ts b/src/notebooks/controllers/connectionDisplayData.ts index f11ba3405f5..645b8fc0bd4 100644 --- a/src/notebooks/controllers/connectionDisplayData.ts +++ b/src/notebooks/controllers/connectionDisplayData.ts @@ -8,6 +8,7 @@ import { IDisposable } from '../../platform/common/types'; import { DataScience } from '../../platform/common/utils/localize'; import { EnvironmentType } from '../../platform/pythonEnvironments/info'; import { IConnectionDisplayData } from './types'; +import { isCondaEnvironmentWithoutPython } from '../../platform/interpreter/helpers'; export class ConnectionDisplayData implements IDisposable, IConnectionDisplayData { private readonly _onDidChange = new EventEmitter(); @@ -57,7 +58,7 @@ export function getKernelConnectionCategorySync(kernelConnection: KernelConnecti } switch (kernelConnection.interpreter.envType) { case EnvironmentType.Conda: - return kernelConnection.interpreter.isCondaEnvWithoutPython + return isCondaEnvironmentWithoutPython(kernelConnection.interpreter) ? DataScience.kernelCategoryForCondaWithoutPython : DataScience.kernelCategoryForConda; case EnvironmentType.Pipenv: diff --git a/src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts b/src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts index 60fbf8d64d2..66eaa8c3bda 100644 --- a/src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts +++ b/src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts @@ -48,6 +48,7 @@ import { IVSCodeNotebookController } from '../types'; import { JupyterServerCollection } from '../../../api'; +import { isCondaEnvironmentWithoutPython } from '../../../platform/interpreter/helpers'; @injectable() export class KernelSourceCommandHandler implements IExtensionSyncActivationService { @@ -294,7 +295,7 @@ export class KernelSourceCommandHandler implements IExtensionSyncActivationServi if ( isLocalConnection(controller.connection) && isPythonKernelConnection(controller.connection) && - controller.connection.interpreter?.isCondaEnvWithoutPython && + isCondaEnvironmentWithoutPython(controller.connection.interpreter) && !isWebExtension() ) { const disposables: IDisposable[] = []; diff --git a/src/notebooks/controllers/kernelSource/localPythonEnvKernelSourceSelector.node.ts b/src/notebooks/controllers/kernelSource/localPythonEnvKernelSourceSelector.node.ts index 1bcd086dea1..3c89de30fe5 100644 --- a/src/notebooks/controllers/kernelSource/localPythonEnvKernelSourceSelector.node.ts +++ b/src/notebooks/controllers/kernelSource/localPythonEnvKernelSourceSelector.node.ts @@ -209,10 +209,7 @@ export class LocalPythonEnvNotebookKernelSourceSelector ); } private async buildDummyEnvironment(e: Environment) { - const displayEmptyCondaEnv = - this.pythonApi.pythonExtensionVersion && - this.pythonApi.pythonExtensionVersion.compare('2023.3.10341119') >= 0; - const interpreter = pythonEnvToJupyterEnv(e, displayEmptyCondaEnv === true); + const interpreter = pythonEnvToJupyterEnv(e); if (!interpreter || this.filter.isPythonEnvironmentExcluded(interpreter)) { return; } diff --git a/src/platform/api/pythonApi.ts b/src/platform/api/pythonApi.ts index c492164db6a..70124aa8810 100644 --- a/src/platform/api/pythonApi.ts +++ b/src/platform/api/pythonApi.ts @@ -40,7 +40,12 @@ import { PythonExtensionActicationFailedError } from '../errors/pythonExtActivat import { PythonExtensionApiNotExportedError } from '../errors/pythonExtApiNotExportedError'; import { getOSType, OSType } from '../common/utils/platform'; import { SemVer } from 'semver'; -import { getCachedVersion, getEnvironmentType, setPythonApi } from '../interpreter/helpers'; +import { + getCachedVersion, + getEnvironmentType, + isCondaEnvironmentWithoutPython, + setPythonApi +} from '../interpreter/helpers'; import { getWorkspaceFolderIdentifier } from '../common/application/workspace.base'; export function deserializePythonEnvironment( @@ -96,12 +101,10 @@ export function resolvedPythonEnvToJupyterEnv( envType = EnvironmentType.VirtualEnv; } } - let isCondaEnvWithoutPython = false; let uri: Uri; let id = env.id; if (!env.executable.uri) { if (envType === EnvironmentType.Conda && supportsEmptyCondaEnv) { - isCondaEnvWithoutPython = true; uri = getOSType() === OSType.Windows ? Uri.joinPath(env.environment?.folderUri || Uri.file(env.path), 'python.exe') @@ -121,18 +124,15 @@ export function resolvedPythonEnvToJupyterEnv( envName: env.environment?.name || '', uri, displayName: env.environment?.name || '', - envType, - isCondaEnvWithoutPython + envType }; } -export function pythonEnvToJupyterEnv(env: Environment, supportsEmptyCondaEnv: boolean): PythonEnvironment | undefined { +export function pythonEnvToJupyterEnv(env: Environment): PythonEnvironment | undefined { const envType = getEnvironmentType(env); - let isCondaEnvWithoutPython = false; let uri: Uri; let id = env.id; if (!env.executable.uri) { - if (envType === EnvironmentType.Conda && supportsEmptyCondaEnv) { - isCondaEnvWithoutPython = true; + if (envType === EnvironmentType.Conda) { uri = getOSType() === OSType.Windows ? Uri.joinPath(env.environment?.folderUri || Uri.file(env.path), 'python.exe') @@ -152,8 +152,7 @@ export function pythonEnvToJupyterEnv(env: Environment, supportsEmptyCondaEnv: b envName: env.environment?.name || '', uri, displayName: env.environment?.name || '', - envType, - isCondaEnvWithoutPython + envType }; } @@ -873,7 +872,7 @@ export class InterpreterService implements IInterpreterService { // & subsequently updated as having python then trigger changes. const pythonInstalledIntoConda = e.type === 'update' && - this._interpreters.get(e.env.id)?.resolved.isCondaEnvWithoutPython && + isCondaEnvironmentWithoutPython(this._interpreters.get(e.env.id)?.resolved) && e.env.executable.uri ? true : false; @@ -888,7 +887,7 @@ export class InterpreterService implements IInterpreterService { e.type === 'update' && info && pythonInstalledIntoConda && - !info.resolved.isCondaEnvWithoutPython + !isCondaEnvironmentWithoutPython(info.resolved) ) { this.triggerEventIfAllowed('interpreterChangeEvent', info.resolved); this.triggerEventIfAllowed('interpretersChangeEvent', info.resolved); diff --git a/src/platform/interpreter/environmentActivationService.node.ts b/src/platform/interpreter/environmentActivationService.node.ts index 797c0935e09..c0e1345800b 100644 --- a/src/platform/interpreter/environmentActivationService.node.ts +++ b/src/platform/interpreter/environmentActivationService.node.ts @@ -174,7 +174,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi api .getActivatedEnvironmentVariables( resource, - serializePythonEnvironment(pythonEnvToJupyterEnv(environment, true))!, + serializePythonEnvironment(pythonEnvToJupyterEnv(environment))!, false ) .catch((ex) => { diff --git a/src/platform/interpreter/helpers.ts b/src/platform/interpreter/helpers.ts index 9d87fd3d836..a2d244e52de 100644 --- a/src/platform/interpreter/helpers.ts +++ b/src/platform/interpreter/helpers.ts @@ -33,8 +33,7 @@ export function getPythonEnvDisplayName(interpreter: PythonEnvironment | Environ } const pythonVersion = getTelemetrySafeVersion(getCachedVersion(interpreter) || '').trim(); // If this is a conda environment without Python, then don't display `Python` in it. - const isCondaEnvWithoutPython = - interpreter.envType === EnvironmentType.Conda && interpreter.isCondaEnvWithoutPython === true; + const isCondaEnvWithoutPython = isCondaEnvironmentWithoutPython(interpreter); const nameWithVersion = pythonVersion ? `Python ${pythonVersion}` : 'Python'; const envName = getPythonEnvironmentName(interpreter); if (isCondaEnvWithoutPython && envName) { @@ -100,10 +99,6 @@ export function getEnvironmentType(env: Environment): EnvironmentType { return EnvironmentType.Unknown; } -export function isCondaEnvironmentWithoutPython(env: Environment) { - return getEnvironmentType(env) === EnvironmentType.Conda && !env.executable.uri; -} - export async function getInterpreterInfo(interpreter?: { id: string }) { if (!interpreter?.id) { return; @@ -117,7 +112,19 @@ export function setPythonApi(api: PythonExtension) { pythonApi = api; } -export function getCachedInterpreterInfo(interpreter?: { id: string }) { +export function isCondaEnvironmentWithoutPython(interpreter?: { id: string }) { + if (!interpreter) { + return false; + } + if (!pythonApi) { + return false; + } + + const env = getCachedInterpreterInfo(interpreter); + return env && getEnvironmentType(env) === EnvironmentType.Conda && !env.executable.uri; +} + +function getCachedInterpreterInfo(interpreter?: { id: string }) { if (!interpreter) { return; } diff --git a/src/platform/interpreter/installer/condaInstaller.node.ts b/src/platform/interpreter/installer/condaInstaller.node.ts index 1f9175b60f0..04107122c6f 100644 --- a/src/platform/interpreter/installer/condaInstaller.node.ts +++ b/src/platform/interpreter/installer/condaInstaller.node.ts @@ -81,21 +81,14 @@ export class CondaInstaller extends ModuleInstaller { // If we just installed a package into a conda env without python init, then Python may have gotten installed // We now need to ensure the conda env gets updated as a result of this. - if ( - ('executable' in interpreter - ? getEnvironmentType(interpreter) - : interpreter.envType === EnvironmentType.Conda) && - ('executable' in interpreter - ? isCondaEnvironmentWithoutPython(interpreter) - : interpreter.isCondaEnvWithoutPython) - ) { + if (isCondaEnvironmentWithoutPython(interpreter)) { const pythonExt = this.serviceContainer.get(IPythonExtensionChecker); if (!pythonExt.isPythonExtensionActive) { return; } const interpreterService = this.serviceContainer.get(IInterpreterService); const updatedCondaEnv = await interpreterService.getInterpreterDetails(interpreter.id); - if (updatedCondaEnv && !updatedCondaEnv.isCondaEnvWithoutPython) { + if (updatedCondaEnv && !isCondaEnvironmentWithoutPython(updatedCondaEnv)) { Object.assign(interpreter, updatedCondaEnv); } } diff --git a/src/platform/interpreter/installer/productInstaller.node.ts b/src/platform/interpreter/installer/productInstaller.node.ts index 777093c6e56..a3a3ff901e3 100644 --- a/src/platform/interpreter/installer/productInstaller.node.ts +++ b/src/platform/interpreter/installer/productInstaller.node.ts @@ -110,9 +110,6 @@ export class DataScienceInstaller { if (cancelTokenSource.token.isCancellationRequested) { return InstallerResponse.Cancelled; } - if (interpreter.isCondaEnvWithoutPython) { - interpreter.isCondaEnvWithoutPython = false; - } return this.isInstalled(product, interpreter).then((isInstalled) => { return isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore; }); diff --git a/src/platform/interpreter/interpreterPackages.node.ts b/src/platform/interpreter/interpreterPackages.node.ts index 7f7928e36db..a90008197a9 100644 --- a/src/platform/interpreter/interpreterPackages.node.ts +++ b/src/platform/interpreter/interpreterPackages.node.ts @@ -15,6 +15,7 @@ import { getDisplayPath } from '../common/platform/fs-paths.node'; import { IInterpreterPackages } from './types'; import { IPythonExecutionFactory } from './types.node'; import { getWorkspaceFolderIdentifier } from '../common/application/workspace.base'; +import { isCondaEnvironmentWithoutPython } from './helpers'; const interestedPackages = new Set( [ @@ -178,7 +179,7 @@ export class InterpreterPackages implements IInterpreterPackages { } private async getPackageInformation({ interpreter }: { interpreter: PythonEnvironment }) { - if (interpreter.isCondaEnvWithoutPython) { + if (isCondaEnvironmentWithoutPython(interpreter)) { return; } const service = await this.executionFactory.createActivatedEnvironment({ diff --git a/src/platform/pythonEnvironments/info/index.ts b/src/platform/pythonEnvironments/info/index.ts index 25216e5cc44..978fa767da1 100644 --- a/src/platform/pythonEnvironments/info/index.ts +++ b/src/platform/pythonEnvironments/info/index.ts @@ -35,7 +35,7 @@ export type InterpreterInformation = { * Details about a Python environment. * @prop envType - the kind of Python environment */ -export type PythonEnvironment = InterpreterInformation & { +export interface PythonEnvironment extends InterpreterInformation { displayName?: string; envType?: EnvironmentType; envName?: string; @@ -48,5 +48,4 @@ export type PythonEnvironment = InterpreterInformation & { * Used for display purposes only (in kernel picker or other places). */ displayPath?: Uri; - isCondaEnvWithoutPython?: boolean; }; From 0771f80ac65fc7c2d1aab48f329b45f9f7986607 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 27 Feb 2024 16:36:19 +1100 Subject: [PATCH 2/2] Fix tests --- src/kernels/helpers.unit.test.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index 064b7db6b66..cab68ea54f8 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -342,7 +342,8 @@ suite('Kernel Connection Helpers', () => { micro: 7, release: undefined, sysVersion: '9.8.7.6-pre' - } + }, + tools: [] } as any ]); const name = getDisplayNameOrNameOfKernelConnection( @@ -376,7 +377,8 @@ suite('Kernel Connection Helpers', () => { micro: 7, release: undefined, sysVersion: '9.8.7' - } + }, + tools: [] } as any ]); const name = getDisplayNameOrNameOfKernelConnection( @@ -455,7 +457,8 @@ suite('Kernel Connection Helpers', () => { micro: 3, release: undefined, sysVersion: '1.2.3' - } + }, + tools: [] } as any ]); const name = getDisplayNameOrNameOfKernelConnection( @@ -540,7 +543,8 @@ suite('Kernel Connection Helpers', () => { micro: 7, release: undefined, sysVersion: '9.8.7.6-pre' - } + }, + tools: [] } as any ]); @@ -570,7 +574,8 @@ suite('Kernel Connection Helpers', () => { micro: 7, release: undefined, sysVersion: '9.8.7.6-pre' - } + }, + tools: [] } as any ]); @@ -600,7 +605,8 @@ suite('Kernel Connection Helpers', () => { micro: 7, release: undefined, sysVersion: '9.8.7.6-pre' - } + }, + tools: [] } as any ]);