Conversation
|
For now the This means that the handler triggers the restore flow before the restart, and the |
6740196 to
de78767
Compare
|
This should also handle kernels being killed and automatically restarted:
|
f8aa3be to
4fcb68c
Compare
| // setup handler when the status of the kernel changes (restart) | ||
| // TODO: is there a better way to handle restarts? | ||
| let restarted = false; | ||
| const statusChanged = async () => { | ||
| // wait for the first `idle` status after a restart | ||
| if (restarted && client.status === 'idle') { | ||
| restarted = false; | ||
| return updateHandler(); | ||
| } | ||
| // handle `starting`, `restarting` and `autorestarting` | ||
| if (client.status.endsWith('starting')) { | ||
| restarted = true; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Not super satisfied with this logic.
It seems to be a bit tricky to really detect a kernel restart.
There was a problem hiding this comment.
An alternative would be to schedule the call to updateHandler with a small time delay.
da8a7b0 to
976a413
Compare
|
Rebased. |
|
cc @afshin @KsavinN if you want to try it. This should handle most of the restart scenarios, for example a click on the restart button or a kernel killed from the command line. However the logic to properly detect a kernel restart is a bit convoluted, and it would be great to have a second look at it later on if we decide to keep it like this for now. |
976a413 to
2dafa81
Compare
afshin
left a comment
There was a problem hiding this comment.
Awesome, thanks! There are still kernel restart quirks but this is strictly an improvement so let's leave those for another PR.
|
Yes, I think we'll need to revisit the logic a bit. Thanks for looking at it 👍 |
Fixes #76, replaces #276.