Remove old code for folder support in interpreter path settings

Don Jayamanne (@DonJayamanne)

This check is done to ensure if entered interpreter path is valid, and if so, the path is returned. I don't see the point of these checks, as we shouldn't support entering interpreter paths this way.

Variable 'pythonPath' refers to the path user entered

vscode-python/src/client/common/configSettings.ts

Lines 674 to 696 in 620b614

    
           const KnownPythonExecutables = ['python', 'python4', 'python3.6', 'python3.5', 'python3', 'python2.7', 'python2']; 
        
           for (let executableName of KnownPythonExecutables) { 
        
               // Suffix with 'python' for linux and 'osx', and 'python.exe' for 'windows'. 
        
               if (IS_WINDOWS) { 
        
                   executableName = `${executableName}.exe`; 
        
                   if (isValidPythonPath(path.join(pythonPath, executableName))) { 
        
                       return path.join(pythonPath, executableName); 
        
                   } 
        
                   if (isValidPythonPath(path.join(pythonPath, 'scripts', executableName))) { 
        
                       return path.join(pythonPath, 'scripts', executableName); 
        
                   } 
        
               } else { 
        
                   if (isValidPythonPath(path.join(pythonPath, executableName))) { 
        
                       return path.join(pythonPath, executableName); 
        
                   } 
        
                   if (isValidPythonPath(path.join(pythonPath, 'bin', executableName))) { 
        
                       return path.join(pythonPath, 'bin', executableName); 
        
                   } 
        
               } 
        
           } 
        
           return pythonPath;

Also, the first check when ensuring if interpreter path is valid is we should be checking if the entered path exists, which we're not doing.
This leads to us launching 14 exec commands <python> -c '1234' everytime a user enters an invalid path - leading to extension consuming too much CPU.

cc Don Jayamanne (@DonJayamanne) Karthik Nadig (@karthiknadig)

Brett Cannon (@brettcannon)

This was for portability.
Users wanted to set the python path in their settings for all users in a team across multiple os.
E.g. assume users have a relative path to a virtual env in their workspace .venv, that would work on Mac Linux and windows without any other change.

Brett Cannon (@brettcannon) might recall this change bring added into the extension.

Karthik Nadig (@karthiknadig)

I see, Karthik Nadig (@karthiknadig) has observed that having these checks can wreck the SSH connection because of the 14 spawns made at worst case.
Also, I don't see this documented anywhere, wonder how many users are using this feature. Maybe we should add telemetry before deciding to removing these, or maybe have this as an optional feature?

Thanks to removing python.pythonPath, I don't think this is necessary anymore and we should just expect people to specify the full path to their environment's Python interpreter.

We now have folder support inbuilt in the new discovery code, the old code is no longer needed.

Verification steps:

Python: Select Interpreter -> Enter interpreter path -> Enter full path to the environment folder (for eg. full path to .venv folder, can even contain ~)
Ensure interpreter still gets selected

Remove old code for folder support in interpreter path setting (#22413)

Merged PR posit-dev/positron-python#277: Merge vscode-python 2023.22 …

Navigation Menu

Repository navigation

Remove old code for folder support in interpreter path settings #12452

12 remaining items

Verification steps:

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	const KnownPythonExecutables = ['python', 'python4', 'python3.6', 'python3.5', 'python3', 'python2.7', 'python2'];

	for (let executableName of KnownPythonExecutables) {
	// Suffix with 'python' for linux and 'osx', and 'python.exe' for 'windows'.
	if (IS_WINDOWS) {
	executableName = `${executableName}.exe`;
	if (isValidPythonPath(path.join(pythonPath, executableName))) {
	return path.join(pythonPath, executableName);
	}
	if (isValidPythonPath(path.join(pythonPath, 'scripts', executableName))) {
	return path.join(pythonPath, 'scripts', executableName);
	}
	} else {
	if (isValidPythonPath(path.join(pythonPath, executableName))) {
	return path.join(pythonPath, executableName);
	}
	if (isValidPythonPath(path.join(pythonPath, 'bin', executableName))) {
	return path.join(pythonPath, 'bin', executableName);
	}
	}
	}

	return pythonPath;

Remove old code for folder support in interpreter path settings #12452

Description

Activity

DonJayamanne commented on Jun 23, 2020

karrtikr commented on Jun 23, 2020

brettcannon commented on Jun 23, 2020

12 remaining items

karrtikr commented on Nov 2, 2023

karrtikr commented on Nov 2, 2023

Verification steps:

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions