Skip to content

Commit

Permalink
Remove fallback via undefined from component adapter (#15231)
Browse files Browse the repository at this point in the history
* Remove falback via undefined from component adapter

* Revert "Remove falback via undefined from component adapter"

This reverts commit 16483d4.

* Remove fallback from onDidCreate in component adapter

* Remove fallback from hasInterpreters

* Remove fallback from isWindowsStoreInterpreter

* Remove fallback from isMacDefaultPythonPath

* Remove fallback from getWorkspaceVirtualEnvInterpreters

* Remove fallback from getInterpreterDetails

* Remove fallback for isCondaEnvironment

* Reove fallback from getCondaEnvironment

* Remove fallback from getInterpreterInformation

* Remove fallback from getInterpreters

* Remove fallback from getWinRegInterpreters

* Use sandbox for stubbing in tensorBoard tests

* Fix test issues
  • Loading branch information
karthiknadig authored Jan 27, 2021
1 parent de45178 commit 2402acc
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 132 deletions.
2 changes: 1 addition & 1 deletion src/client/interpreter/autoSelection/rules/winRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class WindowsRegistryInterpretersAutoSelectionRule extends BaseRuleServic
if (this.platform.osType !== OSType.Windows) {
return NextAction.runNextRule;
}
let interpreters: PythonEnvironment[] | undefined = [];
let interpreters: PythonEnvironment[] = [];
if (await inDiscoveryExperiment(this.experimentService)) {
interpreters = await this.pyenvs.getWinRegInterpreters(resource);
} else {
Expand Down
27 changes: 17 additions & 10 deletions src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,31 +33,38 @@ export interface IVirtualEnvironmentsSearchPathProvider {
export const IComponentAdapter = Symbol('IComponentAdapter');
export interface IComponentAdapter {
// VirtualEnvPrompt
onDidCreate(resource: Resource, callback: () => void): Disposable | undefined;
onDidCreate(resource: Resource, callback: () => void): Disposable;
// IInterpreterLocatorService
hasInterpreters: Promise<boolean | undefined>;
hasInterpreters: Promise<boolean>;
getInterpreters(
resource?: Uri,
options?: GetInterpreterOptions,
source?: PythonEnvSource[],
): Promise<PythonEnvironment[] | undefined>;
): Promise<PythonEnvironment[]>;

// WorkspaceVirtualEnvInterpretersAutoSelectionRule
getWorkspaceVirtualEnvInterpreters(
resource: Uri,
options?: { ignoreCache?: boolean },
): Promise<PythonEnvironment[] | undefined>;
): Promise<PythonEnvironment[]>;

// IInterpreterLocatorService (for WINDOWS_REGISTRY_SERVICE)
getWinRegInterpreters(resource: Resource): Promise<PythonEnvironment[] | undefined>;
getWinRegInterpreters(resource: Resource): Promise<PythonEnvironment[]>;
// IInterpreterService
getInterpreterDetails(pythonPath: string, _resource?: Uri): Promise<undefined | PythonEnvironment>;
getInterpreterDetails(pythonPath: string, _resource?: Uri): Promise<PythonEnvironment>;

// IInterpreterHelper
getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonEnvironment>>;
isMacDefaultPythonPath(pythonPath: string): Promise<boolean | undefined>;
// Undefined is expected on this API, if the environment info retrieval fails.
getInterpreterInformation(pythonPath: string): Promise<Partial<PythonEnvironment> | undefined>;

isMacDefaultPythonPath(pythonPath: string): Promise<boolean>;

// ICondaService
isCondaEnvironment(interpreterPath: string): Promise<boolean | undefined>;
isCondaEnvironment(interpreterPath: string): Promise<boolean>;
// Undefined is expected on this API, if the environment is not conda env.
getCondaEnvironment(interpreterPath: string): Promise<CondaEnvironmentInfo | undefined>;

isWindowsStoreInterpreter(pythonPath: string): Promise<boolean | undefined>;
isWindowsStoreInterpreter(pythonPath: string): Promise<boolean>;
}

export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService');
Expand Down
10 changes: 2 additions & 8 deletions src/client/interpreter/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,13 @@ export function isInterpreterLocatedInWorkspace(interpreter: PythonEnvironment,
return interpreterPath.startsWith(resourcePath);
}

// The parts of IComponentAdapter used here.
interface IComponent {
getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonEnvironment>>;
isMacDefaultPythonPath(pythonPath: string): Promise<boolean | undefined>;
}

@injectable()
export class InterpreterHelper implements IInterpreterHelper {
private readonly persistentFactory: IPersistentStateFactory;

constructor(
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
@inject(IComponentAdapter) private readonly pyenvs: IComponent,
@inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {
this.persistentFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
Expand Down Expand Up @@ -130,7 +124,7 @@ export class InterpreterHelper implements IInterpreterHelper {

public async isMacDefaultPythonPath(pythonPath: string): Promise<boolean> {
if (await inDiscoveryExperiment(this.experimentService)) {
return this.pyenvs.isMacDefaultPythonPath(pythonPath) && Promise.resolve(false);
return this.pyenvs.isMacDefaultPythonPath(pythonPath);
}

return isMacDefaultPythonPath(pythonPath);
Expand Down
17 changes: 5 additions & 12 deletions src/client/interpreter/interpreterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,12 @@ import { inDiscoveryExperiment } from '../common/experiments/helpers';

const EXPIRY_DURATION = 24 * 60 * 60 * 1000;

// The parts of IComponentAdapter used here.
interface IComponent {
hasInterpreters: Promise<boolean | undefined>;
getInterpreterDetails(pythonPath: string): Promise<undefined | PythonEnvironment>;
getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[] | undefined>;
}

@injectable()
export class InterpreterService implements Disposable, IInterpreterService {
public get hasInterpreters(): Promise<boolean> {
return inDiscoveryExperiment(this.experimentService).then((inExp) => {
if (inExp) {
return this.pyenvs.hasInterpreters && Promise.resolve(false);
return this.pyenvs.hasInterpreters;
}
const locator = this.serviceContainer.get<IInterpreterLocatorService>(
IInterpreterLocatorService,
Expand Down Expand Up @@ -95,7 +88,7 @@ export class InterpreterService implements Disposable, IInterpreterService {

constructor(
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
@inject(IComponentAdapter) private readonly pyenvs: IComponent,
@inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {
this.persistentStateFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
Expand Down Expand Up @@ -146,7 +139,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
public async getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[]> {
let environments: PythonEnvironment[] = [];
if (await inDiscoveryExperiment(this.experimentService)) {
environments = (await this.pyenvs.getInterpreters(resource, options)) ?? [];
environments = await this.pyenvs.getInterpreters(resource, options);
} else {
const locator = this.serviceContainer.get<IInterpreterLocatorService>(
IInterpreterLocatorService,
Expand Down Expand Up @@ -201,8 +194,8 @@ export class InterpreterService implements Disposable, IInterpreterService {
}

public async getInterpreterDetails(pythonPath: string, resource?: Uri): Promise<PythonEnvironment | undefined> {
const info = await this.pyenvs.getInterpreterDetails(pythonPath);
if (info !== undefined) {
if (await inDiscoveryExperiment(this.experimentService)) {
const info = await this.pyenvs.getInterpreterDetails(pythonPath);
if (!info.displayName) {
// Set display name for the environment returned by component if it's not set (this should eventually go away)
info.displayName = await this.getDisplayName(info, resource);
Expand Down
26 changes: 11 additions & 15 deletions src/client/interpreter/virtualEnvs/virtualEnvPrompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,18 @@ export class VirtualEnvironmentPrompt implements IExtensionActivationService {
public async activate(resource: Uri): Promise<void> {
if (await inDiscoveryExperiment(this.experimentService)) {
const disposable = this.pyenvs.onDidCreate(resource, () => this.handleNewEnvironment(resource));
if (disposable) {
this.disposableRegistry.push(disposable);
}
// This is to prevent fallback
return;
this.disposableRegistry.push(disposable);
} else {
const builder = this.serviceContainer.get<IInterpreterWatcherBuilder>(IInterpreterWatcherBuilder);
const watcher = await builder.getWorkspaceVirtualEnvInterpreterWatcher(resource);
watcher.onDidCreate(
() => {
this.handleNewEnvironment(resource).ignoreErrors();
},
this,
this.disposableRegistry,
);
}

const builder = this.serviceContainer.get<IInterpreterWatcherBuilder>(IInterpreterWatcherBuilder);
const watcher = await builder.getWorkspaceVirtualEnvInterpreterWatcher(resource);
watcher.onDidCreate(
() => {
this.handleNewEnvironment(resource).ignoreErrors();
},
this,
this.disposableRegistry,
);
}

@traceDecorators.error('Error in event handler for detection of new environment')
Expand Down
6 changes: 3 additions & 3 deletions src/client/jupyter/jupyterIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface ILanguageServer extends Disposable {
}

/**
* This allows Python exntension to update Product enum without breaking Jupyter.
* This allows Python extension to update Product enum without breaking Jupyter.
* I.e. we have a strict contract, else using numbers (in enums) is bound to break across products.
*/
enum JupyterProductToInstall {
Expand Down Expand Up @@ -169,7 +169,7 @@ export class JupyterExtensionIntegration {
) => this.envActivation.getActivatedEnvironmentVariables(resource, interpreter, allowExceptions),
isWindowsStoreInterpreter: async (pythonPath: string): Promise<boolean> => {
if (await inDiscoveryExperiment(this.experimentService)) {
return this.pyenvs.isWindowsStoreInterpreter(pythonPath) && Promise.resolve(false);
return this.pyenvs.isWindowsStoreInterpreter(pythonPath);
}
return isWindowsStoreInterpreter(pythonPath);
},
Expand All @@ -188,7 +188,7 @@ export class JupyterExtensionIntegration {
const interpreter = !isResource(r) ? r : undefined;
const client = await this.languageServerCache.get(resource, interpreter);

// Some langauge servers don't support the connection yet. (like Jedi until we switch to LSP)
// Some language servers don't support the connection yet. (like Jedi until we switch to LSP)
if (client && client.connection && client.capabilities) {
return {
connection: client.connection,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
IDisposableRegistry,
IExperimentService,
IPersistentStateFactory,
Resource,
} from '../../../../common/types';
import { cache } from '../../../../common/utils/decorators';
import {
Expand Down Expand Up @@ -57,13 +56,6 @@ export const CondaLocationsGlobWin = `{${condaGlobPathsForWindows.join(',')}}`;

export const CondaGetEnvironmentPrefix = 'Outputting Environment Now...';

// The parts of IComponentAdapter used here.
interface IComponent {
isCondaEnvironment(interpreterPath: string): Promise<boolean | undefined>;
getCondaEnvironment(interpreterPath: string): Promise<CondaEnvironmentInfo | undefined>;
getWinRegInterpreters(resource: Resource): Promise<PythonEnvironment[] | undefined>;
}

/**
* A wrapper around a conda installation.
*/
Expand All @@ -86,7 +78,7 @@ export class CondaService implements ICondaService {
@inject(IConfigurationService) private configService: IConfigurationService,
@inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IComponentAdapter) private readonly pyenvs: IComponent,
@inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
@inject(IInterpreterLocatorService)
@named(WINDOWS_REGISTRY_SERVICE)
Expand Down Expand Up @@ -226,10 +218,10 @@ export class CondaService implements ICondaService {
* @memberof CondaService
*/
public async isCondaEnvironment(interpreterPath: string): Promise<boolean> {
const result = await this.pyenvs.isCondaEnvironment(interpreterPath);
if (result !== undefined) {
return result;
if (await inDiscoveryExperiment(this.experimentService)) {
return this.pyenvs.isCondaEnvironment(interpreterPath);
}

const dir = path.dirname(interpreterPath);
const { isWindows } = this.platform;
const condaMetaDirectory = isWindows ? path.join(dir, 'conda-meta') : path.join(dir, '..', 'conda-meta');
Expand All @@ -240,10 +232,10 @@ export class CondaService implements ICondaService {
* Return (env name, interpreter filename) for the interpreter.
*/
public async getCondaEnvironment(interpreterPath: string): Promise<{ name: string; path: string } | undefined> {
const found = await this.pyenvs.getCondaEnvironment(interpreterPath);
if (found !== undefined) {
return found;
if (await inDiscoveryExperiment(this.experimentService)) {
return this.pyenvs.getCondaEnvironment(interpreterPath);
}

const isCondaEnv = await this.isCondaEnvironment(interpreterPath);
if (!isCondaEnv) {
return undefined;
Expand Down Expand Up @@ -410,7 +402,7 @@ export class CondaService implements ICondaService {
return 'conda';
}
if (this.platform.isWindows) {
const interpreters: PythonEnvironment[] = (await this.getWinRegEnvs()) || [];
const interpreters: PythonEnvironment[] = await this.getWinRegEnvs();
const condaInterpreters = interpreters.filter(CondaService.detectCondaEnvironment);
const condaInterpreter = CondaService.getLatestVersion(condaInterpreters);
if (condaInterpreter) {
Expand All @@ -426,7 +418,7 @@ export class CondaService implements ICondaService {
return this.getCondaFileFromKnownLocations();
}

private async getWinRegEnvs(): Promise<PythonEnvironment[] | undefined> {
private async getWinRegEnvs(): Promise<PythonEnvironment[]> {
if (await inDiscoveryExperiment(this.experimentService)) {
return this.pyenvs.getWinRegInterpreters(undefined);
}
Expand Down
Loading

0 comments on commit 2402acc

Please sign in to comment.