Use control comm target in base Manager

[martinRenou]

Port of voila-dashboards/voila#766 for JupyterLab and classic Jupyter Notebook

Use the jupyter.widget.control comm target for fetching widget states in the JupyterLab manager and the widgetsnbextension, this is used e.g. when refreshing the JupyterLab page on an already running kernel that contains widgets.

It will also be used by Voila in voila-dashboards/voila#846.

I would really like this to be considered for the coming 8.0.0 release. It would also be nice to backport it to the 7.x branch if we keep making releases from there.

User facing changes

This improves the performances when refreshing the JupyterLab page or when connecting to an already existing kernel that contains widgets.

[github-actions]

👈 Launch a binder notebook on branch martinRenou/ipywidgets/use_control_comm_target

[martinRenou]

Making this WIP for now as I'll be doing the same for the widgetsnbextension

[martinRenou]

@meeseeksbot backport to 7.x

[martinRenou]

meeseeksdev backport to 7.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 7.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -m1 9d36d26e64b15f67808480c79de6f9d10d30a3b8

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #3335: Use control comm target in LabManager'

4. Push to a named branch:

git push YOURFORK 7.x:auto-backport-of-pr-3335-on-7.x

5. Create a PR against branch 7.x, I would have named this PR:

"Backport PR #3335 on branch 7.x (Use control comm target in LabManager)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

[martinRenou]

Comment: could this be part of the base manager?

[martinRenou]

We could probably do this to check for the target:

const reply = await kernel.requestCommInfo({target_name: this.comm_target_name});

The documentation is not entirely clear on the behavior of this when the target is no registered (to be improved):

https://jupyter-client.readthedocs.io/en/stable/messaging.html#comm-info

[martinRenou]

Question: what happens if I reduce the tornado web socket message size limit to something very small? Does it fail? How?

[jasongrout]

Question: what happens if I reduce the tornado web socket message size limit to something very small? Does it fail? How?

My guess is that the message will just never come back from the kernel. I think it would be good to have a timeout on the request that will fall back to the older method. Separately, I think it would be good to have a check in the server zmq/websocket bridge to see if a comm message is too big, and if it is, send a comm_close message both ways (to kernel and frontend) with an error status, indicating the comm channel had an error.

[martinRenou]

We could probably do this to check for the target:

const reply = await kernel.requestCommInfo({target_name: this.comm_target_name});

Unfortunately this doesn't work. The kernel answers to the request with {"status": "ok"} even when there is no target handler registered.

[martinRenou]

I updated the PR this morning:

• Moved the loadFromKernel logic into the base manager, as suggested by @vidartf
• Fallback to the old implementation if the control target is not responding in time (4 seconds), as suggested by @jasongrout

Unfortunately we can't use the commInfo request to know if the target is registered kernel-side. See #3335 (comment)

[martinRenou]

Unfortunately this doesn't work. The kernel answers to the request with {"status": "ok"} even when there is no target handler registered.

Check again: what are the exact response messages when

• the comm target is not registered
• the comm target is registered

[martinRenou]

So it's annoying, requesting the comm info will provide the same answer whether the target is registered or not:

{
    'comms': {},
    'status': 'ok'
}

No target registered:

Target registered:

So using this request will not give us the information about the comm target.

[martinRenou]

Also adding a link to the comm_info_reply implementation in ipykernel: https://github.com/ipython/ipykernel/blob/0ab288fe42f3155c7c7d9257c9be8cf093d175e0/ipykernel/kernelbase.py#L771-L787

[martinRenou]

The xeus implementation looks similar https://github.com/jupyter-xeus/xeus/blob/eab12033ed8dc0202dfba733446dcf7ced10fca1/src/xkernel_core.cpp#L303-L322 (reply status is always "ok" and the comm object is either empty or contains comms)

[jasongrout]

Looks good! Two minor comments to address inline, and then +1 from me to merge. Thanks!

[martinRenou]

Thanks for your review @jasongrout