Nudge kernel with info request until we receive IOPub messages

[SylvainCorlay]

Porting jupyter/notebook#5908 to jupyter_server.

see original discussion in jupyter/jupyter_client#593

Nudge kernel with info request upon opening websocket and upon restart until some IOPub message is received.

[codecov-io]

Codecov Report

Merging #361 (39eefe7) into master (6fef2a8) will decrease coverage by 1.26%.
The diff coverage is 18.42%.

@@            Coverage Diff             @@
##           master     #361      +/-   ##
==========================================
- Coverage   68.24%   66.98%   -1.27%     
==========================================
  Files          56       56              
  Lines        6066     6134      +68     
  Branches      794      804      +10     
==========================================
- Hits         4140     4109      -31     
- Misses       1693     1784      +91     
- Partials      233      241       +8     

Impacted Files	Coverage Δ
jupyter_server/services/kernels/handlers.py	45.74% <18.42%> (-6.52%)	⬇️
jupyter_server/__init__.py	61.53% <0.00%> (-38.47%)	⬇️
jupyter_server/terminal/api_handlers.py	44.11% <0.00%> (-29.42%)	⬇️
jupyter_server/terminal/handlers.py	57.89% <0.00%> (-26.32%)	⬇️
jupyter_server/terminal/__init__.py	76.00% <0.00%> (-16.00%)	⬇️
jupyter_server/_sysinfo.py	81.81% <0.00%> (-12.13%)	⬇️
jupyter_server/utils.py	58.79% <0.00%> (-12.07%)	⬇️
jupyter_server/services/contents/fileio.py	72.25% <0.00%> (-2.10%)	⬇️
jupyter_server/services/contents/filemanager.py	65.53% <0.00%> (-1.71%)	⬇️
jupyter_server/services/kernels/kernelmanager.py	61.90% <0.00%> (-0.80%)	⬇️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fef2a8...39eefe7. Read the comment docs.

[SylvainCorlay]

Now that jupyter/notebook#5908 has been merged, I think that this can get in!

cc @minrk

[Zsailer]

Thanks, @SylvainCorlay. Took for a spin (with minrk/ipykernel@3fdb063 from @minrk) and it works beautifully!

[SylvainCorlay]

I think this is ready. Please feel free to merge! This issue has been bothering us for quite some time!

[Zsailer]

Done and released! jupyter_server 1.1.2

[kevin-bates]

Hi @SylvainCorlay - I'm sorry for posting this against a closed PR, but I figured it's got the most context relative to what I'm seeing across restarts when kernels have had their ports changed.

I'd like to get a better understanding of these lines of code in nudge():

jupyter_server/jupyter_server/services/kernels/handlers.py

Lines 152 to 156 in a0f98a0

	# Use a transient shell channel to prevent leaking
	# shell responses to the front-end.
	shell_channel = kernel.connect_shell()
	# The IOPub used by the client, whose subscriptions we are verifying.
	iopub_channel = self.channels["iopub"]

which are really artifacts (for the purpose of this discussion) of these lines of code:

jupyter_server/jupyter_server/services/kernels/handlers.py

Lines 368 to 372 in a0f98a0

	# on new connections, flush the message buffer
	buffer_info = km.get_buffer(kernel_id, self.session_key)
	if buffer_info and buffer_info['session_key'] == self.session_key:
	self.log.info("Restoring connection for %s", self.session_key)
	self.channels = buffer_info['channels']

1. Do you know why kernel.shell_port is considered transient? The shell_port/channel on the kernel manager is tied to the current shell port so I'm not following what is transient about it.
2. Since nudge() needs to monitor shell and iopub, and self.channels['iopub'] reflects the iopub_port of the previous kernel manager instance (prior to restart), then when it detects that messages have been buffered, nudge() eventually times out in cases where ports change on a restart. It seems like kernel.iopub_channel is the correct port to tie nudge() to regardless of whether messages have been buffered or not because it will always be the current iopub port/channel.

I also find that the connection counts (via self._kernel_connections) are one higher in Lab than Notebook. I suspect this is because Lab uses a WS in its main file-browser pane - so closing the tab of the notebook window only decrements the connections to 1 - so buffering doesn't take place (it only kicks in when connections are zero). Whereas Notebook doesn't open a WS in its primary window, therefore, simply closing the tab will decrement connections to zero and buffering will start. This difference manifests the nudge timeout issue more easily when restarts occur within Notebook (and ports have changed) because the shutdown-reply sequence will get buffered due to the connection count going to zero when the kernel is shutdown (prior to its (re)start).

I know this is all intricately tied to the buffer replay stuff, but I think self.channels['iopub'] should be changed kernel.connect_iopub() and we should be unconditionally calling self.create_stream() - like we do when buffered messages are not detected.
(edit: calling self.create_steam() will reset self.channels which are necessary for replay to deserialize the buffered messages. I think the issue with buffer replay is its too closely wrapped in the handler instance (self).)

Does this make sense?