Skip to content

Commit

Permalink
More changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj committed Jan 19, 2023
1 parent a1137df commit 8a6d48a
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 9 deletions.
3 changes: 3 additions & 0 deletions src/client/activation/activationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { IFileSystem } from '../common/platform/types';
import { IDisposable, IInterpreterPathService, Resource } from '../common/types';
import { Deferred } from '../common/utils/async';
import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types';
import { IActivatedEnvironmentLaunch } from '../interpreter/contracts';
import { traceDecoratorError } from '../logging';
import { sendActivationTelemetry } from '../telemetry/envFileTelemetry';
import { IExtensionActivationManager, IExtensionActivationService, IExtensionSingleActivationService } from './types';
Expand All @@ -37,6 +38,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
@inject(IFileSystem) private readonly fileSystem: IFileSystem,
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService,
@inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
@inject(IActivatedEnvironmentLaunch) private readonly activatedEnvLaunch: IActivatedEnvironmentLaunch,
) {}

private filterServices() {
Expand Down Expand Up @@ -91,6 +93,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {

if (this.workspaceService.isTrusted) {
// Do not interact with interpreters in a untrusted workspace.
await this.activatedEnvLaunch.selectIfLaunchedViaActivatedEnv();
await this.autoSelection.autoSelectInterpreter(resource);
await this.interpreterPathService.copyOldInterpreterStorageValuesToNew(resource);
}
Expand Down
12 changes: 10 additions & 2 deletions src/client/common/persistentState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { inject, injectable, named } from 'inversify';
import { Memento } from 'vscode';
import { IExtensionSingleActivationService } from '../activation/types';
import { traceError } from '../logging';
import { traceError, traceVerbose, traceWarn } from '../logging';
import { ICommandManager } from './application/types';
import { Commands } from './constants';
import {
Expand Down Expand Up @@ -41,13 +41,21 @@ export class PersistentState<T> implements IPersistentState<T> {
}
}

public async updateValue(newValue: T): Promise<void> {
public async updateValue(newValue: T, retryOnce = true): Promise<void> {
try {
if (this.expiryDurationMs) {
await this.storage.update(this.key, { data: newValue, expiry: Date.now() + this.expiryDurationMs });
} else {
await this.storage.update(this.key, newValue);
}
if (retryOnce && JSON.stringify(this.value) != JSON.stringify(newValue)) {
traceVerbose('Storage update failed for key', this.key, ' retrying');
await this.updateValue(undefined as T, false);
await this.updateValue(newValue, false);
if (JSON.stringify(this.value) != JSON.stringify(newValue)) {
traceWarn('Retry failed, storage update failed for key', this.key);
}
}
} catch (ex) {
traceError('Error while updating storage for key:', this.key, ex);
}
Expand Down
6 changes: 5 additions & 1 deletion src/client/common/process/pythonExecutionFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { inject, injectable } from 'inversify';

import { IEnvironmentActivationService } from '../../interpreter/activation/types';
import { IComponentAdapter } from '../../interpreter/contracts';
import { IActivatedEnvironmentLaunch, IComponentAdapter } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
Expand Down Expand Up @@ -52,6 +52,10 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
public async create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService> {
let { pythonPath } = options;
if (!pythonPath || pythonPath === 'python') {
const activatedEnvLaunch = this.serviceContainer.get<IActivatedEnvironmentLaunch>(
IActivatedEnvironmentLaunch,
);
await activatedEnvLaunch.selectIfLaunchedViaActivatedEnv();
// If python path wasn't passed in, we need to auto select it and then read it
// from the configuration.
const interpreterPath = this.interpreterPathExpHelper.get(options.resource);
Expand Down
4 changes: 1 addition & 3 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,9 @@ export namespace Interpreters {
'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings.',
);
export const activatedCondaEnvLaunch = l10n.t(
'Interpreters.activatedCondaEnvLaunch',
'We noticed VSCode was launched from an activated conda environment, would you like to select it?',
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
);
export const environmentPromptMessage = l10n.t(
'Interpreters.environmentPromptMessage',
'We noticed a new environment has been created. Do you want to select it for the workspace folder?',
);
export const entireWorkspace = l10n.t('Select at workspace level');
Expand Down
8 changes: 6 additions & 2 deletions src/client/interpreter/interpreterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ export class InterpreterService implements Disposable, IInterpreterService {
}

public async refresh(resource?: Uri): Promise<void> {
const activatedEnvLaunch = this.serviceContainer.get<IActivatedEnvironmentLaunch>(IActivatedEnvironmentLaunch);
await activatedEnvLaunch.selectIfLaunchedViaActivatedEnv();
const interpreterDisplay = this.serviceContainer.get<IInterpreterDisplay>(IInterpreterDisplay);
await interpreterDisplay.refresh(resource);
this.ensureEnvironmentContainsPython(this.configService.getSettings(resource).pythonPath).ignoreErrors();
Expand Down Expand Up @@ -182,6 +180,12 @@ export class InterpreterService implements Disposable, IInterpreterService {
}

public async getActiveInterpreter(resource?: Uri): Promise<PythonEnvironment | undefined> {
const activatedEnvLaunch = this.serviceContainer.get<IActivatedEnvironmentLaunch>(IActivatedEnvironmentLaunch);
await activatedEnvLaunch.selectIfLaunchedViaActivatedEnv();
// Config service also updates itself on interpreter config change,
// so yielding control here to make sure it goes first and updates
// itself before we can query it.
await sleep(1);
let path = this.configService.getSettings(resource).pythonPath;
if (pathUtils.basename(path) === path) {
// Value can be `python`, `python3`, `python3.9` etc.
Expand Down
6 changes: 5 additions & 1 deletion src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { ConfigurationTarget } from 'vscode';
import { IExtensionSingleActivationService } from '../../activation/types';
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IProcessServiceFactory } from '../../common/process/types';
import { sleep } from '../../common/utils/async';
import { cache } from '../../common/utils/decorators';
import { Common, Interpreters } from '../../common/utils/localize';
import { traceError, traceWarn } from '../../logging';
import { Conda } from '../../pythonEnvironments/common/environmentManagers/conda';
Expand Down Expand Up @@ -82,13 +84,15 @@ export class ActivatedEnvironmentLaunch implements IExtensionSingleActivationSer
}
}

@cache(-1, true)
public async selectIfLaunchedViaActivatedEnv(): Promise<void> {
this.wasTriggered = true;
const prefix = await this.getPrefixOfActivatedEnv();
if (!prefix) {
return;
}
this.wasTriggered = true;
await this.setPrefixAsInterpeter(prefix);
await sleep(1);
}

private async setPrefixAsInterpeter(prefix: string) {
Expand Down
7 changes: 7 additions & 0 deletions src/test/activation/activationManager.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { FileSystem } from '../../client/common/platform/fileSystem';
import { IFileSystem } from '../../client/common/platform/types';
import { IDisposable, IInterpreterPathService } from '../../client/common/types';
import { IInterpreterAutoSelectionService } from '../../client/interpreter/autoSelection/types';
import { IActivatedEnvironmentLaunch } from '../../client/interpreter/contracts';
import * as EnvFileTelemetry from '../../client/telemetry/envFileTelemetry';
import { sleep } from '../core';

Expand All @@ -43,8 +44,11 @@ suite('Activation Manager', () => {
let activeResourceService: IActiveResourceService;
let documentManager: typemoq.IMock<IDocumentManager>;
let interpreterPathService: typemoq.IMock<IInterpreterPathService>;
let activatedEnvLaunch: typemoq.IMock<IActivatedEnvironmentLaunch>;
let fileSystem: IFileSystem;
setup(() => {
activatedEnvLaunch = typemoq.Mock.ofType<IActivatedEnvironmentLaunch>();
activatedEnvLaunch.setup((a) => a.selectIfLaunchedViaActivatedEnv()).returns(() => Promise.resolve());
interpreterPathService = typemoq.Mock.ofType<IInterpreterPathService>();
interpreterPathService
.setup((i) => i.copyOldInterpreterStorageValuesToNew(typemoq.It.isAny()))
Expand All @@ -70,6 +74,7 @@ suite('Activation Manager', () => {
instance(fileSystem),
instance(activeResourceService),
interpreterPathService.object,
activatedEnvLaunch.object,
);

sinon.stub(EnvFileTelemetry, 'sendActivationTelemetry').resolves();
Expand Down Expand Up @@ -102,6 +107,7 @@ suite('Activation Manager', () => {
instance(fileSystem),
instance(activeResourceService),
interpreterPathService.object,
activatedEnvLaunch.object,
);
await managerTest.activateWorkspace(resource);

Expand Down Expand Up @@ -132,6 +138,7 @@ suite('Activation Manager', () => {
instance(fileSystem),
instance(activeResourceService),
interpreterPathService.object,
activatedEnvLaunch.object,
);
await managerTest.activateWorkspace(resource);

Expand Down

0 comments on commit 8a6d48a

Please sign in to comment.