Skip to content

Commit 97bad7d

Browse files
DonJayamanneDonJayamanne
authored
Use activated python proc to check if modules are installed (microsoft#9644)
For #9643 * Use activated python proc to check if modules are installed * Fixes to use python environment correctly * Fixes * Fix tests * Add news entry * Oops
1 parent 68a2dd2 commit 97bad7d

9 files changed

Lines changed: 15 additions & 12 deletions

File tree

‎news/2 Fixes/9566.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Use the correct interpreter when creating the Python execution service used as a fallback by the Daemon.

‎news/2 Fixes/9643.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Use an activated environment python process to check if modules are installed.

‎pythonFiles/datascience/daemon/daemon_python.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def __getitem__(self, item):
8282
self.log.debug("Ignoring non-exit method during shutdown: %s", item)
8383
raise KeyError
8484

85-
self.log.info("Execute rpc method %s", item)
85+
self.log.info("Execute rpc method %s from %s", item, sys.executable)
8686
return super().__getitem__(item)
8787

8888
def start(self):

‎src/client/common/installer/productInstaller.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,15 @@ export abstract class BaseInstaller {
7878
return true;
7979
}
8080
// User may have customized the module name or provided the fully qualified path.
81-
const pythonPath = isResource(resource) ? undefined : resource.path;
81+
const interpreter = isResource(resource) ? undefined : resource;
8282
const uri = isResource(resource) ? resource : undefined;
8383
const executableName = this.getExecutableNameFromSettings(product, uri);
8484

8585
const isModule = this.isExecutableAModule(product, uri);
8686
if (isModule) {
87-
const pythonProcess = await this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory).create({ resource: uri, pythonPath });
87+
const pythonProcess = await this.serviceContainer
88+
.get<IPythonExecutionFactory>(IPythonExecutionFactory)
89+
.createActivatedEnvironment({ resource: uri, interpreter, allowEnvironmentFetchExceptions: true });
8890
return pythonProcess.isModuleInstalled(executableName);
8991
} else {
9092
const process = await this.serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory).create(uri);

‎src/client/common/process/pythonExecutionFactory.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
6363
const disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
6464
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
6565
const logger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
66+
67+
const interpreter = await interpreterService.getInterpreterDetails(pythonPath);
6668
const activatedProcPromise = this.createActivatedEnvironment({
6769
allowEnvironmentFetchExceptions: true,
68-
pythonPath: pythonPath,
70+
interpreter: interpreter,
6971
resource: options.resource,
7072
bypassCondaExecution: true
7173
});
72-
7374
// No daemon support in Python 2.7.
74-
const interpreter = await interpreterService.getInterpreterDetails(pythonPath);
7575
if (interpreter?.version && interpreter.version.major < 3) {
7676
return activatedProcPromise!;
7777
}
@@ -90,7 +90,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
9090
return daemon;
9191
};
9292

93-
// Ensure we do not create muliple daemon pools for the same python interpreter.
93+
// Ensure we do not create multiple daemon pools for the same python interpreter.
9494
let promise = this.daemonsPerPythonService.get(daemonPoolKey);
9595
if (!promise) {
9696
promise = start();

‎src/client/common/process/types.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ export type DaemonExecutionFactoryCreationOptions = ExecutionFactoryCreationOpti
9999
};
100100
export type ExecutionFactoryCreateWithEnvironmentOptions = {
101101
resource?: Uri;
102-
pythonPath?: string;
103102
interpreter?: PythonInterpreter;
104103
allowEnvironmentFetchExceptions?: boolean;
105104
/**

‎src/client/datascience/jupyter/kernels/kernelService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ export class KernelService {
227227
throw new Error('Interpreter does not have a display name');
228228
}
229229

230-
const execServicePromise = this.execFactory.create({ pythonPath: interpreter.path });
230+
const execServicePromise = this.execFactory.createActivatedEnvironment({ interpreter, allowEnvironmentFetchExceptions: true, bypassCondaExecution: true });
231231
// Swallow errors if we get out of here and not resolve this.
232232
execServicePromise.ignoreErrors();
233233
const name = this.generateKernelNameForIntepreter(interpreter);

‎src/test/common/installer/installer.unit.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ suite('Module Installer only', () => {
476476
const pythonExecutionFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
477477
const pythonExecutionService = TypeMoq.Mock.ofType<IPythonExecutionService>();
478478
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory))).returns(() => pythonExecutionFactory.object);
479-
pythonExecutionFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonExecutionService.object));
479+
pythonExecutionFactory.setup(p => p.createActivatedEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonExecutionService.object));
480480
pythonExecutionService
481481
// tslint:disable-next-line: no-any
482482
.setup(p => (p as any).then)
@@ -496,7 +496,7 @@ suite('Module Installer only', () => {
496496
const pythonExecutionFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
497497
const pythonExecutionService = TypeMoq.Mock.ofType<IPythonExecutionService>();
498498
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory))).returns(() => pythonExecutionFactory.object);
499-
pythonExecutionFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonExecutionService.object));
499+
pythonExecutionFactory.setup(p => p.createActivatedEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonExecutionService.object));
500500
pythonExecutionService
501501
// tslint:disable-next-line: no-any
502502
.setup(p => (p as any).then)

‎src/test/datascience/jupyter/kernels/kernelService.unit.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ suite('Data Science - KernelService', () => {
5151
execService = mock(PythonExecutionService);
5252
installer = mock(ProductInstaller);
5353
jupyterInterpreterExecutionService = mock(JupyterCommandFinderInterpreterExecutionService);
54-
when(execFactory.create(anything())).thenResolve(instance(execService));
54+
when(execFactory.createActivatedEnvironment(anything())).thenResolve(instance(execService));
5555
// tslint:disable-next-line: no-any
5656
(instance(execService) as any).then = undefined;
5757

0 commit comments

Comments
 (0)