Wait for ready on raw session start and respect timeout on kernel change #11768
Conversation
…ct timeout and retry
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -131,7 +138,13 @@ export class RawJupyterSession extends BaseJupyterSession { | |||
|
|
|||
| this.outputChannel.appendLine(localize.DataScience.kernelStarted().format(kernel.display_name || kernel.name)); | |||
|
|
|||
| return this.startRawSession(kernel, interpreter); | |||
| const newSession = await waitForPromise(this.startRawSession(kernel, interpreter), timeoutMS); | |||
There was a problem hiding this comment.
Needed to respect the restart timeout that was getting passed in here.
| @@ -209,6 +222,9 @@ export class RawJupyterSession extends BaseJupyterSession { | |||
| // Create our raw session, it will own the process lifetime | |||
| const result = new RawSession(process); | |||
|
|
|||
| // When our kernel connects and gets a status message it triggers the ready promise | |||
| await result.kernel.ready; | |||
There was a problem hiding this comment.
This was actually the more interesting part of the fix. When we were having those initial windows launch fixes I examined this promise, but it didn't actually help with the case where the ports were not open yet. But we do need it here. Ready waits for that initial kernel_status message to be returned. So without this here we might have a bad kernel that can't actually start (regardless of if the process exits or not) but we would leak outside of the start timeout check. So the start or switch timeout check would not trigger, but we'd be stuck with a hung kernel.
IanMatthewHuff
requested review from
DonJayamanne,
rchiodo,
DavidKutu and
greazer
Codecov Report
@@ Coverage Diff @@
## master #11768 +/- ##
==========================================
- Coverage 60.63% 60.40% -0.24%
==========================================
Files 628 630 +2
Lines 33943 34122 +179
Branches 4782 4797 +15
==========================================
+ Hits 20581 20611 +30
- Misses 12365 12512 +147
- Partials 997 999 +2
Continue to review full report at Codecov.
|
For #11752
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).