[v2] Control when a Cell is executing #279

[MarcosVn]

• Reimplements the proposed solution for the described scenario in Control when a Cell is executing #279 issue (after Kernel state for Cell and Output #282 changes)

• Added the isExecuting attribute in CellState to track/control when a cell is executing;

• Also added isAnyCellExecuting to the cells map to control if there is any execution in progress (or if all cells are idle);

• Improved related comments;

• Updated CellAdapter to update the isExecuting state in new _execute generic function;

• Added the CellExecuteControl.tsx example.

[MarcosVn]

Hey, @echarles - how are you?

I submitted a new version for #279 in this PR and I think it's much simpler with the changes!

I have a single issue I'd like to align on.

In the _execute() function of CellAdapter, it seems that when the executed code causes an exception in Python, the promise (await kernelMessagePromise)!; on our side is never resolved, and nothing is captured. In my case, this is a problem because the isExecuting state state will never be set to false in this scenario.

Here is some evidence of what I mentioned:

How do you think we could resolve this?

[echarles]

Awesome!

How do you think we could resolve this?

In this code block,

jupyter-ui/packages/react/src/jupyter/kernel/KernelExecutor.ts

Lines 216 to 244 in ec9818c

	case 'error':
	this._outputs.push(message.content as IStream);
	this._outputsChanged.emit(this._outputs);
	this._model.add(output);
	this._modelChanged.emit(this._model);
	if (this._stopOnError) {
	kernelsStore.getState().setExecutionPhase(this._kernelConnection.id, ExecutionPhase.completed_with_error);
	}
	break;
	case 'clear_output':
	const wait = (message as IClearOutputMsg).content.wait;
	this._model.clear(wait);
	break;
	case 'update_display_data':
	this._outputs.push(message.content as IDisplayUpdate);
	this._outputsChanged.emit(this._outputs);
	// FIXME this needs more advanced analysis see OutputArea
	this._model.add(output);
	this._modelChanged.emit(this._model);
	break;
	case 'status':
	const executionState = (message.content as any).execution_state as KernelMessage.Status;
	const connectionStatus = this._kernelConnection.connectionStatus;
	const kernelState = toKernelState(connectionStatus!, executionState);
	this._kernelState.setExecutionState(this._kernelConnection.id, kernelState);
	break;
	default:
	break;
	}

, at the end of the error case, I would add this._executed.resolve(this._model); (no guarantee... :)

[MarcosVn]

@echarles

at the end of the error case, I would add this._executed.resolve(this._model); (no guarantee... :)

I tested it here and unfortunately it didn't work.
However, debugging, I believe I identified the problem, which is in:

// const { ename, evalue, traceback } = (
//   content as any as KernelMessage.IErrorMsg
// ).content;

I confess that I didn't fully understand this destructuring, but today it fails, which causes the _onReply function to never reject the Promise - in the next line.

I changed this block to what my typing indicated here as correct and it worked, evidence below:

Let me know if you agree with the change.

FYI: @fcollonval.

[echarles]

LGTM Thx @MarcosVn

[echarles]

Merged and 0.17.0 released. Thank you so much for this @MarcosVn