Skip to content

Add new command to kick off (refresh) interpreter discovery #7762

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

Merged
merged 11 commits into from
May 27, 2025
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export async function* pythonRuntimeDiscoverer(
traceInfo(`pythonRuntimeDiscoverer: recommended interpreter: ${recommendedInterpreter?.path}`);

// Discover Python interpreters
await interpreterService.triggerRefresh().ignoreErrors();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what we needed for this to work for Python! 🎉

Screenshot 2025-05-27 at 2 54 16 PM

However, it does change what the Python discoverer does for all uses. Do we think this is a good way to go? The alternative is that discoverAllRuntimes() on the runtime startup service could gain an optional boolean argument that specifies whether this is a refresh or an original discovery; we would then pipe it through the API command through to here:

return this.discoverPythonRuntimes();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refreshing every time could slow down discovery if users open up multiple windows, since it runs on extension activation. However, finding interpreters each new window might be a nice experience (and actually make it less necessary to use a refresh command). I think its fine to do every time.

let interpreters = interpreterService.getInterpreters();

traceInfo(`pythonRuntimeDiscoverer: discovered ${interpreters.length} Python interpreters`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export const LANGUAGE_RUNTIME_RESTART_ACTIVE_SESSION_ID = 'workbench.action.lang
export const LANGUAGE_RUNTIME_RENAME_SESSION_ID = 'workbench.action.language.runtime.renameSession';
export const LANGUAGE_RUNTIME_RENAME_ACTIVE_SESSION_ID = 'workbench.action.language.runtime.renameActiveSession';
export const LANGUAGE_RUNTIME_DUPLICATE_ACTIVE_SESSION_ID = 'workbench.action.language.runtime.duplicateActiveSession';

export const LANGUAGE_RUNTIME_SELECT_RUNTIME_ID = 'workbench.action.languageRuntime.selectRuntime';
export const LANGUAGE_RUNTIME_DISCOVER_RUNTIMES_ID = 'workbench.action.language.runtime.discoverAllRuntimes';

/**
* Helper function that askses the user to select a language from the list of registered language
Expand Down Expand Up @@ -967,6 +967,29 @@ export function registerLanguageRuntimeActions() {
}
});

registerAction2(class extends Action2 {
/**
* Constructor.
*/
constructor() {
super({
id: LANGUAGE_RUNTIME_DISCOVER_RUNTIMES_ID,
title: nls.localize2('workbench.action.language.runtime.discoverAllRuntimes', "Discover All Interpreters"),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use wording here that is more like "Refresh Interpreter Discovery", if we think that is better or more clear?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the wording in the PR!

f1: true,
category
});
}

async run(accessor: ServicesAccessor) {
// Access service.
const runtimeStartupService = accessor.get(IRuntimeStartupService);

// Kick off discovery.
runtimeStartupService.rediscoverAllRuntimes();
}
});


/**
* Arguments passed to the Execute Code actions.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ class TestRuntimeStartupService implements IRuntimeStartupService {
// No-op in test implementation
}

public async rediscoverAllRuntimes() {
// No-op in test implementation
}

registerRuntimeManager(_manager: IRuntimeManager): IDisposable {
return Disposable.None;
}
Expand Down
50 changes: 48 additions & 2 deletions src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,52 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup
};
}

/**
* Kicks off a refresh of runtime discovery, after initial discovery.
*/
public async rediscoverAllRuntimes(): Promise<void> {

// If we haven't completed discovery once already, don't do anything.
if (this._startupPhase !== RuntimeStartupPhase.Complete) {
this._logService.warn('[Runtime startup] Runtime discovery refresh called before initial discovery is complete.');
return;
}

// Set up event to notify when runtimes are added.
const oldRuntimes = this._languageRuntimeService.registeredRuntimes;
this._register(
this._languageRuntimeService.onDidChangeRuntimeStartupPhase(
(phase) => {
if (phase === RuntimeStartupPhase.Complete) {
const newRuntimes = this._languageRuntimeService.registeredRuntimes;
const addedRuntimes = newRuntimes.filter(newRuntime => {
return !oldRuntimes.some(oldRuntime => {
return oldRuntime.runtimeId === newRuntime.runtimeId;
});
});

// If any runtimes were added, show a notification.
if (addedRuntimes.length > 0) {
this._notificationService.info(nls.localize('positron.runtimeStartupService.runtimesAddedMessage',
"Found {0} new interpreter{1}: {2}.",
addedRuntimes.length,
addedRuntimes.length > 1 ? 's' : '',
addedRuntimes.map(runtime => { return runtime.runtimeName; }).join(', ')));
}
}
}
)
);

this._logService.debug('[Runtime startup] Refreshing runtime discovery.');
this._discoveryCompleteByExtHostId.forEach((_, extHostId, m) => {
m.set(extHostId, false);
});

this.discoverAllRuntimes();

}

/**
* Runs as an event handler when the active runtime changes.
*
Expand Down Expand Up @@ -693,7 +739,7 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup
}

/**
* Activates all of the extensions that provides language runtimes, then
* Activates all of the extensions that provide language runtimes, then
* enters the discovery phase, in which each extension is asked to supply
* its language runtime metadata.
*/
Expand Down Expand Up @@ -739,7 +785,7 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup
// language runtime providers.
this.setStartupPhase(RuntimeStartupPhase.Discovering);

// Ask each extension to provide its language runtime metadata.
// Ask each extension host to provide its language runtime metadata.
for (const manager of this._runtimeManagers) {
manager.discoverAllRuntimes(disabledLanguages);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ export interface IRuntimeStartupService {
*/
completeDiscovery(id: number): void;

/**
* Kick off a user-driven refresh of runtime discovery, after the initial discovery.
*/
rediscoverAllRuntimes(): Promise<void>;

/**
* Get the sessions that were (or will be) restored into this window.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ export class TestRuntimeStartupService implements IRuntimeStartupService {
// No-op in test implementation
}

/**
* {@inheritDoc}
*/
public async rediscoverAllRuntimes() {
// No-op in test implementation
}

/**
* {@inheritDoc}
*/
Expand Down
Loading