Skip to content

Commit 933101a

Browse files
authored
Cherry pick more fixes for feb point2 (#10183)
* Ensure DS auto start code is non-blocking (#10171) * Perf fixes * Add news entry * Include interperter name in message (#10174) For #10071 Fix to ensure interpreter is included in message, here's the actual message seen by the user. Error: Jupyter cannot be started. Error attempting to locate jupyter: 'Kernelspec' module not installed in the selected interpreter ({0}). Note the {0} * Ignore display name when searching interpreters (#10175) Partial fix for #10173 Comparing against path of interpreter alone is sufficient. Figured there's no harm in trying to minimise occurrences of dup kernels (basically just compare against python executable path, as thats sufficient and always accurate) Display name: Can change from version to version of Python extension (i.e. it shouldn't have been used as a unique identifier) Display name can change after extension loads and more information about interpreter is available. * Track cold/warm times to execute notebook cells (#10180) For #10176
1 parent dcc695c commit 933101a

File tree

8 files changed

+31
-9
lines changed

8 files changed

+31
-9
lines changed

news/2 Fixes/10170.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Perf improvements to executing startup code for `Data Science` features when extension loads.

news/3 Code Health/10176.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Track cold/warm times to execute notebook cells.

src/client/datascience/activation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export class Activation implements IExtensionSingleActivationService {
3030
this.disposables.push(this.jupyterInterpreterService.onDidChangeInterpreter(this.onDidChangeInterpreter, this));
3131
// Warm up our selected interpreter for the extension
3232
this.jupyterInterpreterService.setInitialInterpreter().ignoreErrors();
33-
await this.contextService.activate();
33+
this.contextService.activate().ignoreErrors();
3434
}
3535

3636
private onDidOpenNotebookEditor(_: INotebookEditor) {

src/client/datascience/interactive-ipynb/nativeEditor.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
7777
public get onDidChangeViewState(): Event<void> {
7878
return this._onDidChangeViewState.event;
7979
}
80+
private sentExecuteCellTelemetry: boolean = false;
8081
private _onDidChangeViewState = new EventEmitter<void>();
8182
private closedEvent: EventEmitter<INotebookEditor> = new EventEmitter<INotebookEditor>();
8283
private executedEvent: EventEmitter<INotebookEditor> = new EventEmitter<INotebookEditor>();
@@ -362,8 +363,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
362363
}
363364

364365
protected submitCode(code: string, file: string, line: number, id?: string, editor?: TextEditor, debug?: boolean): Promise<boolean> {
366+
const stopWatch = new StopWatch();
367+
const submitCodePromise = super.submitCode(code, file, line, id, editor, debug).finally(() => this.sendPerceivedCellExecute(stopWatch));
365368
// When code is executed, update the version number in the metadata.
366-
return super.submitCode(code, file, line, id, editor, debug).then(value => {
369+
return submitCodePromise.then(value => {
367370
this.updateVersionInfoInNotebook()
368371
.then(() => {
369372
this.metadataUpdatedEvent.fire(this);
@@ -513,6 +516,18 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
513516
// Actually don't close, just let the error bubble out
514517
}
515518

519+
private sendPerceivedCellExecute(runningStopWatch?: StopWatch) {
520+
if (runningStopWatch) {
521+
const props = { notebook: true };
522+
if (!this.sentExecuteCellTelemetry) {
523+
this.sentExecuteCellTelemetry = true;
524+
sendTelemetryEvent(Telemetry.ExecuteCellPerceivedCold, runningStopWatch.elapsedTime, props);
525+
} else {
526+
sendTelemetryEvent(Telemetry.ExecuteCellPerceivedWarm, runningStopWatch.elapsedTime, props);
527+
}
528+
}
529+
}
530+
516531
/**
517532
* Update the Python Version number in the notebook data.
518533
*

src/client/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export class JupyterInterpreterSubCommandExecutionService implements IJupyterSub
8383
}
8484

8585
if (productsNotInstalled.length === 1 && productsNotInstalled[0] === Product.kernelspec) {
86-
return DataScience.jupyterKernelSpecModuleNotFound();
86+
return DataScience.jupyterKernelSpecModuleNotFound().format(interpreter.path);
8787
}
8888

8989
return getMessageForLibrariesNotInstalled(productsNotInstalled, interpreter.displayName);

src/client/datascience/jupyter/kernels/kernelService.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,6 @@ export class KernelService {
9999
if (item.language.toLowerCase() !== PYTHON_LANGUAGE.toLowerCase()) {
100100
return false;
101101
}
102-
if (item.display_name !== option.displayName) {
103-
return false;
104-
}
105102
return this.fileSystem.arePathsSame(item.argv[0], option.path) || this.fileSystem.arePathsSame(item.metadata?.interpreter?.path || '', option.path);
106103
});
107104
} else {

src/client/telemetry/index.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,8 +1459,16 @@ export interface IEventNamePropertyMapping {
14591459
[Telemetry.DisableInteractiveShiftEnter]: never | undefined;
14601460
[Telemetry.EnableInteractiveShiftEnter]: never | undefined;
14611461
[Telemetry.ExecuteCell]: never | undefined;
1462-
[Telemetry.ExecuteCellPerceivedCold]: never | undefined;
1463-
[Telemetry.ExecuteCellPerceivedWarm]: never | undefined;
1462+
/**
1463+
* Telemetry sent to capture first time execution of a cell.
1464+
* If `notebook = true`, this its telemetry for native editor/notebooks.
1465+
*/
1466+
[Telemetry.ExecuteCellPerceivedCold]: undefined | { notebook: boolean };
1467+
/**
1468+
* Telemetry sent to capture subsequent execution of a cell.
1469+
* If `notebook = true`, this its telemetry for native editor/notebooks.
1470+
*/
1471+
[Telemetry.ExecuteCellPerceivedWarm]: undefined | { notebook: boolean };
14641472
[Telemetry.ExecuteNativeCell]: never | undefined;
14651473
[Telemetry.ExpandAll]: never | undefined;
14661474
[Telemetry.ExportNotebook]: never | undefined;

src/test/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ suite('Data Science - Jupyter InterpreterSubCommandExecutionService', () => {
223223

224224
const reason = await jupyterInterpreterExecutionService.getReasonForJupyterNotebookNotBeingSupported(undefined);
225225

226-
assert.equal(reason, DataScience.jupyterKernelSpecModuleNotFound());
226+
assert.equal(reason, DataScience.jupyterKernelSpecModuleNotFound().format(selectedJupyterInterpreter.path));
227227
});
228228
test('Can start jupyer notebook', async () => {
229229
const output = await jupyterInterpreterExecutionService.startNotebook([], {});

0 commit comments

Comments
 (0)