Execution API

[roblourens]

#106744
#106741
#105847

cc @rebornix @jrieken

[roblourens]

Noticed one issue, if extensions can initiate an execution with createNotebookCellExecution without executeCells having been called, then there is no way for that execution to be canceled. Rearranged things so that the execution object (a pending object at first) is passed to executeCells, and the cancel token is part of the execution object.

This also loses the relationship between a "job" of multiple cells executing and one cancellation token for the job. But I'm not sure we want that anyway. VS Code can't have any concept of multiple cells executing together unless the execution object can be created for multiple cells together.

[jrieken]

I believe it would be for LiveShare but for now we can pretend that we don't know that and not have this

[roblourens]

I was thinking that liveshare takes over the kernel or something but I guess I don't know exactly how it works

[roblourens]

I guess it probably doesn't do that on the host and it just has to observe

[jrieken]

Noticed one issue, if extensions can initiate an execution with createNotebookCellExecution without executeCells having been called, then there is no way for that execution to be canceled.

Wouldn't interrupt still work?

[roblourens]

Yes, just the cancellation token case where you care which cell I clicked stop on would not work

[roblourens]

Tried to implement some working form of the NotebookCellExecution and I'm going in circles on the exact behavior between the executeCells API and createNotebookCellExecution.

I think some things will be clear with more implementation. One thing I've decided is that executeCells can't pass a cancellation token or execution object to the kernel, partly because executeCells is a call for multiple cells, partly because a token doesn't make sense in a void method - what is the lifetime? We can't create a cancel token until the kernel puts the cell in the pending state by calling the create method.

Ideally we would be able to say that a cell is pending the instant the run button is clicked without a roundtrip to the kernel. This is probably still possible but it's much simpler to ignore it for now. Since the kernel is the real source of truth about whether a cell is executing, it's better to say that it's not pending until the kernel says it is, and then vscode probably does not do any validation on whether it's ok to call create. Avoids issues about what happens if the user clicks the run button at the exact same time as the kernel independently says that a cell has started executing.

We could also make createNotebookCellExecution only accessible to a kernel, if we wanted to.

I think the info about the run state will be internally moved out of metadata to some other object that is edited in the same way with workspace edits and cell edits.

[roblourens]

I've had an idea for a different shape for this API which would make some things simpler, which would look something like this:

interface NotebookKernel {
    executeCells(document: NotebookDocument, cells: NotebookCellRange[], executionObject: NotebookCellExecution): Promise<SomeResult>;
}

// Kernel can call executeCommand('executeCell', { cell }); to trigger an execution

Peng and I discussed this. It's more in a direction similar to what we have today but keeps the execution object concept. Essentially the execution object would only be created by vscode and passed via the executeCells API, and if a kernel needs to trigger an execution, it would call some API or use executeCommand to do that, and vscode would call executeCells shortly after.

Pros are that it would give more of a "single direction" of data flow between vscode and the kernel. The run button clicking case and the kernel-initiated execution case would work exactly the same way. The executeCells API really feels like it should return a promise that actually follows the lifetime of the execution process, and this enables that.

Cons are that this inverses the relationship between vscode and the kernel. If the kernel really is the source of truth about whether a cell is executing, then it's wrong to have an API model where the kernel has to request that vscode start executing a cell. Really it has to be vscode that requests that the kernel start an execution, and the kernel telling vscode that it is executing. Tbh I'm having trouble telling whether this is just a philosophical problem or a real one. But I think that this solution has to lead to vscode being the one to maintain the execution state of a cell, rather than the kernel.

Also the shape above is not correct, we would need to figure out how to pass N execution objects for cells in the range, which is weird, or change the API to execute a single cell, which is also weird.

[rebornix]

but keeps the execution object concept

After exploring different options, I do prefer the execution object concept as it's a good abstraction of the challenge kernels are dealing handling. As long as we support multiple cell selection in the UI, the first thing extension authors need to think about is how to handle execution when more than one cell is selected. We as extension authors would need to answer a couple of questions

• does the kernel support execution in parallel?
  • if it does, how to update their execution state (success/fail, order, ellapsed time) separately?
  • if not, how to turn selected cells into running state one by one (then the ellapsed time calculated for each cell can be correct)?

The execution object concept is a proper way to do cell level execution state manipulation. Relying executeCells to return the result won't work as only the kernel knows how cells are being executed.

to_be_continued

[jrieken]

it would call some API or use executeCommand to do that, and vscode would call executeCells shortly after.

When a kernel is becoming the selected/active kernel, e.g when calling resolveKernel, we could pass a factory that allows to create execution objects. That factory would only be valid as long as the kernel is selected. This would prevent the awkward self-invoking of executeCell and still put us in control

[rebornix]

Notes to myself since I'm going in circles during discussion, there are a couple of scenarios we want to support and also a few No-gos

• Scenarios
  • Users select multiple cells, execute, cells get executed one by one
    • User can cancel individual cell execution, next cell still gets executed
  • Users select multiple cells, execute, cells get executed at the same time
    • Users can cancel a cell execution, other cells are not affected
  • Users run a cell, and the kernel runs dependent cells first
    • Users cancel a dependent cell's execution, all following cell execution are aborted (from pending state to abort/finish/error)
  • Users request to cancel execution on all running cells
  • Live Share guest joined, host needs to share the execution state of current notebook to guest. Live Share would need to know the execution state of cells in the editor
• No Go
  • Rely on the executeCell return result for the cell execution state
    • It doesn't work for multi cell selection. Kernels decide how cells would be turned into what state (running, pending, finished, etc)
    • The return result might only be used as indicator that execution requests are sent successfully to the kernel

[roblourens]

When a kernel is becoming the selected/active kernel, e.g when calling resolveKernel, we could pass a factory that allows to create execution objects. That factory would only be valid as long as the kernel is selected. This would prevent the awkward self-invoking of executeCell and still put us in control

This is more along the lines of the original suggestion (that is currently implemented in this PR) where the kernel creates execution objects. I like being able to restrict the execution object creation to the active kernel though. There should be no race or other but when switching kernels where a kernel might be confused about whether it is active, vscode has to be able to enforce this.

[jrieken]

Unsure if this should be prohibited and iff so you must also check on the renderer because there are N extension hosts

[roblourens]

If only the active kernel is allowed to have executions, then I think this is sufficient. That isn't implemented but I think we want that.

[roblourens]

Remaining TODOs

• Implement showing cancel button when any cell is being executed, for a kernel that implements interrupt
• Implement partial metadata edit