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.

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

fix global python interpreter filtering in Project Wizard #5502

Merged
merged 4 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions extensions/positron-python/src/client/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export namespace Commands {
export const Get_Create_Environment_Providers = 'python.getCreateEnvironmentProviders';
export const Is_Conda_Installed = 'python.isCondaInstalled';
export const Get_Conda_Python_Versions = 'python.getCondaPythonVersions';
export const Is_Global_Python = 'python.isGlobalPython';
// --- End Positron ---
export const InstallJupyter = 'python.installJupyter';
export const InstallPython = 'python.installPython';
Expand Down
21 changes: 21 additions & 0 deletions extensions/positron-python/src/client/positron/createEnvApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import {
} from '../pythonEnvironments/creation/proposed.createEnvApis';
import { handleCreateEnvironmentCommand } from '../pythonEnvironments/creation/createEnvironment';
import { IPythonRuntimeManager } from './manager';
import { getExtension } from '../common/vscodeApis/extensionsApi';
import { PythonExtension } from '../api/types';
import { PVSC_EXTENSION_ID } from '../common/constants';

/**
* A simplified version of an environment provider that can be used in the Positron Project Wizard
Expand Down Expand Up @@ -70,3 +73,21 @@ export async function createEnvironmentAndRegister(
}
return result;
}

/**
* Checks if the given interpreter is a global python installation.
* @param interpreterPath The interpreter path to check.
* @returns True if the interpreter is a global python installation, false if it is not, and
* undefined if the check could not be performed.
* Implementation is based on isGlobalPythonSelected in extensions/positron-python/src/client/pythonEnvironments/creation/common/createEnvTriggerUtils.ts
*/
export async function isGlobalPython(interpreterPath: string): Promise<boolean | undefined> {
const extension = getExtension<PythonExtension>(PVSC_EXTENSION_ID);
if (!extension) {
return undefined;
}
const extensionApi: PythonExtension = extension.exports as PythonExtension;
const interpreterDetails = await extensionApi.environments.resolveEnvironment(interpreterPath);
const isGlobal = interpreterDetails?.environment === undefined;
return isGlobal;
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ import { CreateEnvironmentOptionsInternal } from './types';
import { getCondaPythonVersions } from './provider/condaUtils';
import { IPythonRuntimeManager } from '../../positron/manager';
import { Conda } from '../common/environmentManagers/conda';
import { createEnvironmentAndRegister, getCreateEnvironmentProviders } from '../../positron/createEnvApi';
import {
createEnvironmentAndRegister,
getCreateEnvironmentProviders,
isGlobalPython,
} from '../../positron/createEnvApi';
// --- End Positron ---

class CreateEnvironmentProviders {
Expand Down Expand Up @@ -109,6 +113,7 @@ export function registerCreateEnvironmentFeatures(
},
),
registerCommand(Commands.Get_Conda_Python_Versions, () => getCondaPythonVersions()),
registerCommand(Commands.Is_Global_Python, (interpreterPath: string) => isGlobalPython(interpreterPath)),
// --- End Positron ---
registerCreateEnvironmentProvider(new VenvCreationProvider(interpreterQuickPick)),
registerCreateEnvironmentProvider(condaCreationProvider()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,3 @@ export enum PythonEnvironmentProvider {
Venv = 'Venv',
Conda = 'Conda'
}

/**
* PythonRuntimeFilter enum.
*/
export enum PythonRuntimeFilter {
Global = 'Global',
Pyenv = 'Pyenv'
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs';
import { ILanguageRuntimeMetadata, ILanguageRuntimeService } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService';
import { IRuntimeSessionService } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
import { IRuntimeStartupService, RuntimeStartupPhase } from 'vs/workbench/services/runtimeStartup/common/runtimeStartupService';
import { EnvironmentSetupType, NewProjectWizardStep, PythonEnvironmentProvider, PythonRuntimeFilter } from 'vs/workbench/browser/positronNewProjectWizard/interfaces/newProjectWizardEnums';
import { EnvironmentSetupType, NewProjectWizardStep, PythonEnvironmentProvider } from 'vs/workbench/browser/positronNewProjectWizard/interfaces/newProjectWizardEnums';
import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { ILogService } from 'vs/platform/log/common/log';
Expand Down Expand Up @@ -86,12 +86,6 @@ export interface INewProjectWizardStateManager {
readonly onUpdateProjectDirectory: Event<void>;
}

/**
* RuntimeFilter type.
* More filters can be added here as needed.
*/
type RuntimeFilter = PythonRuntimeFilter;

/**
* NewProjectWizardStateManager class.
* This class is used to manage the state of the New Project Wizard.
Expand Down Expand Up @@ -174,8 +168,7 @@ export class NewProjectWizardStateManager
.then(() => {
this._runtimeStartupComplete = true;
this._updateInterpreterRelatedState();
}
);
});
} else {
// Register disposables.
this._register(
Expand Down Expand Up @@ -607,30 +600,16 @@ export class NewProjectWizardStateManager
return;
}

const langId = this._getLangId();
let runtimeSourceFilters: RuntimeFilter[] | undefined = undefined;

// Add runtime filters for new Venv Python environments.
if (langId === LanguageIds.Python && this._pythonEnvSetupType === EnvironmentSetupType.NewEnvironment) {
if (this._getEnvProviderName() === PythonEnvironmentProvider.Venv) {
// TODO: instead of applying our own filters, we should use the filters provided by the
// extension for Global python environments.
// extensions/positron-python/src/client/pythonEnvironments/creation/common/createEnvTriggerUtils.ts
// isGlobalPython
runtimeSourceFilters = [PythonRuntimeFilter.Global, PythonRuntimeFilter.Pyenv];
}
}

// Update the interpreters list.
this._interpreters = this._getFilteredInterpreters(runtimeSourceFilters);
this._interpreters = await this._getFilteredInterpreters();

// Update the interpreter that should be selected in the dropdown.
if (!this._selectedRuntime || !this._interpreters?.includes(this._selectedRuntime)) {
this._resetSelectedRuntime();
}

// For Python projects, check if ipykernel needs to be installed.
if (langId === LanguageIds.Python) {
if (this._getLangId() === LanguageIds.Python) {
this._installIpykernel = await this._getInstallIpykernel();
}

Expand Down Expand Up @@ -753,7 +732,7 @@ export class NewProjectWizardStateManager
this._condaPythonVersionInfo = EMPTY_CONDA_PYTHON_VERSION_INFO;

if (!this._pythonEnvProviders?.length) {
this._services.logService.error('No Python environment providers found.');
this._services.logService.error('[Project Wizard] No Python environment providers found.');
return;
}

Expand All @@ -762,7 +741,7 @@ export class NewProjectWizardStateManager
(provider) => provider.name === PythonEnvironmentProvider.Conda
);
if (!providersIncludeConda) {
this._services.logService.info('Conda is not available as an environment provider.');
this._services.logService.info('[Project Wizard] Conda is not available as an environment provider.');
return;
}

Expand All @@ -772,7 +751,7 @@ export class NewProjectWizardStateManager
);
if (!this._isCondaInstalled) {
this._services.logService.warn(
'Conda is available as an environment provider, but it is not installed.'
'[Project Wizard] Conda is available as an environment provider, but it is not installed.'
);
return;
}
Expand All @@ -781,7 +760,7 @@ export class NewProjectWizardStateManager
const pythonVersionInfo: CondaPythonVersionInfo | undefined =
await this._services.commandService.executeCommand('python.getCondaPythonVersions');
if (!pythonVersionInfo) {
this._services.logService.warn('No Conda Python versions found.');
this._services.logService.warn('[Project Wizard] No Conda Python versions found.');
return;
}

Expand Down Expand Up @@ -821,15 +800,14 @@ export class NewProjectWizardStateManager
}

/**
* Retrieves the interpreters that match the language ID and runtime source filters. Sorts the
* interpreters by runtime source.
* @param runtimeSourceFilters Optional runtime source filters to apply.
* @returns The filtered interpreters.
* Retrieves the interpreters that match the current language ID and environment setup type if
* applicable.
* @returns The filtered interpreters sorted by runtime source, or undefined if runtime startup is
* not complete or a Conda environment is being used.
*/
private _getFilteredInterpreters(runtimeSourceFilters?: RuntimeFilter[]): ILanguageRuntimeMetadata[] | undefined {
const langId = this._getLangId();

private async _getFilteredInterpreters(): Promise<ILanguageRuntimeMetadata[] | undefined> {
if (this._usesCondaEnv()) {
this._services.logService.trace(`[Project Wizard] Conda environments do not have registered runtimes`);
// Conda environments do not have registered runtimes. Instead, we have a list of Python
// versions available for Conda environments, which is stored in condaPythonVersionInfo.
return undefined;
Expand All @@ -838,41 +816,52 @@ export class NewProjectWizardStateManager
// We don't want to return a partial list of interpreters if the runtime startup is not
// complete, so we return undefined in that case.
if (!this._runtimeStartupComplete) {
this._services.logService.warn('[Project Wizard] Requested filtered interpreters before runtime startup is complete. Please come by later!');
return undefined;
}

// Once the runtime startup is complete, we can return the filtered list of interpreters.
return this._services.languageRuntimeService.registeredRuntimes
// Filter by language ID and runtime source.
.filter(
(runtime) =>
runtime.languageId === langId &&
this._includeRuntimeSource(runtime.runtimeSource, runtimeSourceFilters)
)
// Sort by runtime source.
const langId = this._getLangId();
let runtimesForLang = this._services.languageRuntimeService.registeredRuntimes
.filter(runtime => runtime.languageId === langId);

// If we're creating a new Python environment, only return Global runtimes.
if (langId === LanguageIds.Python
&& this._pythonEnvSetupType === EnvironmentSetupType.NewEnvironment
) {
const globalRuntimes = [];
for (const runtime of runtimesForLang) {
const interpreterPath = runtime.extraRuntimeData.pythonPath as string ?? runtime.runtimePath;
const isGlobal = await this.services.commandService.executeCommand(
'python.isGlobalPython',
interpreterPath
) satisfies boolean | undefined;
if (isGlobal === undefined) {
this._services.logService.error(
`[Project Wizard] Unable to determine if Python interpreter '${interpreterPath}' is global`
);
continue;
}
if (isGlobal) {
globalRuntimes.push(runtime);
} else {
this._services.logService.trace(`[Project Wizard] Skipping non-global Python interpreter '${interpreterPath}'`);
}
}
// If the global runtimes list is a different length than the original runtimes list,
// then we only want to show the global runtimes.
if (runtimesForLang.length !== globalRuntimes.length) {
runtimesForLang = globalRuntimes;
}
}

// Return the runtimes, sorted by runtime source.
return runtimesForLang
.sort((left, right) =>
left.runtimeSource.localeCompare(right.runtimeSource)
);
}

/**
* Determines if the runtime source should be included based on the filters.
* @param runtimeSource The runtime source to check.
* @param filters The runtime source filters to apply.
* @returns True if the runtime source should be included, false otherwise.
* If no filters are provided, all runtime sources are included.
*/
private _includeRuntimeSource(
runtimeSource: string,
filters?: RuntimeFilter[]
) {
return (
!filters ||
!filters.length ||
filters.find((rs) => rs === runtimeSource) !== undefined
);
}

/**
* Resets the properties of the project configuration that should not be persisted when the
* project type changes.
Expand Down
Loading