don't try to prewarm an execution service (microsoft#13377)

* don't try to prewarm an execution service * remove funky old cast * update unit tests Co-authored-by: Ian Huff <ianhuff@ravikun-dev2.redmond.corp.microsoft.com>

Author: IanMatthewHuff

news/2 Fixes/13258.md

@@ -0,0 +1 @@
+Prevent daemon from trying to prewarm an execution service.

src/client/common/process/pythonExecutionFactory.ts

@@ -76,7 +76,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
 
  public async createDaemon<T extends IPythonDaemonExecutionService | IDisposable>(
  options: DaemonExecutionFactoryCreationOptions
- ): Promise<T> {
+ ): Promise<T | IPythonExecutionService> {
  const pythonPath = options.pythonPath
  ? options.pythonPath
  : this.configService.getSettings(options.resource).pythonPath;
@@ -93,7 +93,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
  });
  // No daemon support in Python 2.7 or during shutdown
  if (!interpreterService || (interpreter?.version && interpreter.version.major < 3)) {
- return (activatedProcPromise! as unknown) as T;
+ return activatedProcPromise;
  }
 
  // Ensure we do not start multiple daemons for the same interpreter.

src/client/common/process/types.ts

@@ -154,7 +154,7 @@ export interface IPythonExecutionFactory {
  */
  createDaemon<T extends IPythonDaemonExecutionService | IDisposable>(
  options: DaemonExecutionFactoryCreationOptions
- ): Promise<T>;
+ ): Promise<T | IPythonExecutionService>;
  createActivatedEnvironment(options: ExecutionFactoryCreateWithEnvironmentOptions): Promise<IPythonExecutionService>;
  createCondaExecutionService(
  pythonPath: string,

src/client/datascience/kernel-launcher/kernelDaemonPool.ts

@@ -7,7 +7,7 @@ import { inject, injectable } from 'inversify';
 import { IWorkspaceService } from '../../common/application/types';
 import { traceError } from '../../common/logger';
 
-import { IPythonExecutionFactory } from '../../common/process/types';
+import { IPythonExecutionFactory, IPythonExecutionService } from '../../common/process/types';
 import { IDisposable, Resource } from '../../common/types';
 import { noop } from '../../common/utils/misc';
 import { IEnvironmentVariablesProvider } from '../../common/variables/types';
@@ -23,7 +23,7 @@ type IKernelDaemonInfo = {
  workspaceResource: Resource;
  workspaceFolderIdentifier: string;
  interpreterPath: string;
- daemon: Promise<IPythonKernelDaemon>;
+ daemon: Promise<IPythonKernelDaemon | IPythonExecutionService>;
 };
 
 @injectable()
@@ -68,7 +68,7 @@ export class KernelDaemonPool implements IDisposable {
  resource: Resource,
  kernelSpec: IJupyterKernelSpec,
  interpreter?: PythonInterpreter
- ): Promise<IPythonKernelDaemon> {
+ ): Promise<IPythonKernelDaemon | IPythonExecutionService> {
  const pythonPath = interpreter?.path || kernelSpec.argv[0];
  // If we have environment variables in the kernel.json, then its not we support.
  // Cuz there's no way to know before hand what kernelspec can be used, hence no way to know what envs are required.
@@ -104,14 +104,26 @@ export class KernelDaemonPool implements IDisposable {
  dedicated: true,
  resource
  });
- daemon.then((d) => this.disposables.push(d)).catch(noop);
+ daemon
+ .then((d) => {
+ if ('dispose' in d) {
+ this.disposables.push(d);
+ }
+ })
+ .catch(noop);
  return daemon;
  }
  private async onDidEnvironmentVariablesChange(affectedResoruce: Resource) {
  const workspaceFolderIdentifier = this.workspaceService.getWorkspaceFolderIdentifier(affectedResoruce);
  this.daemonPool = this.daemonPool.filter((item) => {
  if (item.workspaceFolderIdentifier === workspaceFolderIdentifier) {
- item.daemon.then((d) => d.dispose()).catch(noop);
+ item.daemon
+ .then((d) => {
+ if ('dispose' in d) {
+ d.dispose();
+ }
+ })
+ .catch(noop);
  return false;
  }
  return true;
@@ -132,7 +144,16 @@ export class KernelDaemonPool implements IDisposable {
  const daemon = this.createDaemon(resource, interpreter.path);
  // Once a daemon is created ensure we pre-warm it (will load ipykernel and start the kernker process waiting to start the actual kernel code).
  // I.e. we'll start python process thats the kernel, but will not start the kernel module (`python -m ipykernel`).
- daemon.then((d) => d.preWarm()).catch(traceError.bind(`Failed to prewarm kernel daemon ${interpreter.path}`));
+ daemon
+ .then((d) => {
+ // Prewarm if we support prewarming
+ if ('preWarm' in d) {
+ d.preWarm().catch(traceError.bind(`Failed to prewarm kernel daemon ${interpreter.path}`));
+ }
+ })
+ .catch((e) => {
+ traceError(`Failed to load deamon: ${e}`);
+ });
  this.daemonPool.push({
  daemon,
  interpreterPath: interpreter.path,
@@ -172,7 +193,13 @@ export class KernelDaemonPool implements IDisposable {
  this.daemonPool = this.daemonPool.filter((item) => {
  const interpreterForWorkspace = currentInterpreterInEachWorksapce.get(item.key);
  if (!interpreterForWorkspace || !this.fs.areLocalPathsSame(interpreterForWorkspace, item.interpreterPath)) {
- item.daemon.then((d) => d.dispose()).catch(noop);
+ item.daemon
+ .then((d) => {
+ if ('dispose' in d) {
+ d.dispose();
+ }
+ })
+ .catch(noop);
  return false;
  }
 

src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts

@@ -6,7 +6,7 @@
 import { ChildProcess } from 'child_process';
 import { inject, injectable } from 'inversify';
 import { IDisposable } from 'monaco-editor';
-import { IPythonExecutionService, ObservableExecutionResult } from '../../common/process/types';
+import { ObservableExecutionResult } from '../../common/process/types';
 import { Resource } from '../../common/types';
 import { noop } from '../../common/utils/misc';
 import { PythonInterpreter } from '../../pythonEnvironments/info';
@@ -47,14 +47,9 @@ export class PythonKernelLauncherDaemon implements IDisposable {
 
  // The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2.
  // Use a check for the daemon.start function here before we call it.
- if (!daemon.start) {
+ if (!('start' in daemon)) {
  // If we don't have a KernelDaemon here then we have an execution service and should use that to launch
- // Typing is a bit funk here, as createDaemon can return an execution service instead of the requested
- // daemon class
- // tslint:disable-next-line:no-any
- const executionService = (daemon as any) as IPythonExecutionService;
-
- const observableOutput = executionService.execModuleObservable(moduleName, moduleArgs, {
+ const observableOutput = daemon.execModuleObservable(moduleName, moduleArgs, {
  env,
  cwd: workingDirectory
  });

src/test/datascience/kernel-launcher/kernelDaemonPool.unit.test.ts

@@ -66,6 +66,9 @@ suite('DataScience - Kernel Daemon Pool', () => {
  when(daemon1.preWarm()).thenResolve();
  when(daemon2.preWarm()).thenResolve();
  when(daemon3.preWarm()).thenResolve();
+ when(daemon1.dispose()).thenResolve();
+ when(daemon2.dispose()).thenResolve();
+ when(daemon3.dispose()).thenResolve();
 
  when(envVars.onDidEnvironmentVariablesChange).thenReturn(didEnvVarsChange.event);
  when(interpeterService.onDidChangeInterpreter).thenReturn(didChangeInterpreter.event);

src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts

@@ -66,8 +66,6 @@ suite('DataScience - Kernel Launcher Daemon', () => {
  when(
  executionService.execModuleObservable('ipkernel_launcher', deepEqual(['-f', 'file.json']), anything())
  ).thenReturn(instance(observableOutputForDaemon));
- // Make sure that it doesn't have a start function. Normally the proxy will pretend to have a start function, which we are checking for non-existance
- when((executionService as any).start).thenReturn(false);
  // Else ts-mockit doesn't allow us to return an instance of a mock as a return value from an async function.
  (instance(executionService) as any).then = undefined;
  when(daemonPool.get(anything(), anything(), anything())).thenResolve(instance(executionService) as any);