Skip to content

Commit 91d4fed

Browse files
DonJayamanneDonJayamanne
authored
Include error in status and persist exec times (microsoft#12134)
For #10496 * Persist execution times in cell metadata * Update status if there are errors to include error message Tests when opening an existing notebook * Validate metaata * Validate status message
1 parent b74f424 commit 91d4fed

9 files changed

Lines changed: 295 additions & 13 deletions

File tree

‎src/client/datascience/notebook/executionHelpers.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,27 @@ export function updateCellOutput(notebookCellModel: ICell, outputs: nbformat.IOu
140140
model.update(updateCell);
141141
}
142142

143+
/**
144+
* Store execution start and end times in ISO format for portability.
145+
*/
146+
export function updateCellExecutionTimes(
147+
notebookCellModel: ICell,
148+
model: INotebookModel,
149+
startTime?: number,
150+
duration?: number
151+
) {
152+
const startTimeISO = startTime ? new Date(startTime).toISOString() : undefined;
153+
const endTimeISO = duration && startTime ? new Date(startTime + duration).toISOString() : undefined;
154+
updateCellMetadata(
155+
notebookCellModel,
156+
{
157+
end_execution_time: endTimeISO,
158+
start_execution_time: startTimeISO
159+
},
160+
model
161+
);
162+
}
163+
143164
export function updateCellMetadata(
144165
notebookCellModel: ICell,
145166
metadata: Partial<IBaseCellVSCodeMetadata>,

‎src/client/datascience/notebook/executionService.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ import {
2323
handleUpdateDisplayDataMessage,
2424
hasTransientOutputForAnotherCell,
2525
updateCellExecutionCount,
26+
updateCellExecutionTimes,
2627
updateCellOutput,
2728
updateCellWithErrorStatus
2829
} from './executionHelpers';
30+
import { getCellStatusMessageBasedOnFirstErrorOutput } from './helpers';
2931
import { INotebookExecutionService } from './types';
3032
// tslint:disable-next-line: no-var-requires no-require-imports
3133
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
@@ -185,9 +187,23 @@ export class NotebookExecutionService implements INotebookExecutionService {
185187
? vscodeNotebookEnums.NotebookCellRunState.Idle
186188
: vscodeNotebookEnums.NotebookCellRunState.Success;
187189
cell.metadata.statusMessage = '';
190+
191+
// Update metadata in our model.
192+
const notebookCellModel = findMappedNotebookCellModel(cell, model.cells);
193+
updateCellExecutionTimes(
194+
notebookCellModel,
195+
model,
196+
cell.metadata.runStartTime,
197+
cell.metadata.lastRunDuration
198+
);
199+
188200
// If there are any errors in the cell, then change status to error.
189201
if (cell.outputs.some((output) => output.outputKind === vscodeNotebookEnums.CellOutputKind.Error)) {
190202
cell.metadata.runState = vscodeNotebookEnums.NotebookCellRunState.Error;
203+
cell.metadata.statusMessage = getCellStatusMessageBasedOnFirstErrorOutput(
204+
// tslint:disable-next-line: no-any
205+
notebookCellModel.data.outputs as any
206+
);
191207
}
192208
deferred.resolve();
193209
}

‎src/client/datascience/notebook/helpers.ts‎

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {
1616
// tslint:disable-next-line: no-var-requires no-require-imports
1717
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
1818
import * as uuid from 'uuid/v4';
19+
import { NotebookCellRunState } from '../../../../typings/vscode-proposed';
1920
import {
2021
concatMultilineStringInput,
2122
concatMultilineStringOutput,
@@ -90,24 +91,62 @@ export function cellToVSCNotebookCellData(cell: ICell): NotebookCellData | undef
9091
return;
9192
}
9293

93-
return {
94+
// tslint:disable-next-line: no-any
95+
const outputs = cellOutputsToVSCCellOutputs(cell.data.outputs as any);
96+
97+
// If we have an execution count & no errors, then success state.
98+
// If we have an execution count & errors, then error state.
99+
// Else idle state.
100+
const hasErrors = outputs.some((output) => output.outputKind === vscodeNotebookEnums.CellOutputKind.Error);
101+
const hasExecutionCount = typeof cell.data.execution_count === 'number' && cell.data.execution_count > 0;
102+
let runState: NotebookCellRunState;
103+
let statusMessage: string | undefined;
104+
if (!hasExecutionCount) {
105+
runState = vscodeNotebookEnums.NotebookCellRunState.Idle;
106+
} else if (hasErrors) {
107+
runState = vscodeNotebookEnums.NotebookCellRunState.Error;
108+
// Error details are stripped from the output, get raw output.
109+
// tslint:disable-next-line: no-any
110+
statusMessage = getCellStatusMessageBasedOnFirstErrorOutput(cell.data.outputs as any);
111+
} else {
112+
runState = vscodeNotebookEnums.NotebookCellRunState.Success;
113+
}
114+
115+
const notebookCellData: NotebookCellData = {
94116
cellKind:
95117
cell.data.cell_type === 'code' ? vscodeNotebookEnums.CellKind.Code : vscodeNotebookEnums.CellKind.Markdown,
96118
language: cell.data.cell_type === 'code' ? PYTHON_LANGUAGE : MARKDOWN_LANGUAGE,
97119
metadata: {
98120
editable: true,
99121
executionOrder: typeof cell.data.execution_count === 'number' ? cell.data.execution_count : undefined,
100122
hasExecutionOrder: cell.data.cell_type === 'code',
101-
runState: vscodeNotebookEnums.NotebookCellRunState.Idle,
123+
runState,
102124
runnable: cell.data.cell_type === 'code',
103125
custom: {
104126
cellId: cell.id
105127
}
106128
},
107129
source: concatMultilineStringInput(cell.data.source),
108-
// tslint:disable-next-line: no-any
109-
outputs: cellOutputsToVSCCellOutputs(cell.data.outputs as any)
130+
outputs
110131
};
132+
133+
if (statusMessage) {
134+
notebookCellData.metadata.statusMessage = statusMessage;
135+
}
136+
137+
const startExecutionTime = cell.data.metadata.vscode?.start_execution_time
138+
? new Date(Date.parse(cell.data.metadata.vscode.start_execution_time)).getTime()
139+
: undefined;
140+
const endExecutionTime = cell.data.metadata.vscode?.end_execution_time
141+
? new Date(Date.parse(cell.data.metadata.vscode.end_execution_time)).getTime()
142+
: undefined;
143+
144+
if (startExecutionTime && typeof endExecutionTime === 'number') {
145+
notebookCellData.metadata.runStartTime = startExecutionTime;
146+
notebookCellData.metadata.lastRunDuration = endExecutionTime - startExecutionTime;
147+
}
148+
149+
return notebookCellData;
111150
}
112151

113152
export function cellOutputsToVSCCellOutputs(outputs?: nbformat.IOutput[]): CellOutput[] {
@@ -205,11 +244,31 @@ function translateStreamOutput(output: nbformat.IStream): CellStreamOutput | Cel
205244
}
206245
};
207246
}
247+
248+
/**
249+
* We will display the error message in the status of the cell.
250+
* The `ename` & `evalue` is displayed at the top of the output by VS Code.
251+
* As we're displaying the error in the statusbar, we don't want this dup error in output.
252+
* Hence remove this.
253+
*/
208254
export function translateErrorOutput(output: nbformat.IError): CellErrorOutput {
209255
return {
210-
ename: output.ename,
211-
evalue: output.evalue,
256+
ename: '',
257+
evalue: '',
212258
outputKind: vscodeNotebookEnums.CellOutputKind.Error,
213259
traceback: output.traceback
214260
};
215261
}
262+
263+
export function getCellStatusMessageBasedOnFirstErrorOutput(outputs?: nbformat.IOutput[]): string {
264+
if (!Array.isArray(outputs)) {
265+
return '';
266+
}
267+
const errorOutput = (outputs.find((output) => output.output_type === 'error') as unknown) as
268+
| nbformat.IError
269+
| undefined;
270+
if (!errorOutput) {
271+
return '';
272+
}
273+
return `${errorOutput.ename}${errorOutput.evalue ? ': ' : ''}${errorOutput.evalue}`;
274+
}

‎src/test/common.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,9 @@ export class TestEventHandler<T extends void | any = any> implements IDisposable
652652
public get count(): number {
653653
return this.handledEvents.length;
654654
}
655+
public get all(): T[] {
656+
return this.handledEvents;
657+
}
655658
private readonly handler: IDisposable;
656659
// tslint:disable-next-line: no-any
657660
private readonly handledEvents: any[] = [];

‎src/test/datascience/notebook/contentProvider.unit.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ suite('Data Science - NativeNotebook ContentProvider', () => {
7272
editable: true,
7373
executionOrder: 10,
7474
hasExecutionOrder: true,
75-
runState: (vscodeNotebookEnums as any).NotebookCellRunState.Idle,
75+
runState: (vscodeNotebookEnums as any).NotebookCellRunState.Success,
7676
runnable: true,
7777
custom: {
7878
cellId: 'MyCellId1'

‎src/test/datascience/notebook/helper.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { IDisposable } from '../../../client/common/types';
1818
import { noop, swallowExceptions } from '../../../client/common/utils/misc';
1919
import { NotebookContentProvider } from '../../../client/datascience/notebook/contentProvider';
2020
import { ICell, INotebookEditorProvider, INotebookProvider } from '../../../client/datascience/types';
21-
import { waitForCondition } from '../../common';
21+
import { createEventHandler, waitForCondition } from '../../common';
2222
import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants';
2323
import { closeActiveWindows, initialize } from '../../initialize';
2424
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
@@ -252,3 +252,19 @@ export function assertVSCCellHasErrors(cell: NotebookCell) {
252252
assert.equal(cell.metadata.runState, vscodeNotebookEnums.NotebookCellRunState.Error);
253253
return true;
254254
}
255+
export function assertVSCCellHasErrorOutput(cell: NotebookCell) {
256+
assert.ok(
257+
cell.outputs.filter((output) => output.outputKind === vscodeNotebookEnums.CellOutputKind.Error).length,
258+
'No error output in cell'
259+
);
260+
return true;
261+
}
262+
263+
export async function saveActiveNotebook(disposables: IDisposable[]) {
264+
const api = await initialize();
265+
const editorProvider = api.serviceContainer.get<INotebookEditorProvider>(INotebookEditorProvider);
266+
const savedEvent = createEventHandler(editorProvider.activeEditor!.model!, 'changed', disposables);
267+
await commands.executeCommand('workbench.action.files.saveAll');
268+
269+
await waitForCondition(async () => savedEvent.all.some((e) => e.kind === 'save'), 5_000, 'Not saved');
270+
}

‎src/test/datascience/notebook/helpers.unit.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ suite('Data Science - NativeNotebook helpers', () => {
5757
editable: true,
5858
executionOrder: 10,
5959
hasExecutionOrder: true,
60-
runState: vscodeNotebookEnums.NotebookCellRunState.Idle,
60+
runState: vscodeNotebookEnums.NotebookCellRunState.Success,
6161
runnable: true,
6262
custom: {
6363
cellId: 'MyCellId1'
@@ -204,8 +204,8 @@ suite('Data Science - NativeNotebook helpers', () => {
204204
[
205205
{
206206
outputKind: vscodeNotebookEnums.CellOutputKind.Error,
207-
ename: 'Error Name',
208-
evalue: 'Error Value',
207+
ename: '',
208+
evalue: '',
209209
traceback: ['stack1', 'stack2', 'stack3']
210210
}
211211
]

0 commit comments

Comments
 (0)