Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugger always disabled with ipykernel 6.4.0? #770

Closed
jtpio opened this issue Sep 9, 2021 · 6 comments · Fixed by #773
Closed

Debugger always disabled with ipykernel 6.4.0? #770

jtpio opened this issue Sep 9, 2021 · 6 comments · Fixed by #773

Comments

@jtpio
Copy link

jtpio commented Sep 9, 2021

Using the latest 6.4.0 version of ipykernel, it looks like the debugger is always disabled.

This was first noticed on Binder using this gist: https://gist.github.com/jtpio/6ce26381703355e0ef1da4af742b7f72

But can be reproduced locally:

mamba create -n tmp python -y
conda activate tmp

# install the latest versions
python -m pip install -U ipykernel debugpy jupyterlab

# start JupyterLab
jupyter lab

The spec seems to be installed with debugger: false on disk:

$ cat ${CONDA_PREFIX}/share/jupyter/kernels/python3/kernel.json
{
 "argv": [
  "python",
  "-m",
  "ipykernel_launcher",
  "-f",
  "{connection_file}"
 ],
 "display_name": "Python 3 (ipykernel)",
 "language": "python",
 "metadata": {
  "debugger": false
 }
}

And the kernelspecs return debugger: false for ipykernel at startup:

image

Dependencies:

$ pip list | grep -E "debugpy|ipykernel"
debugpy             1.4.1
ipykernel           6.4.0
@jtpio
Copy link
Author

jtpio commented Sep 9, 2021

Looks like this might be related to #712 and #767

@fcollonval
Copy link
Contributor

fcollonval commented Sep 10, 2021

The kernelspec is not dynamic; it is served from a JSON file shipped within the wheel. So the solution implemented in #767 does not work.

I would recommend reverting #767 for now as removing debugpy also break environment consistency as debugpy is still listed as mandatory dependency: #767 (comment)

@JohanMabille
Copy link
Contributor

JohanMabille commented Sep 10, 2021

There is a field in the kernel_info reply that indicates whether a kernel supports debugging. This seems redundant with the kernelspec but it has the advantage of being dynamic.

Is there a reason for using the kernelspec instead of the kernel_info reply field in JupyterLab? Otherwise, switching to the kernel_info reply could fix this issue (although I guess it requires an additinoal patch in ipykernel).

@jtpio
Copy link
Author

jtpio commented Sep 10, 2021

Is there a reason for using the kernelspec instead of the kernel_info reply field in JupyterLab?

Yes. The debugger extension used to use the field from the kernel_info_reply, but this led to issues for example when reloading the page. JupyterLab would attempt to send new kernel_info_request messages on startup while the kernel would be blocked, for example when blocked at a breakpoint. More info in jupyterlab/debugger#457 and jupyterlab/debugger#220.

@JohanMabille
Copy link
Contributor

JohanMabille commented Sep 10, 2021

Thanks @jtpio!

So a possible solution to this issue could be:

  • change the protocol to send the kernel_info request on the control channel to avoid freezing page refresh when the code hit a breakpoint
  • rely on this field instead of the kernelspec for detecting debugging capacity.

OR

the kernelspec always sets the field in the kernelspec to True, and it's up to package maintainers who want to remove dependency on debugpy to overwrite this field when they generate the package.

@minrk
Copy link
Member

minrk commented Sep 10, 2021

#773 should fix the wheel metadata

I do think getting this (and all kernel capability detection) from kernel_info_reply is a better practice in general, since it's more reliably accurate than something set at build-time.

To solve the 'kernel busy on reload' issue, maybe the server should cache a first-time kernel info reply and serve that somehow without re-pinging kernels?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants