Skip to content

Commit a6b29b6

Browse files
karthiknadigkarthiknadig
committed
Ensure wheels control group uses no wheel ptvsd
1 parent 764106b commit a6b29b6

File tree

8 files changed

+118
-43
lines changed

8 files changed

+118
-43
lines changed

‎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: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,13 +189,15 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
189189
return oldPtvsd;
190190
}
191191
const pythonVersion = await this.getKernelPythonVersion(notebook);
192-
// The new debug adapter is only supported in 3.7
192+
// The new debug adapter with wheels is only supported in 3.7
193193
// Code can be found here (src/client/debugger/extension/adapter/factory.ts).
194-
if (!pythonVersion || pythonVersion.major < 3 || pythonVersion.minor < 7){
195-
return oldPtvsd;
194+
if (!pythonVersion || !(pythonVersion.major === 3 && pythonVersion.minor === 7)) {
195+
// Return debugger without wheels
196+
path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'no_wheels');
196197
}
197198

198-
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python');
199+
// We are here so tis is python 3.7, return debugger with wheels
200+
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'wheels');
199201
}
200202
private async calculatePtvsdPathList(notebook: INotebook): Promise<string | undefined> {
201203
const extraPaths: string[] = [];

‎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
*/

‎src/test/debugger/extension/adapter/factory.unit.test.ts

Lines changed: 82 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ suite('Debugging - Adapter Factory', () => {
4545
let spiedInstance: ExperimentsManager;
4646

4747
const nodeExecutable = { command: 'node', args: [] };
48-
const ptvsdAdapterPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'ptvsd', 'adapter');
48+
const ptvsdAdapterPathWithWheels = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'wheels', 'ptvsd', 'adapter');
49+
const ptvsdAdapterPathWithoutWheels = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'new_ptvsd', 'no_wheels', 'ptvsd', 'adapter');
4950
const pythonPath = path.join('path', 'to', 'python', 'interpreter');
5051
const interpreter = {
5152
architecture: Architecture.Unknown,
@@ -55,6 +56,15 @@ suite('Debugging - Adapter Factory', () => {
5556
type: InterpreterType.Unknown,
5657
version: new SemVer('3.7.4-test')
5758
};
59+
const python36Path = path.join('path', 'to', 'active', 'interpreter');
60+
const interpreterPython36Details = {
61+
architecture: Architecture.Unknown,
62+
path: python36Path,
63+
sysPrefix: '',
64+
sysVersion: '',
65+
type: InterpreterType.Unknown,
66+
version: new SemVer('3.6.8-test')
67+
};
5868
const oldValueOfVSC_PYTHON_UNIT_TEST = process.env.VSC_PYTHON_UNIT_TEST;
5969
const oldValueOfVSC_PYTHON_CI_TEST = process.env.VSC_PYTHON_CI_TEST;
6070

@@ -103,11 +113,7 @@ suite('Debugging - Adapter Factory', () => {
103113
when(interpreterService.getInterpreterDetails(pythonPath)).thenResolve(interpreter);
104114
when(interpreterService.getInterpreters(anything())).thenResolve([interpreter]);
105115

106-
factory = new DebugAdapterDescriptorFactory(
107-
instance(interpreterService),
108-
instance(appShell),
109-
experimentsManager
110-
);
116+
factory = new DebugAdapterDescriptorFactory(instance(interpreterService), instance(appShell), experimentsManager);
111117
});
112118

113119
teardown(() => {
@@ -133,7 +139,7 @@ suite('Debugging - Adapter Factory', () => {
133139

134140
test('Return the value of configuration.pythonPath as the current python path if it exists and if we are in the experiment', async () => {
135141
const session = createSession({ pythonPath });
136-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath]);
142+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);
137143

138144
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
139145

@@ -144,7 +150,7 @@ suite('Debugging - Adapter Factory', () => {
144150

145151
test('Return the path of the active interpreter as the current python path if we are in the experiment, it exists and configuration.pythonPath is not defined', async () => {
146152
const session = createSession({});
147-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath]);
153+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);
148154

149155
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
150156
when(interpreterService.getActiveInterpreter(anything())).thenResolve(interpreter);
@@ -156,7 +162,7 @@ suite('Debugging - Adapter Factory', () => {
156162

157163
test('Return the path of the first available interpreter as the current python path if we are in the experiment, configuration.pythonPath is not defined and there is no active interpreter', async () => {
158164
const session = createSession({});
159-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath]);
165+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);
160166

161167
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
162168

@@ -213,27 +219,21 @@ suite('Debugging - Adapter Factory', () => {
213219
assert.deepEqual(descriptor, nodeExecutable);
214220
});
215221

216-
test('Return old node debugger when the active interpreter is not Python 3.7', async () => {
217-
const python36Path = path.join('path', 'to', 'active', 'interpreter');
218-
const interpreterPython36Details = {
219-
architecture: Architecture.Unknown,
220-
path: pythonPath,
221-
sysPrefix: '',
222-
sysVersion: '',
223-
type: InterpreterType.Unknown,
224-
version: new SemVer('3.6.8-test')
225-
};
222+
test('Return Python debug adapter without wheels executable when the active interpreter is not Python 3.7', async () => {
223+
const debugExecutable = new DebugAdapterExecutable(python36Path, [ptvsdAdapterPathWithoutWheels]);
226224
const session = createSession({});
227225

226+
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
227+
when(interpreterService.getInterpreters(anything())).thenResolve([interpreterPython36Details]);
228228
when(interpreterService.getInterpreterDetails(python36Path)).thenResolve(interpreterPython36Details);
229229

230230
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
231231

232-
assert.deepEqual(descriptor, nodeExecutable);
232+
assert.deepEqual(descriptor, debugExecutable);
233233
});
234234

235-
test('Return Python debug adapter executable when in the experiment and with the active interpreter being Python 3.7', async () => {
236-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath]);
235+
test('Return Python debug adapter with wheels executable when in the experiment and with the active interpreter being Python 3.7', async () => {
236+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);
237237
const session = createSession({});
238238

239239
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
@@ -250,9 +250,9 @@ suite('Debugging - Adapter Factory', () => {
250250
await expect(promise).to.eventually.be.rejectedWith('Debug Adapter Executable not provided');
251251
});
252252

253-
test('Pass the --log-dir argument to PTVSD is configuration.logToFile is set', async () => {
253+
test('Pass the --log-dir argument to PTVSD if configuration.logToFile is set, with active interpreter Python 3.7', async () => {
254254
const session = createSession({ logToFile: true });
255-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath, '--log-dir', EXTENSION_ROOT_DIR]);
255+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels, '--log-dir', EXTENSION_ROOT_DIR]);
256256

257257
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
258258

@@ -261,9 +261,22 @@ suite('Debugging - Adapter Factory', () => {
261261
assert.deepEqual(descriptor, debugExecutable);
262262
});
263263

264-
test('Don\'t pass the --log-dir argument to PTVSD is configuration.logToFile is not set', async () => {
264+
test('Pass the --log-dir argument to PTVSD if configuration.logToFile is set, with active interpreter not Python 3.7', async () => {
265+
const session = createSession({ logToFile: true });
266+
const debugExecutable = new DebugAdapterExecutable(python36Path, [ptvsdAdapterPathWithoutWheels, '--log-dir', EXTENSION_ROOT_DIR]);
267+
268+
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
269+
when(interpreterService.getInterpreters(anything())).thenResolve([interpreterPython36Details]);
270+
when(interpreterService.getInterpreterDetails(python36Path)).thenResolve(interpreterPython36Details);
271+
272+
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
273+
274+
assert.deepEqual(descriptor, debugExecutable);
275+
});
276+
277+
test('Don\'t pass the --log-dir argument to PTVSD if configuration.logToFile is not set, with active interpreter Python 3.7', async () => {
265278
const session = createSession({});
266-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath]);
279+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);
267280

268281
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
269282

@@ -272,25 +285,63 @@ suite('Debugging - Adapter Factory', () => {
272285
assert.deepEqual(descriptor, debugExecutable);
273286
});
274287

275-
test('Don\'t pass the --log-dir argument to PTVSD is configuration.logToFile is set but false', async () => {
288+
test('Don\'t pass the --log-dir argument to PTVSD if configuration.logToFile is not set, with active interpreter not Python 3.7', async () => {
289+
const session = createSession({});
290+
const debugExecutable = new DebugAdapterExecutable(python36Path, [ptvsdAdapterPathWithoutWheels]);
291+
292+
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
293+
when(interpreterService.getInterpreters(anything())).thenResolve([interpreterPython36Details]);
294+
when(interpreterService.getInterpreterDetails(python36Path)).thenResolve(interpreterPython36Details);
295+
296+
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
297+
298+
assert.deepEqual(descriptor, debugExecutable);
299+
});
300+
301+
test('Don\'t pass the --log-dir argument to PTVSD if configuration.logToFile is set but false, with active interpreter Python 3.7', async () => {
302+
const session = createSession({ logToFile: false });
303+
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);
304+
305+
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
306+
307+
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
308+
309+
assert.deepEqual(descriptor, debugExecutable);
310+
});
311+
312+
test('Don\'t pass the --log-dir argument to PTVSD if configuration.logToFile is set but false, with active interpreter not Python 3.7', async () => {
276313
const session = createSession({ logToFile: false });
277-
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPath]);
314+
const debugExecutable = new DebugAdapterExecutable(python36Path, [ptvsdAdapterPathWithoutWheels]);
278315

279316
when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
317+
when(interpreterService.getInterpreters(anything())).thenResolve([interpreterPython36Details]);
318+
when(interpreterService.getInterpreterDetails(python36Path)).thenResolve(interpreterPython36Details);
280319

281320
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
282321

283322
assert.deepEqual(descriptor, debugExecutable);
284323
});
285324

286-
test('Send experiment group telemetry if inside the wheels experiment', async () => {
325+
test('Send experiment group telemetry if inside the wheels experiment, with active interpreter Python 3.7', async () => {
287326
const session = createSession({});
288327
when(spiedInstance.userExperiments).thenReturn([{ name: DebugAdapterNewPtvsd.experiment, salt: DebugAdapterNewPtvsd.experiment, min: 0, max: 0 }]);
289328

290329
await factory.createDebugAdapterDescriptor(session, nodeExecutable);
291330

292-
assert.deepEqual(Reporter.eventNames, [EventName.PYTHON_EXPERIMENTS]);
293-
assert.deepEqual(Reporter.properties, [{ expName: DebugAdapterNewPtvsd.experiment }]);
331+
assert.deepEqual(Reporter.eventNames, [EventName.PYTHON_EXPERIMENTS, EventName.DEBUG_ADAPTER_USING_WHEELS_PATH]);
332+
assert.deepEqual(Reporter.properties, [{ expName: DebugAdapterNewPtvsd.experiment }, { usingWheels: 'true' }]);
333+
});
334+
335+
test('Send experiment group telemetry if inside the wheels experiment, with active interpreter not Python 3.6', async () => {
336+
const session = createSession({});
337+
when(spiedInstance.userExperiments).thenReturn([{ name: DebugAdapterNewPtvsd.experiment, salt: DebugAdapterNewPtvsd.experiment, min: 0, max: 0 }]);
338+
when(interpreterService.getInterpreters(anything())).thenResolve([interpreterPython36Details]);
339+
when(interpreterService.getInterpreterDetails(python36Path)).thenResolve(interpreterPython36Details);
340+
341+
await factory.createDebugAdapterDescriptor(session, nodeExecutable);
342+
343+
assert.deepEqual(Reporter.eventNames, [EventName.PYTHON_EXPERIMENTS, EventName.DEBUG_ADAPTER_USING_WHEELS_PATH]);
344+
assert.deepEqual(Reporter.properties, [{ expName: DebugAdapterNewPtvsd.experiment }, { usingWheels: 'false' }]);
294345
});
295346

296347
test('Send control group telemetry if inside the DA experiment control group', async () => {

0 commit comments

Comments
 (0)