Repository navigation
WIP - Stop using exec - #8230
Karthik Nadig (karthiknadig) wants to merge 2 commits into
Conversation
Don Jayamanne (DonJayamanne)
left a comment
Don Jayamanne (DonJayamanne)
left a comment
There was a problem hiding this comment.
Now, we shouldn't be changing everything to use python process factory (sooner places can have severe perf implications, specially in Windows, major differences).
| return processService.exec(pythonPath, ['--version'], { mergeStdOutErr: true }) | ||
| const processService = await this.pythonServiceFactory.create({ pythonPath: pythonPath }); | ||
| return processService.exec(['--version'], { mergeStdOutErr: true }) | ||
| .then(output => output.stdout.splitLines()[0]) |
There was a problem hiding this comment.
For version info, I'd use the process service. This is used in a number of contexts and for optimization we shouldn't use python process, as that information might not be available.
Basically for perf don't change.
| const processService = await this.pythonServiceFactory.create({ pythonPath: options.command }); | ||
| const args = Array.isArray(options.args) ? options.args : []; | ||
| return processService.exec(options.command, args.concat(['-c', 'import sys;print(sys.executable)']), {}) | ||
| return processService.exec(args.concat(['-c', 'import sys;print(sys.executable)']), {}) |
There was a problem hiding this comment.
This is required to build interpreter information. I.e. python Execution factory friends on this
|
Don Jayamanne (@DonJayamanne) See my comments here #7697 (comment) Definitely like your feed back there. I need to push one more PR to match the exact updates in the comment i made on the issue. |
|
Please don't update |
2 participants
For #7697