Stop using "exec()" in the extension. #7697

Activity

1. Eric Snow (ericsnowcurrently)

   added

   needs PR

   debtCode quality issuesCode quality issues

   feature-interpreter

   on Sep 30, 2019
2. Karthik Nadig (karthiknadig)

   added this to the 2019 October Sprint 2 milestone on Oct 9, 2019
3. Graham Wheeler (gramster)

   added

   area-environmentsFeatures relating to handling interpreter environmentsFeatures relating to handling interpreter environments

   and removed

   feature-interpreter

   on Oct 10, 2019
4. Karthik Nadig (karthiknadig)

   self-assigned this

   on Oct 19, 2019
5. Karthik Nadig (karthiknadig)

   modified the milestones: 2019 October Sprint 2, 2019 October Sprint 3 on Oct 23, 2019
6. karthiknadig commented on Oct 26, 2019

   

   Karthik Nadig (karthiknadig)

   on Oct 26, 2019

   Last edited by karthiknadig

   Member

   More actions

   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.py

   • src/client/terminals/codeExecution/helper.ts: Updated
7. DonJayamanne commented on Oct 26, 2019

   

   Don Jayamanne (DonJayamanne)

   on Oct 26, 2019

   More actions

   Agreed with karthik, except with interpreter version. Don't update, comments in PR.
8. ericsnowcurrently commented on Oct 28, 2019

   

   Eric Snow (ericsnowcurrently)

   on Oct 28, 2019

   Author

   More actions

   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".
9. ericsnowcurrently commented on Oct 28, 2019

   

   Eric Snow (ericsnowcurrently)

   on Oct 28, 2019

   Author

   More actions

   • src/client/testing/common/services/discovery.ts: Looks like it is already using IPythonExecutionService

   Yep, I missed that.

   • src/client/common/installer/moduleInstaller.ts: Looks like this needs sudo support in IProcessService. Not really a IPythonExecutionService

   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.

   • 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

   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.

   • src/client/interpreter/display/shebangCodeLensProvider.ts: I see IProcessService.exec() being used here. I see getFullyQualifiedPathToInterpreter, is the idea to replace it with IPythonExecutionService?

   The main goal is to use PythonExecutionService, one way or the other. :)

   • 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.

   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.

   • src/client/interpreter/virtualEnvs/index.ts: Uses pyenv not python. So should not use IPythonExecutionService

   Yeah, I'm pretty sure this is locator code too.

   • src/client/providers/importSortProvider.ts: Already using IPythonExecutionService when using sortImports.py

   Yeah, I think you're right.
10. Karthik Nadig (karthiknadig)

    added

    in progress

    on Oct 30, 2019
11. Karthik Nadig (karthiknadig)

    assigned

    Kim-Adeline Miguel (kimadeline)

    on Nov 5, 2019
12. Karthik Nadig (karthiknadig)

    added

    in review

    and removed

    in progress

    on Nov 5, 2019
13. Karthik Nadig (karthiknadig)

    modified the milestones: 2019 October Sprint 3, 2019 November Sprint 1 on Nov 6, 2019
14. kimadeline commented on Nov 7, 2019

    

    Kim-Adeline Miguel (kimadeline)

    on Nov 7, 2019

    More actions

    ✅ Validated using 2019.11.47235-dev:

    • src/client/testing/navigation/symbolProvider.ts: navigated to tests from the test explorer view
    • src/client/terminals/codeExecution/helper.ts: executed line in terminal, executed selection in terminal
15. Kim-Adeline Miguel (kimadeline)

    closed this as completedon Nov 7, 2019
16. ghost removed

    needs PR

    on Nov 7, 2019
17. lock

    locked as resolved and limited conversation to collaborators on Nov 14, 2019