When the remote server is actually 'localhost' we can behave better ... #8527
Conversation
| if (kernel.kind !== 'connectToLiveKernel' && kernel.kernelSpec && kernel.interpreter) { | ||
| if ( | ||
| kernel.kind !== 'connectToLiveKernel' && | ||
| kernel.kind != 'startUsingRemoteKernelSpec' && |
There was a problem hiding this comment.
Yep. Thanks. Which also prompted me to lookup the difference. === does not do any type manipulation and just compares the values. == will convert the values to the same types. So I guess === is faster.
| export function isLocalHostConnection(kernelConnection: KernelConnectionMetadata): boolean { | ||
| if (kernelConnection.kind === 'connectToLiveKernel' || kernelConnection.kind === 'startUsingRemoteKernelSpec') { | ||
| const parsed = new url.URL(kernelConnection.baseUrl); | ||
| return parsed.hostname.toLocaleLowerCase() === 'localhost' || parsed.host === '127.0.0.1'; |
There was a problem hiding this comment.
I was going to ask if localhost is more configurable, and I believe it is. But I think that we only have to worry about how Jupyter starts it, so this is fine I believe.
There was a problem hiding this comment.
Was going to ask if we needed to check for an npm module or something for a more comprehensive loopback check, but I don't think we need to.
Codecov Report
@@ Coverage Diff @@
## main #8527 +/- ##
=======================================
- Coverage 71% 56% -16%
=======================================
Files 378 378
Lines 23985 24008 +23
Branches 3692 3701 +9
=======================================
- Hits 17266 13477 -3789
- Misses 5227 9370 +4143
+ Partials 1492 1161 -331
|
| kernelConnection.kind === 'connectToLiveKernel' || | ||
| kernelConnection.kind === 'startUsingRemoteKernelSpec' || | ||
| !kernelConnection.interpreter | ||
| ) { |
| kernel.kind !== 'connectToLiveKernel' && | ||
| kernel.interpreter && | ||
| kernel.kind !== 'startUsingRemoteKernelSpec' | ||
| ) { |
There was a problem hiding this comment.
Shoot even more, yup, i think i know what would have happened...
Thanks
There was a problem hiding this comment.
No these are my fault. A remote kernel couldn't have an interpreter until my change. So your old if statement would have worked.
| @@ -344,7 +344,7 @@ export async function waitForKernelToGetAutoSelected(expectedLanguage?: string, | |||
| ); | |||
| return preferred != undefined; | |||
| }, | |||
| 3_000, | |||
| 30_000, | |||
There was a problem hiding this comment.
This timeout was failing because now remote connections try to match interpreters. Getting intepreter details was taking longer than 3 seconds.
There was a problem hiding this comment.
This is what was causing my test failures AFAIK. Hopefully next run completes fully.
Fixes #8285
Fixes #1551
Special case some scenarios to act like local when the server is 'localhost' or '127.0.0.1'
Also fix remote intellisense to work in those scenarios or in true remote.