Repository navigation
Stop using "exec()" in the extension. #7697
Description
Switch to using PythonExecutionFactory (instead of exec()) in the following places:
- testing
src/client/testing/common/services/discovery.tssrc/client/testing/navigation/symbolProvider.ts
- installers
src/client/common/installer/moduleInstaller.tssrc/client/common/installer/poetryInstaller.ts
[ ] old debug adapter- interpreter discovery
src/client/interpreter/display/shebangCodeLensProvider.tssrc/client/interpreter/interpreterVersion.tssrc/client/interpreter/locators/services/condaService.tssrc/client/interpreter/locators/services/currentPathService.tssrc/client/interpreter/locators/services/pipEnvService.tssrc/client/interpreter/virtualEnvs/index.ts
- import sorting
src/client/providers/importSortProvider.ts
- code execution / terminal
src/client/terminals/codeExecution/helper.ts
Activity
karthiknadig commentedon Oct 26, 2019
Eric Snow (@ericsnowcurrently) what is the motivation for this work item?
This is what i am seeing so far: #8230
src/client/testing/common/services/discovery.ts: Looks like it is already using IPythonExecutionService
src/client/testing/navigation/symbolProvider.ts: Updated
src/client/common/installer/moduleInstaller.ts: Looks like this needs sudo support in IProcessService. Not really a IPythonExecutionService
src/client/common/installer/poetryInstaller.ts: This is using poetry path setting. Since that path is to an executable this should not be using IPythonExecutionService
src/client/interpreter/display/shebangCodeLensProvider.ts: I see IProcessService.exec() being used here. I see
getFullyQualifiedPathToInterpreter, is the idea to replace it with IPythonExecutionService?src/client/interpreter/interpreterVersion.ts: Updated.
src/client/interpreter/locators/services/condaService.ts: This is conda specific, in some cases cond has to be activated to directly use python. I don't see why this should use IPythonExecutionService
src/client/interpreter/locators/services/currentPathService.ts: Updated. Feels like it should not use that.
src/client/interpreter/locators/services/pipEnvService.ts: Updated. Feels like it should not use that.
src/client/interpreter/virtualEnvs/index.ts: Uses pyenv not python. So should not use IPythonExecutionService
src/client/providers/importSortProvider.ts: Already using IPythonExecutionService when using
sortImports.pysrc/client/terminals/codeExecution/helper.ts: Updated
DonJayamanne commentedon Oct 26, 2019
Agreed with karthik, except with interpreter version. Don't update, comments in PR.
ericsnowcurrently commentedon Oct 28, 2019
What do you mean by "what is the motivation"? If you mean "why does this issue exist?", I created it to track what files were running Python using exec (or a proxy for it) directly, rather than through the PythonExecutionService. Those places would not be able to take advantage of specializations in PythonExecutionService, including our use of "conda run".
ericsnowcurrently commentedon Oct 28, 2019
Yep, I missed that.
Shouldn't PythonExecutionService support sudo then? Otherwise we have to duplicate here the specializations we make there, including "conda run" support. Also, our use of sudo should be extrenely limited and possibly removed if possible.
Using the poetry path setting is potentially problematic, since it would not necessarily have effect on the currently selected interpreter environment. Also, I expect it does not cooperate well with Anaconda. So this needs more understanding.
The main goal is to use PythonExecutionService, one way or the other. :)
I think you're right. These are interpreter locators. So they should run indepedently of the currently selected interpreter. That said, there may be more to it than that.
Yeah, I'm pretty sure this is locator code too.
Yeah, I think you're right.
kimadeline commentedon Nov 7, 2019
✅ Validated using 2019.11.47235-dev:
src/client/testing/navigation/symbolProvider.ts: navigated to tests from the test explorer viewsrc/client/terminals/codeExecution/helper.ts: executed line in terminal, executed selection in terminal