Add waitUntil for onDidStart event
#16375
Conversation
|
|
||
| show() { | ||
| let execution: IReference<NotebookExecution> | undefined; | ||
| try { |
There was a problem hiding this comment.
@DonJayamanne I wrapped this in a try-catch and added the justification below - do you see any problems with this?
There was a problem hiding this comment.
would it be better to pass in a flag to show to skip the execution check? also, let me know if you'd prefer that I keep this class in the original file (I figured moving it out might be cleaner now that the other file has a dependency on it as well)
There was a problem hiding this comment.
What is the exact failure. This line exists to show a progress indicator against the kernel...
& thats actually a good thing, else there's no way to display progress indicators other than relying on the progress message.
There was a problem hiding this comment.
Here is the trace I get:
Error: duplicate cell execution for vscode-notebook-cell:/Users/paulwang/Downloads/demo.ipynb#W0sZmlsZQ%3D%3D
at ey._createNotebookExecution (file:///Applications/Visual%20Studio%20Code%20-%20Insiders.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:153:26392)
at Object.createNotebookExecution (file:///Applications/Visual%20Studio%20Code%20-%20Insiders.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:153:21721)
at NotebookExecutionReferenceCollection.createReferencedObject (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/standalone/api/kernels/kernelProgressIndicator.ts:38:55)
at NotebookExecutionReferenceCollection.acquire (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/platform/common/utils/lifecycle.ts:321:52)
at _KernelExecutionProgressIndicator.show (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/standalone/api/kernels/kernelProgressIndicator.ts:90:68)
at waitUntil (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/standalone/api/kernels/index.ts:111:61)
at Ph.value (/Users/paulwang/.vscode-insiders/extensions/vscode-test-person.helloworld-sample-0.0.1/src/extension.ts:33:5)
at P.B (file:///Applications/Visual%20Studio%20Code%20-%20Insiders.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:27:2373)
at P.fire (file:///Applications/Visual%20Studio%20Code%20-%20Insiders.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:27:2591)
at /Users/paulwang/Desktop/tmp/vscode-jupyter/src/standalone/api/kernels/index.ts:100:50
at AsyncEmitter.fireAsync (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/platform/common/utils/events.ts:104:17)
at KernelProvider.<anonymous> (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/kernels/kernelProvider.node.ts:101:64)
at AsyncEmitter.fireAsync (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/platform/common/utils/events.ts:104:17)
at /Users/paulwang/Desktop/tmp/vscode-jupyter/src/kernels/kernel.ts:272:46
at Function.wrapKernelMethodImpl (/Users/paulwang/Desktop/tmp/vscode-jupyter/src/notebooks/controllers/kernelConnector.ts:481:21) (at console.<anonymous> (file:///Applications/Visual%20Studio%20Code%20-%20Insiders.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:156:26594))
If I understood correctly, we probably don't want to create an execution reference when the waitUntil is called since it's possible that no kernel execution actually happens there, but we can let that happen when execute is called (we're just borrowing the messaging logic here).
pwang347
changed the title
waitUntil for onDidStart eventwaitUntil for onDidStart event
|
|
||
| show() { | ||
| let execution: IReference<NotebookExecution> | undefined; | ||
| try { |
There was a problem hiding this comment.
What is the exact failure. This line exists to show a progress indicator against the kernel...
& thats actually a good thing, else there's no way to display progress indicators other than relying on the progress message.
For more context, see #16387