Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate Mac Interpreters in the background #3909

Merged
merged 2 commits into from Jan 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions news/1 Enhancements/3908.md
@@ -0,0 +1 @@
Validate Mac Interpreters in the background.
7 changes: 7 additions & 0 deletions src/client/application/diagnostics/applicationDiagnostics.ts
Expand Up @@ -9,6 +9,7 @@ import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
import { ILogger, IOutputChannel } from '../../common/types';
import { IServiceContainer } from '../../ioc/types';
import { IApplicationDiagnostics } from '../types';
import { InvalidMacPythonInterpreterServiceId } from './checks/macPythonInterpreter';
import { IDiagnostic, IDiagnosticsService, ISourceMapSupportService } from './types';

@injectable()
Expand All @@ -27,6 +28,12 @@ export class ApplicationDiagnostics implements IApplicationDiagnostics {
await diagnosticsService.handle(diagnostics);
}
}));

// Validate the Mac interperter in the background.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DonJayamanne Perhaps I'm being dense here, but you are already validating the Mac interpreter (not in the background) in the above lines 23 to 30.
In order to improve loading times of Python Extension, we should remove validating Mac interpreter in lines 23 to 30, and only validate it in the background.

const maccInterpreterService = this.serviceContainer.get<IDiagnosticsService>(IDiagnosticsService, InvalidMacPythonInterpreterServiceId);
maccInterpreterService.diagnose()
.then(diagnostics => maccInterpreterService.handle(diagnostics))
.ignoreErrors();
}
private log(diagnostics: IDiagnostic[]): void {
const logger = this.serviceContainer.get<ILogger>(ILogger);
Expand Down
140 changes: 140 additions & 0 deletions src/client/application/diagnostics/checks/macPythonInterpreter.ts
@@ -0,0 +1,140 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable } from 'inversify';
import { ConfigurationChangeEvent, DiagnosticSeverity, Uri } from 'vscode';
import { IWorkspaceService } from '../../../common/application/types';
import '../../../common/extensions';
import { IPlatformService } from '../../../common/platform/types';
import { IConfigurationService, IDisposableRegistry } from '../../../common/types';
import { IInterpreterHelper, IInterpreterService, InterpreterType } from '../../../interpreter/contracts';
import { IServiceContainer } from '../../../ioc/types';
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
import { IDiagnosticsCommandFactory } from '../commands/types';
import { DiagnosticCodes } from '../constants';
import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../promptHandler';
import { DiagnosticScope, IDiagnostic, IDiagnosticCommand, IDiagnosticHandlerService } from '../types';

const messages = {
[DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic]: 'You have selected the macOS system install of Python, which is not recommended for use with the Python extension. Some functionality will be limited, please select a different interpreter.',
[DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic]: 'The macOS system install of Python is not recommended, some functionality in the extension will be limited. Install another version of Python for the best experience.'
};

export class InvalidMacPythonInterpreterDiagnostic extends BaseDiagnostic {
constructor(code: DiagnosticCodes) {
super(code, messages[code], DiagnosticSeverity.Error, DiagnosticScope.WorkspaceFolder);
}
}

export const InvalidMacPythonInterpreterServiceId = 'InvalidMacPythonInterpreterServiceId';

@injectable()
export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
protected changeThrottleTimeout = 1000;
DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
private timeOut?: NodeJS.Timer;
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer,
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IPlatformService) private readonly platform: IPlatformService,
@inject(IInterpreterHelper) private readonly helper: IInterpreterHelper) {
super(
[
DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic,
DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic
], serviceContainer);
this.addPythonPathChangedHandler();
}
public async diagnose(): Promise<IDiagnostic[]> {
if (!this.platform.isMac) {
return [];
}
const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
const settings = configurationService.getSettings();
if (settings.disableInstallationChecks === true) {
return [];
}

const hasInterpreters = await this.interpreterService.hasInterpreters;
if (!hasInterpreters) {
return [];
}

const currentInterpreter = await this.interpreterService.getActiveInterpreter();
if (!currentInterpreter) {
return [];
}

if (!this.helper.isMacDefaultPythonPath(settings.pythonPath)) {
return [];
}
if (!currentInterpreter || currentInterpreter.type !== InterpreterType.Unknown) {
return [];
}

const interpreters = await this.interpreterService.getInterpreters();
if (interpreters.filter(i => !this.helper.isMacDefaultPythonPath(i.path)).length === 0) {
return [new InvalidMacPythonInterpreterDiagnostic(DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic)];
}

return [new InvalidMacPythonInterpreterDiagnostic(DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic)];
}
public async handle(diagnostics: IDiagnostic[]): Promise<void> {
if (diagnostics.length === 0) {
return;
}
const messageService = this.serviceContainer.get<IDiagnosticHandlerService<MessageCommandPrompt>>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerServiceId);
await Promise.all(diagnostics.map(async diagnostic => {
if (!this.canHandle(diagnostic)) {
return;
}
const commandPrompts = this.getCommandPrompts(diagnostic);
return messageService.handle(diagnostic, { commandPrompts, message: diagnostic.message });
}));
}
protected addPythonPathChangedHandler() {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
disposables.push(workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
}
protected async onDidChangeConfiguration(event: ConfigurationChangeEvent) {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.map(workspace => workspace.uri) : [undefined];
if (workspacesUris.findIndex(uri => event.affectsConfiguration('python.pythonPath', uri)) === -1) {
return;
}
// Lets wait, for more changes, dirty simple throttling.
if (this.timeOut) {
clearTimeout(this.timeOut);
this.timeOut = undefined;
}
this.timeOut = setTimeout(() => {
this.timeOut = undefined;
this.diagnose().then(dianostics => this.handle(dianostics)).ignoreErrors();
}, this.changeThrottleTimeout);
}
private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] {
const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
switch (diagnostic.code) {
case DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic: {
return [{
prompt: 'Select Python Interpreter',
command: commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: 'python.setInterpreter' })
}];
}
case DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic: {
return [{
prompt: 'Learn more',
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://code.visualstudio.com/docs/python/python-tutorial#_prerequisites' })
},
{
prompt: 'Download',
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://www.python.org/downloads' })
}];
}
default: {
throw new Error('Invalid diagnostic for \'InvalidMacPythonInterpreterService\'');
}
}
}
}
66 changes: 4 additions & 62 deletions src/client/application/diagnostics/checks/pythonInterpreter.ts
Expand Up @@ -4,12 +4,10 @@
'use strict';

import { inject, injectable } from 'inversify';
import { ConfigurationChangeEvent, DiagnosticSeverity, Uri } from 'vscode';
import { IWorkspaceService } from '../../../common/application/types';
import { DiagnosticSeverity } from 'vscode';
import '../../../common/extensions';
import { IPlatformService } from '../../../common/platform/types';
import { IConfigurationService, IDisposableRegistry } from '../../../common/types';
import { IInterpreterHelper, IInterpreterService, InterpreterType } from '../../../interpreter/contracts';
import { IConfigurationService } from '../../../common/types';
import { IInterpreterService } from '../../../interpreter/contracts';
import { IServiceContainer } from '../../../ioc/types';
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
import { IDiagnosticsCommandFactory } from '../commands/types';
Expand All @@ -19,8 +17,6 @@ import { DiagnosticScope, IDiagnostic, IDiagnosticCommand, IDiagnosticHandlerSer

const messages = {
[DiagnosticCodes.NoPythonInterpretersDiagnostic]: 'Python is not installed. Please download and install Python before using the extension.',
[DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic]: 'You have selected the macOS system install of Python, which is not recommended for use with the Python extension. Some functionality will be limited, please select a different interpreter.',
[DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic]: 'The macOS system install of Python is not recommended, some functionality in the extension will be limited. Install another version of Python for the best experience.',
[DiagnosticCodes.NoCurrentlySelectedPythonInterpreterDiagnostic]: 'No Python interpreter is selected. You need to select a Python interpreter to enable features such as IntelliSense, linting, and debugging.'
};

Expand All @@ -34,17 +30,12 @@ export const InvalidPythonInterpreterServiceId = 'InvalidPythonInterpreterServic

@injectable()
export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
protected changeThrottleTimeout = 1000;
private timeOut?: NodeJS.Timer;
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
super(
[
DiagnosticCodes.NoPythonInterpretersDiagnostic,
DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic,
DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic,
DiagnosticCodes.NoCurrentlySelectedPythonInterpreterDiagnostic
], serviceContainer);
this.addPythonPathChangedHandler();
}
public async diagnose(): Promise<IDiagnostic[]> {
const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
Expand All @@ -65,24 +56,7 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.NoCurrentlySelectedPythonInterpreterDiagnostic)];
}

const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
if (!platform.isMac) {
return [];
}

const helper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
if (!helper.isMacDefaultPythonPath(settings.pythonPath)) {
return [];
}
if (!currentInterpreter || currentInterpreter.type !== InterpreterType.Unknown) {
return [];
}
const interpreters = await interpreterService.getInterpreters();
if (interpreters.filter(i => !helper.isMacDefaultPythonPath(i.path)).length === 0) {
return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic)];
}

return [new InvalidPythonInterpreterDiagnostic(DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic)];
return [];
}
public async handle(diagnostics: IDiagnostic[]): Promise<void> {
if (diagnostics.length === 0) {
Expand All @@ -97,27 +71,6 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
return messageService.handle(diagnostic, { commandPrompts, message: diagnostic.message });
}));
}
protected addPythonPathChangedHandler() {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
disposables.push(workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
}
protected async onDidChangeConfiguration(event: ConfigurationChangeEvent) {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.map(workspace => workspace.uri) : [undefined];
if (workspacesUris.findIndex(uri => event.affectsConfiguration('python.pythonPath', uri)) === -1) {
return;
}
// Lets wait, for more changes, dirty simple throttling.
if (this.timeOut) {
clearTimeout(this.timeOut);
this.timeOut = undefined;
}
this.timeOut = setTimeout(() => {
this.timeOut = undefined;
this.diagnose().then(dianostics => this.handle(dianostics)).ignoreErrors();
}, this.changeThrottleTimeout);
}
private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] {
const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
switch (diagnostic.code) {
Expand All @@ -127,23 +80,12 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService {
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://www.python.org/downloads' })
}];
}
case DiagnosticCodes.MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic:
case DiagnosticCodes.NoCurrentlySelectedPythonInterpreterDiagnostic: {
return [{
prompt: 'Select Python Interpreter',
command: commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: 'python.setInterpreter' })
}];
}
case DiagnosticCodes.MacInterpreterSelectedAndNoOtherInterpretersDiagnostic: {
return [{
prompt: 'Learn more',
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://code.visualstudio.com/docs/python/python-tutorial#_prerequisites' })
},
{
prompt: 'Download',
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://www.python.org/downloads' })
}];
}
default: {
throw new Error('Invalid diagnostic for \'InvalidPythonInterpreterService\'');
}
Expand Down
2 changes: 2 additions & 0 deletions src/client/application/diagnostics/serviceRegistry.ts
Expand Up @@ -10,6 +10,7 @@ import { EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagn
import { InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId } from './checks/invalidDebuggerType';
import { InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId } from './checks/invalidPythonPathInDebugger';
import { LSNotSupportedDiagnosticService, LSNotSupportedDiagnosticServiceId } from './checks/lsNotSupported';
import { InvalidMacPythonInterpreterService, InvalidMacPythonInterpreterServiceId } from './checks/macPythonInterpreter';
import { PowerShellActivationHackDiagnosticsService, PowerShellActivationHackDiagnosticsServiceId } from './checks/powerShellActivation';
import { InvalidPythonInterpreterService, InvalidPythonInterpreterServiceId } from './checks/pythonInterpreter';
import { DiagnosticsCommandFactory } from './commands/factory';
Expand All @@ -27,6 +28,7 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, LSNotSupportedDiagnosticService, LSNotSupportedDiagnosticServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, PowerShellActivationHackDiagnosticsService, PowerShellActivationHackDiagnosticsServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidMacPythonInterpreterService, InvalidMacPythonInterpreterServiceId);
serviceManager.addSingleton<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory, DiagnosticsCommandFactory);
serviceManager.addSingleton<IApplicationDiagnostics>(IApplicationDiagnostics, ApplicationDiagnostics);
}