Produce kernelinfo on kernel ready #2834
Conversation
|
Fixes #2831 |
maybe .. |
| "event" => { | ||
| "kernelInfo" => $kernelInfo, | ||
| "kernelInfos" => $kernelInfo |
There was a problem hiding this comment.
@brettfo am I doing this right?
colombod
force-pushed
the
kernel_info
branch
2 times, most recently
from
d843531
to
e18ca77
Compare
| .Take(1) | ||
| .Subscribe(e => | ||
| { | ||
| kernelReadyReceived = true; | ||
| _kernelReady = e; |
There was a problem hiding this comment.
Since this is all within the same method, could we use a local instead of the field for kernelReady?
We already cache all kernel infos that we have seen so far in _remoteKernelInfos. So, it feels a bit strange that at least some of those kernel infos would also be avbailable via the _kernelReady field. Plus the filed is not used outside this method anyway.
There was a problem hiding this comment.
(Looks like making the above change would also solve the nullability error that caused the build break in CI)
There was a problem hiding this comment.
but the connector can be reused that is why is stored, you will see that event only when the process is started
There was a problem hiding this comment.
All kernel infos are already avaialble in _remoteKernelInfos, so you could just fetch it from there (like we do inside CreateProxyKernelAsync below), could you not?
There was a problem hiding this comment.
In other words, I think we can use a local for the first time case when the process is started (since we need to wait until the kernelReady is received in that case). For all other cases where process is already present but connector is reused, we can use the previously cached kernelInfos present inside _remoteKernelInfos.
kernel ready contains kernelInfo and is used to setup proxys at connection time Update src/dotnet-interactive/Connection/StdIoKernelConnector.cs Co-authored-by: Shyam N <shyamnamboodiripad@users.noreply.github.com> Update src/dotnet-interactive/Connection/StdIoKernelConnector.cs Co-authored-by: Shyam N <shyamnamboodiripad@users.noreply.github.com> nullability check for null
No description provided.