don't try to prewarm an execution service #13377
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #13377 +/- ## ========================================== - Coverage 59.71% 59.71% -0.01% ========================================== Files 670 670 Lines 37338 37346 +8 Branches 5307 5312 +5 ========================================== + Hits 22298 22301 +3 Misses 13905 13905 - Partials 1135 1140 +5
Continue to review full report at Codecov.
|
| // The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2. | ||
| // Use a check for the daemon.start function here before we call it. | ||
| if (!daemon.start) { | ||
| if (!('start' in daemon)) { |
There was a problem hiding this comment.
This is where I'd made the previous fix to 2.7 non-daemon launching. But that funky type coming back from the daemon creation was still causing issues in other locations (such as with prewarm) so I converted the deamon create to return the actual type union that it creates and then make sure to handle the cases that might be different for a daemon versus an execution service. I should have changed it the first time around 😞 .
IanMatthewHuff requested review from DavidKutu, DonJayamanne, greazer, joyceerhl and rchiodo 
| ); | ||
| } | ||
| | ||
| public async createDaemon<T extends IPythonDaemonExecutionService | IDisposable>( |
There was a problem hiding this comment.
Would it make more sense to put this 'or' on the 'extends'? Or does that not work?
There was a problem hiding this comment.
I was thinking no, since the <> part I see as the "request" for what the user wants. Honestly IPythonDaemonExecutionService is almost the same as IPythonExecutionService so it could go together, but I felt like the distinction was important here between what users are actually request (some type of IPythonDaemonExecutionService) and the fact that if we can't create one then we just return an IPythonExecutionService.
| Kudos, SonarCloud Quality Gate passed!
|
| 27 failures were just linter test failures so pushing. |
3 participants
For #13258
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).