Add middleware that enhances the `ty` configuration section with the active Python interpreter by MichaReiser · Pull Request #65 · astral-sh/ty-vscode

MichaReiser · 2025-06-27T17:14:48Z

Summary

One step closer towards #26

This PR adds a custom middleware that intercepts the configuration request and it adds the information for the selected and active python interpreter. The value returned by the Python extension doesn't necessarily have to point to a valid python environment. But we can use it as a fallback if it does.

Test Plan

Screen.Recording.2025-07-10.at.14.23.50.mov

MichaReiser · 2025-06-27T17:29:32Z

src/common/server.ts

+      if (middleware.isDidChangeConfigurationRegistered()) {
+        logger.debug("Python interpreter changed, sending DidChangeConfigurationNotification");
+
+        // If the Python interpreter changes,
+        newLSClient.sendNotification(DidChangeConfigurationNotification.type, undefined);
+      }


This is a bit overkill at the moment because we always restart the server when the interpreter changes (because we might need to use a different binary now). I kept the implementation because it took me a rather long time to figure out how to know if the server registered the capability but I can also remove it again.

I think it would also be great if we can avoid restarting the LSP if the interpreter changes....

Yeah, agreed. I think the only reason that we would use a different binary is if the ty binary used is installed in the virtual environment where the interpreter is present.

I don't know if we can avoid the restart completely but we could check if the interpreter is even going to be used to find the binary path. If it's not going to be used, then we can avoid restarting the server when the interpreter changes.

The checks that I'm referring to is the one before line 101 in the below code:

ty-vscode/src/common/server.ts

Lines 79 to 136 in 6e934d0

async function findBinaryPath(settings: ISettings): Promise<string> {

if (!vscode.workspace.isTrusted) {

logger.info(`Workspace is not trusted, using bundled executable: ${BUNDLED_EXECUTABLE}`);

return BUNDLED_EXECUTABLE;

}

// 'path' setting takes priority over everything.

if (settings.path.length > 0) {

for (const path of settings.path) {

if (await fsapi.pathExists(path)) {

logger.info(`Using 'path' setting: ${path}`);

return path;

}

}

logger.info(`Could not find executable in 'path': ${settings.path.join(", ")}`);

}

if (settings.importStrategy === "useBundled") {

logger.info(`Using bundled executable: ${BUNDLED_EXECUTABLE}`);

return BUNDLED_EXECUTABLE;

}

// Otherwise, we'll call a Python script that tries to locate a binary.

let tyBinaryPath: string | undefined;

try {

const stdout = await executeFile(settings.interpreter[0], [FIND_BINARY_SCRIPT_PATH]);

tyBinaryPath = stdout.trim();

} catch (err) {

vscode.window

.showErrorMessage(

"Unexpected error while trying to find the ty binary. See the logs for more details.",

"Show Logs",

)

.then((selection) => {

if (selection) {

logger.channel.show();

}

});

logger.error(`Error while trying to find the ty binary: ${err}`);

}

if (tyBinaryPath && tyBinaryPath.length > 0) {

// First choice: the executable found by the script.

logger.info(`Using the ty binary: ${tyBinaryPath}`);

return tyBinaryPath;

}

// Second choice: the executable in the global environment.

const environmentPath = await which(BINARY_NAME, { nothrow: true });

if (environmentPath) {

logger.info(`Using environment executable: ${environmentPath}`);

return environmentPath;

}

// Third choice: bundled executable.

logger.info(`Falling back to bundled executable: ${BUNDLED_EXECUTABLE}`);

return BUNDLED_EXECUTABLE;

}

So, if we know how we're going to choose the ty binary, then we can use that information to avoid restarting the server when it's not required.

MichaReiser · 2025-06-27T17:29:48Z

src/common/logger.ts

@@ -1,4 +1,4 @@
-import * as util from "util";
+import * as util from "node:util";


I fixed a few issues that biome found along the way

MichaReiser · 2025-06-27T17:56:50Z

src/client.ts

+          if (param.section === "ty") {
+            const scopeUri = param.scopeUri ? Uri.parse(param.scopeUri) : undefined;
+            const activeEnvironment =
+              pythonExtension.environments.getActiveEnvironmentPath(scopeUri);


I'm not sure yet if we should call resolveEnvironment here. The benefit is that we only get valid environments. The downside is that ty has to repeat this work and may still decide that this isn't an active environment (for some reason or another)

src/client.ts

src/common/server.ts

dhruvmanila · 2025-07-03T10:22:38Z

src/common/python.ts

-async function getPythonExtensionAPI(): Promise<PythonExtension | undefined> {
-  if (_api) {
-    return _api;
-  }
-  _api = await PythonExtension.api();
-  return _api;
+export async function getPythonExtensionAPI(): Promise<PythonExtension> {
+  const api = _api || (await PythonExtension.api());
+  _api = api;
+  return api;


Unrelated to your PR, we might have to make this optional again after #5

Yes, we need to make this optional as part of #5

dhruvmanila · 2025-07-03T10:26:29Z

src/common/server.ts

+      if (middleware.isDidChangeConfigurationRegistered()) {
+        logger.debug("Python interpreter changed, sending DidChangeConfigurationNotification");
+
+        // If the Python interpreter changes,
+        newLSClient.sendNotification(DidChangeConfigurationNotification.type, undefined);
+      }


Yeah, agreed. I think the only reason that we would use a different binary is if the ty binary used is installed in the virtual environment where the interpreter is present.

I don't know if we can avoid the restart completely but we could check if the interpreter is even going to be used to find the binary path. If it's not going to be used, then we can avoid restarting the server when the interpreter changes.

The checks that I'm referring to is the one before line 101 in the below code:

ty-vscode/src/common/server.ts

Lines 79 to 136 in 6e934d0

async function findBinaryPath(settings: ISettings): Promise<string> {

if (!vscode.workspace.isTrusted) {

logger.info(`Workspace is not trusted, using bundled executable: ${BUNDLED_EXECUTABLE}`);

return BUNDLED_EXECUTABLE;

}

// 'path' setting takes priority over everything.

if (settings.path.length > 0) {

for (const path of settings.path) {

if (await fsapi.pathExists(path)) {

logger.info(`Using 'path' setting: ${path}`);

return path;

}

}

logger.info(`Could not find executable in 'path': ${settings.path.join(", ")}`);

}

if (settings.importStrategy === "useBundled") {

logger.info(`Using bundled executable: ${BUNDLED_EXECUTABLE}`);

return BUNDLED_EXECUTABLE;

}

// Otherwise, we'll call a Python script that tries to locate a binary.

let tyBinaryPath: string | undefined;

try {

const stdout = await executeFile(settings.interpreter[0], [FIND_BINARY_SCRIPT_PATH]);

tyBinaryPath = stdout.trim();

} catch (err) {

vscode.window

.showErrorMessage(

"Unexpected error while trying to find the ty binary. See the logs for more details.",

"Show Logs",

)

.then((selection) => {

if (selection) {

logger.channel.show();

}

});

logger.error(`Error while trying to find the ty binary: ${err}`);

}

if (tyBinaryPath && tyBinaryPath.length > 0) {

// First choice: the executable found by the script.

logger.info(`Using the ty binary: ${tyBinaryPath}`);

return tyBinaryPath;

}

// Second choice: the executable in the global environment.

const environmentPath = await which(BINARY_NAME, { nothrow: true });

if (environmentPath) {

logger.info(`Using environment executable: ${environmentPath}`);

return environmentPath;

}

// Third choice: bundled executable.

logger.info(`Falling back to bundled executable: ${BUNDLED_EXECUTABLE}`);

return BUNDLED_EXECUTABLE;

}

So, if we know how we're going to choose the ty binary, then we can use that information to avoid restarting the server when it's not required.

dhruvmanila · 2025-07-03T10:27:35Z

I'm assuming that this should be merged along with astral-sh/ruff#19012, right?

dhruvmanila · 2025-07-07T14:13:56Z

So, all in all, to fully support this, we require implementing the following in the server:

Registering for the didChangeConfiguration notification
[ty] Use python version and path from Python extension ruff#19012

I added a dbg statement in the ty server's configuration response handler and the pythonExtension.activeEnvironment setting was set correctly.

I might be missing something but how does this work then without registering the didChangeConfiguration notification?

dhruvmanila

Looks good. I think we need to validate and probably change the way registration is being handled here. We probably don't need to look at the registration options for the didChangeConfiguration notification (as it doesn't exists) and directly add the id to didChangeRegistrations. I think the TyMiddleware is only for the ty server so that should be accurate and the server won't be providing any registration options either.

src/client.ts

…active Python interpreter

MichaReiser force-pushed the micha/python-interpreter branch 2 times, most recently from 7e1ed8c to 6e934d0 Compare June 27, 2025 17:26

MichaReiser commented Jun 27, 2025

View reviewed changes

MichaReiser marked this pull request as ready for review June 27, 2025 17:30

MichaReiser added the internal label Jun 27, 2025

MichaReiser commented Jun 27, 2025

View reviewed changes

dhruvmanila self-requested a review June 30, 2025 06:53

dhruvmanila self-assigned this Jun 30, 2025

dhruvmanila reviewed Jul 3, 2025

View reviewed changes

dhruvmanila removed their assignment Jul 7, 2025

dhruvmanila mentioned this pull request Jul 7, 2025

LSP client settings astral-sh/ty#82

Closed

MichaReiser force-pushed the micha/python-interpreter branch from b7d731f to 6a606ee Compare July 9, 2025 18:05

MichaReiser mentioned this pull request Jul 10, 2025

[ty] Use python version and path from Python extension astral-sh/ruff#19012

Merged

MichaReiser requested a review from dhruvmanila July 11, 2025 14:18

dhruvmanila approved these changes Jul 14, 2025

View reviewed changes

src/client.ts Show resolved Hide resolved

MichaReiser added 3 commits July 14, 2025 11:50

Add middleware that enhances the ty configuration section with the …

51e1c1e

…active Python interpreter

Resolve properties before passing to JSON

c973a45

Simplify notification handler tracking

0995df8

MichaReiser force-pushed the micha/python-interpreter branch from 6a606ee to 0995df8 Compare July 14, 2025 09:52

Comment

7ed963e

MichaReiser merged commit 78c1803 into main Jul 14, 2025
1 check passed

MichaReiser deleted the micha/python-interpreter branch July 14, 2025 10:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Add middleware that enhances the `ty` configuration section with the active Python interpreter#65

Add middleware that enhances the `ty` configuration section with the active Python interpreter#65
MichaReiser merged 4 commits intomainfrom
micha/python-interpreter

MichaReiser commented Jun 27, 2025 •

edited

Loading

Uh oh!

MichaReiser Jun 27, 2025

Uh oh!

dhruvmanila Jul 3, 2025 •

edited

Loading

Uh oh!

MichaReiser Jun 27, 2025

Uh oh!

MichaReiser Jun 27, 2025

Uh oh!

Uh oh!

Uh oh!

dhruvmanila Jul 3, 2025

Uh oh!

MichaReiser Jul 7, 2025

Uh oh!

dhruvmanila Jul 3, 2025 •

edited

Loading

Uh oh!

dhruvmanila commented Jul 3, 2025

Uh oh!

dhruvmanila commented Jul 7, 2025

Uh oh!

dhruvmanila left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	async function findBinaryPath(settings: ISettings): Promise<string> {
	if (!vscode.workspace.isTrusted) {
	logger.info(`Workspace is not trusted, using bundled executable: ${BUNDLED_EXECUTABLE}`);
	return BUNDLED_EXECUTABLE;
	}

	// 'path' setting takes priority over everything.
	if (settings.path.length > 0) {
	for (const path of settings.path) {
	if (await fsapi.pathExists(path)) {
	logger.info(`Using 'path' setting: ${path}`);
	return path;
	}
	}
	logger.info(`Could not find executable in 'path': ${settings.path.join(", ")}`);
	}

	if (settings.importStrategy === "useBundled") {
	logger.info(`Using bundled executable: ${BUNDLED_EXECUTABLE}`);
	return BUNDLED_EXECUTABLE;
	}

	// Otherwise, we'll call a Python script that tries to locate a binary.
	let tyBinaryPath: string \| undefined;
	try {
	const stdout = await executeFile(settings.interpreter[0], [FIND_BINARY_SCRIPT_PATH]);
	tyBinaryPath = stdout.trim();
	} catch (err) {
	vscode.window
	.showErrorMessage(
	"Unexpected error while trying to find the ty binary. See the logs for more details.",
	"Show Logs",
	)
	.then((selection) => {
	if (selection) {
	logger.channel.show();
	}
	});
	logger.error(`Error while trying to find the ty binary: ${err}`);
	}

	if (tyBinaryPath && tyBinaryPath.length > 0) {
	// First choice: the executable found by the script.
	logger.info(`Using the ty binary: ${tyBinaryPath}`);
	return tyBinaryPath;
	}

	// Second choice: the executable in the global environment.
	const environmentPath = await which(BINARY_NAME, { nothrow: true });
	if (environmentPath) {
	logger.info(`Using environment executable: ${environmentPath}`);
	return environmentPath;
	}

	// Third choice: bundled executable.
	logger.info(`Falling back to bundled executable: ${BUNDLED_EXECUTABLE}`);
	return BUNDLED_EXECUTABLE;
	}

		@@ -1,4 +1,4 @@
		import * as util from "util";
		import * as util from "node:util";

Comments

Conversation

MichaReiser commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

MichaReiser Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dhruvmanila Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented Jul 3, 2025

Uh oh!

dhruvmanila commented Jul 7, 2025

Uh oh!

dhruvmanila left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaReiser commented Jun 27, 2025 •

edited

Loading

dhruvmanila Jul 3, 2025 •

edited

Loading

dhruvmanila Jul 3, 2025 •

edited

Loading