Skip to content

Commit 8874744

Browse files
karthiknadigkarthiknadig
authored
Ensure wheels experiment control and experiment groups uses right wheel (microsoft#8461)
* Ensure wheels control group uses no wheel ptvsd * Improve condition readability in jupyter debugger
1 parent 4303d8e commit 8874744

8 files changed

Lines changed: 138 additions & 63 deletions

File tree

‎build/ci/templates/steps/build.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ steps:
2222
python -m pip install -U pip
2323
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r requirements.txt
2424
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python/old_ptvsd --no-cache-dir --implementation py --no-deps --upgrade 'ptvsd==4.3.2'
25+
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python/new_ptvsd/no_wheels --no-cache-dir --implementation py --no-deps --upgrade 'ptvsd==5.0.0a7'
2526
failOnStderr: true
2627
displayName: "pip install requirements"
2728

‎build/ci/templates/test_phases.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ steps:
9696
python -m pip install --upgrade -r build/test-requirements.txt
9797
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r requirements.txt
9898
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python/old_ptvsd --no-cache-dir --implementation py --no-deps --upgrade 'ptvsd==4.3.2'
99+
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python/new_ptvsd/no_wheels --no-cache-dir --implementation py --no-deps --upgrade 'ptvsd==5.0.0a7'
99100
displayName: 'pip install system test requirements'
100101
condition: and(succeeded(), eq(variables['NeedsPythonTestReqs'], 'true'))
101102

‎pythonFiles/install_ptvsd.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77

88
EXTENSION_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
9-
DEBUGGER_DEST = os.path.join(EXTENSION_ROOT, "pythonFiles", "lib", "python")
9+
DEBUGGER_DEST = os.path.join(EXTENSION_ROOT, "pythonFiles", "lib", "python", "new_ptvsd", "wheels")
1010
DEBUGGER_PACKAGE = "ptvsd"
1111
DEBUGGER_VERSION = "5.0.0a7"
1212
DEBUGGER_PYTHON_VERSIONS = ("cp37",)

‎src/client/datascience/jupyter/jupyterDebugger.ts‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,7 @@ import { EXTENSION_ROOT_DIR } from '../../constants';
1717
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
1818
import { concatMultilineStringOutput } from '../common';
1919
import { Identifiers, Telemetry } from '../constants';
20-
import {
21-
CellState,
22-
ICell,
23-
ICellHashListener,
24-
IConnection,
25-
IFileHashes,
26-
IJupyterDebugger,
27-
INotebook,
28-
ISourceMapRequest
29-
} from '../types';
20+
import { CellState, ICell, ICellHashListener, IConnection, IFileHashes, IJupyterDebugger, INotebook, ISourceMapRequest } from '../types';
3021
import { JupyterDebuggerNotInstalledError } from './jupyterDebuggerNotInstalledError';
3122
import { JupyterDebuggerRemoteNotSupported } from './jupyterDebuggerRemoteNotSupported';
3223
import { ILiveShareHasRole } from './liveshare/types';
@@ -45,8 +36,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
4536
@inject(IPlatformService) private platform: IPlatformService,
4637
@inject(IWorkspaceService) private workspace: IWorkspaceService,
4738
@inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager
48-
) {
49-
}
39+
) {}
5040

5141
public async startDebugging(notebook: INotebook): Promise<void> {
5242
traceInfo('start debugging');
@@ -114,9 +104,11 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
114104
public async hashesUpdated(hashes: IFileHashes[]): Promise<void> {
115105
// Make sure that we have an active debugging session at this point
116106
if (this.debugService.activeDebugSession) {
117-
await Promise.all(hashes.map((fileHash) => {
118-
return this.debugService.activeDebugSession!.customRequest('setPydevdSourceMap', this.buildSourceMap(fileHash));
119-
}));
107+
await Promise.all(
108+
hashes.map(fileHash => {
109+
return this.debugService.activeDebugSession!.customRequest('setPydevdSourceMap', this.buildSourceMap(fileHash));
110+
})
111+
);
120112
}
121113
}
122114

@@ -184,18 +176,19 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
184176
*/
185177
private async getPtvsdPath(notebook: INotebook): Promise<string> {
186178
const oldPtvsd = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'old_ptvsd');
187-
if (!this.experimentsManager.inExperiment(DebugAdapterDescriptorFactory.experiment) ||
188-
!this.experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment)){
179+
if (!this.experimentsManager.inExperiment(DebugAdapterDescriptorFactory.experiment) || !this.experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment)) {
189180
return oldPtvsd;
190181
}
191182
const pythonVersion = await this.getKernelPythonVersion(notebook);
192-
// The new debug adapter is only supported in 3.7
183+
// The new debug adapter with wheels is only supported in 3.7
193184
// Code can be found here (src/client/debugger/extension/adapter/factory.ts).
194-
if (!pythonVersion || pythonVersion.major < 3 || pythonVersion.minor < 7){
195-
return oldPtvsd;
185+
if (pythonVersion && pythonVersion.major === 3 && pythonVersion.minor === 7) {
186+
// Return debugger with wheels
187+
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'wheels');
196188
}
197189

198-
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python');
190+
// We are here so this is NOT python 3.7, return debugger without wheels
191+
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'no_wheels');
199192
}
200193
private async calculatePtvsdPathList(notebook: INotebook): Promise<string | undefined> {
201194
const extraPaths: string[] = [];
@@ -311,7 +304,12 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
311304
const minor = parseInt(packageVersionMatch[2], 10);
312305
const patch = parseInt(packageVersionMatch[3], 10);
313306
return {
314-
major, minor, patch, build: [], prerelease: [], raw: `${major}.${minor}.${patch}`
307+
major,
308+
minor,
309+
patch,
310+
build: [],
311+
prerelease: [],
312+
raw: `${major}.${minor}.${patch}`
315313
};
316314
}
317315
}
@@ -335,7 +333,11 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
335333
@captureTelemetry(Telemetry.PtvsdPromptToInstall)
336334
private async promptToInstallPtvsd(notebook: INotebook, oldVersion: Version | undefined): Promise<void> {
337335
const promptMessage = oldVersion ? localize.DataScience.jupyterDebuggerInstallPtvsdUpdate() : localize.DataScience.jupyterDebuggerInstallPtvsdNew();
338-
const result = await this.appShell.showInformationMessage(promptMessage, localize.DataScience.jupyterDebuggerInstallPtvsdYes(), localize.DataScience.jupyterDebuggerInstallPtvsdNo());
336+
const result = await this.appShell.showInformationMessage(
337+
promptMessage,
338+
localize.DataScience.jupyterDebuggerInstallPtvsdYes(),
339+
localize.DataScience.jupyterDebuggerInstallPtvsdNo()
340+
);
339341

340342
if (result === localize.DataScience.jupyterDebuggerInstallPtvsdYes()) {
341343
await this.installPtvsd(notebook);
@@ -424,7 +426,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
424426
const data = outputs[0].data;
425427
if (data && data.hasOwnProperty('text/plain')) {
426428
// tslint:disable-next-line:no-any
427-
return ((data as any)['text/plain']);
429+
return (data as any)['text/plain'];
428430
}
429431
if (outputs[0].output_type === 'stream') {
430432
const stream = outputs[0] as nbformat.IStream;

‎src/client/debugger/extension/adapter/factory.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { traceVerbose } from '../../../common/logger';
1212
import { IExperimentsManager } from '../../../common/types';
1313
import { EXTENSION_ROOT_DIR } from '../../../constants';
1414
import { IInterpreterService } from '../../../interpreter/contracts';
15+
import { sendTelemetryEvent } from '../../../telemetry';
16+
import { EventName } from '../../../telemetry/constants';
1517
import { RemoteDebugOptions } from '../../debugAdapter/types';
1618
import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
1719
import { IDebugAdapterDescriptorFactory } from '../types';
@@ -24,7 +26,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
2426
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
2527
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
2628
@inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager
27-
) { }
29+
) {}
2830
public async createDebugAdapterDescriptor(session: DebugSession, executable: DebugAdapterExecutable | undefined): Promise<DebugAdapterDescriptor> {
2931
const configuration = session.configuration as (LaunchRequestArguments | AttachRequestArguments);
3032

@@ -37,11 +39,17 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
3739
return new DebugAdapterServer(port, configuration.host);
3840
} else {
3941
const pythonPath = await this.getPythonPath(configuration, session.workspaceFolder);
40-
if (await this.useNewPtvsd(pythonPath)) {
41-
// If logToFile is set in the debug config then pass --log-dir <path-to-extension-dir> when launching the debug adapter.
42-
const logArgs = configuration.logToFile ? ['--log-dir', EXTENSION_ROOT_DIR] : [];
43-
const ptvsdPathToUse = this.getPtvsdPath();
44-
return new DebugAdapterExecutable(pythonPath, [path.join(ptvsdPathToUse, 'adapter'), ...logArgs]);
42+
// If logToFile is set in the debug config then pass --log-dir <path-to-extension-dir> when launching the debug adapter.
43+
const logArgs = configuration.logToFile ? ['--log-dir', EXTENSION_ROOT_DIR] : [];
44+
const ptvsdPathToUse = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd');
45+
if (pythonPath.length !== 0) {
46+
if (await this.useNewPtvsd(pythonPath)) {
47+
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, { usingWheels: true });
48+
return new DebugAdapterExecutable(pythonPath, [path.join(ptvsdPathToUse, 'wheels', 'ptvsd', 'adapter'), ...logArgs]);
49+
} else {
50+
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, { usingWheels: false });
51+
return new DebugAdapterExecutable(pythonPath, [path.join(ptvsdPathToUse, 'no_wheels', 'ptvsd', 'adapter'), ...logArgs]);
52+
}
4553
}
4654
}
4755
} else {
@@ -73,7 +81,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
7381
}
7482

7583
public getPtvsdPath(): string {
76-
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'ptvsd');
84+
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'no_wheels', 'ptvsd');
7785
}
7886

7987
public getRemotePtvsdArgs(remoteDebugOptions: RemoteDebugOptions): string[] {

‎src/client/telemetry/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export enum EventName {
3939
WORKSPACE_SYMBOLS_GO_TO = 'WORKSPACE_SYMBOLS.GO_TO',
4040
EXECUTION_CODE = 'EXECUTION_CODE',
4141
EXECUTION_DJANGO = 'EXECUTION_DJANGO',
42+
DEBUG_ADAPTER_USING_WHEELS_PATH = 'DEBUG_ADAPTER.USING_WHEELS_PATH',
4243
DEBUG_SESSION_ERROR = 'DEBUG_SESSION.ERROR',
4344
DEBUG_SESSION_START = 'DEBUG_SESSION.START',
4445
DEBUG_SESSION_STOP = 'DEBUG_SESSION.STOP',

‎src/client/telemetry/index.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,17 @@ export interface IEventNamePropertyMapping {
264264
*/
265265
enabled: boolean;
266266
};
267+
/**
268+
* Telemetry event captured when debug adapter executable is created
269+
*/
270+
[EventName.DEBUG_ADAPTER_USING_WHEELS_PATH]: {
271+
/**
272+
* Carries boolean
273+
* - `true` if path used for the adapter is the debugger with wheels.
274+
* - `false` if path used for the adapter is the source only version of the debugger.
275+
*/
276+
usingWheels: boolean;
277+
};
267278
/**
268279
* Telemetry captured before starting debug session.
269280
*/

0 commit comments

Comments
 (0)