-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display kernel failure msg in last executed cell #9587
Conversation
// Also when kernels die, we don't restart automatically with raw kernels, | ||
// We should'nt do the same with jupyter (else startup code will not run). | ||
promisedArgs.push(Promise.resolve('--KernelManager.autorestart=False')); | ||
if (this.context.extensionMode === ExtensionMode.Test) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting this change, else this breaks jupyter.
I.e. if a kernel dies, then we never get a response back for anything, we don't know whether the died or not.
With auto start enabled, jupyter will at a minimum re-start the kernel and we know that it was auto-restarted as the kernel died. Note: Jupyter auto restarts when kernels die.
Hence this change is required (& it was wrong to add)
// & this is very slow (we dont want users to wait because of kernel failures). | ||
// Also when kernels die, we don't restart automatically with raw kernels, | ||
// We should'nt do the same with jupyter (else startup code will not run). | ||
promisedArgs.push(Promise.resolve('--KernelManager.autorestart=False')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting this change, else this breaks jupyter.
I.e. if a kernel dies, then we never get a response back for anything, we don't know whether the died or not.
With auto start enabled, jupyter will at a minimum re-start the kernel and we know that it was auto-restarted as the kernel died. Note: Jupyter auto restarts when kernels die.
Hence this change is required (& it was wrong to add)
Codecov Report
@@ Coverage Diff @@
## main #9587 +/- ##
=====================================
Coverage 72% 73%
=====================================
Files 189 193 +4
Lines 8563 8451 -112
Branches 1261 1238 -23
=====================================
- Hits 6232 6171 -61
+ Misses 1848 1807 -41
+ Partials 483 473 -10
|
private onDidStartKernel(kernel: IKernel) { | ||
this.kernelsStartedSuccessfully.add(kernel); | ||
} | ||
private onKernelStatusChanged({ kernel }: { status: KernelMessage.Status; kernel: IKernel }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remvoed code form here as:
- progress for restarting kernels has nothing to do with command handler
- DIsplaying warnings about dead kernels has nothing to do with command handler
As both of the above are two distinct features, i've moved into 2 separate files.
Fixes #9375