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

Implementation of the debugger #597

Merged
merged 14 commits into from
Mar 8, 2021
Merged

Conversation

JohanMabille
Copy link
Contributor

@JohanMabille JohanMabille commented Feb 23, 2021

This PR is the initial implementation of the debugger in IPykernel, according to the recent changes in the Jupyter messaging protocol. Although the implementation is not complete, it already contains a minimal set of features that makes the debugger usable, and it is in an acceptable state for a review. @Carreau sorry to ping you so late, I wish I could open this PR sooner.

debug_ipython

1. Requirements

Also this PR is based on #585. We can either rebase once it is merged, or close it and use this PR only.

2. Main changes

  • The control channel runs on a separate thread as of Run control channel in separate thread #585
  • A new ZQMStream has been added for communicating with debugpy. This stream relies on a stream zmq socket (see the ZMQ_STREAM section of zmq_socket API), and is processed by the control thread.
  • Communicating through a stream socket means a message can be split into many tcp frames, or to the contrary, a single tcp frame may contain many messages. Therefore, the kernel must rebuild / split the messages before processing them. This is the role of the DebugpyMessageQueue.
  • DebugpyClient implements a low level client for debugpy. It is responsible for connecting to / disconnecting from debugpy, sending DAP requests, and receiving DAP response and events.
  • Debugger implements the debugger logic.
  • Messages received on the control channel are pushed to an intermediate queue, to avoid starting to process a debug request while the previous one is not finished.
  • A custom caching compiler is passed to the IPython interactive shell. This compiler is responsible for computing a file name mapped with a code cell. This file name must match that computed in the frontend, and is used to dump the code. This is required by debugpy to be able to hit breakpoints when executing code.

3. Debugpy logic

  • Debugpy is started once by executing code on the shell thread. This is required by debugpy to be able to stop on breakpoints. This code is sent by the control thread to the shell socket the first time a debug request wrapping the initialize command is received.

  • Once debugpy runs, you start a debug session by connecting a client socket, and you end it by disconnecting the client socket. Notice that debugpy still runs in the background.

  • The initialization sequence of debugpy is different from that of ptvsd, which was used when implementing the frontend. In order to keep the frontend independent from this logic, the initialization sequence is totally handled by the kernel after it receives an attach command

4. Future improvements

  • After stopping on a breakpoint, the callstack panel shows the whole callstack, including methods form ipykernel. We may want to filter that and keep the methods from the notebook cells only.
  • Similarly, clicking on "next" does not show the next line in the current cell, since it executes the next python statement in ipykernel. We may improve that by looping on sending next requests until we stop on a line from the current notebook cell.
  • Variable inspection when the execution has not stopped on a breakpoint is not implemented yet
  • A lot a logs will be removed. I think they might be helpful for people wnating to play with the implementation and track down what happens in the kernel.

cc @SylvainCorlay @afshin @jasongrout @minrk @ellisonbg @blink1073 @jtpio @martinRenou

@SylvainCorlay
Copy link
Member

cc @Carreau @mlucool, you might be interested in this.

@JohanMabille JohanMabille force-pushed the debugger branch 2 times, most recently from 248fcc2 to f5fa005 Compare March 1, 2021 08:27
@JohanMabille
Copy link
Contributor Author

JohanMabille commented Mar 1, 2021

It looks like we should drop support for Python 3.6 since IPython did it in version 7.17 and IPykernel depends on IPython 7.21 in this PR.

@willingc
Copy link
Member

willingc commented Mar 6, 2021

cc @MSeal @captainsafia @rgbkrk @betatim You may be interested in this.

@SylvainCorlay SylvainCorlay force-pushed the debugger branch 2 times, most recently from f89cc8d to b94fa9f Compare March 8, 2021 10:31
@SylvainCorlay
Copy link
Member

Tests are passing! This is ready for review.

@blink1073 blink1073 added this to the 6.0 milestone Mar 8, 2021
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic!

@blink1073 blink1073 merged commit f96d50d into ipython:master Mar 8, 2021
@JohanMabille JohanMabille deleted the debugger branch March 8, 2021 13:31
@SylvainCorlay
Copy link
Member

cc @ccordoba12 @impact27

@ccordoba12
Copy link
Member

Thanks for the ping @SylvainCorlay, this is great news! Thanks for all the work you and your team have put into this!

I have one question about the implementation: is it necessary to use Debugpy as the debugger backend? The thing is we have our own debugger, which is tightly integrated with Spyder, so we couldn't switch easily to Debugpy.

@SylvainCorlay
Copy link
Member

The front-end implementation in lab only requires that the kernel implements the Jupyter Debugger Procotol, which is essentially the DAP over the control channel. Xeus-python and xeus-robot both implement the protocol (in the case of xeus-robot, it is for the Robotframework language). In ipykernel, it uses debugpy (which is the successor of ptvsd). We could iterate on this to make it an extension point to another implementation. I think a difference with the spyder kernels is that the latter open a new websocket for debugger communication.

@ccordoba12
Copy link
Member

In ipykernel, it uses debugpy (which is the successor of ptvsd). We could iterate on this to make it an extension point to another implementation

Great! If that can be made configurable, we can certainly hook our debugger into it.

I think a difference with the spyder kernels is that the latter open a new websocket for debugger communication.

Right, but our idea is to switch to the control channel (now that it's threaded) to avoid having our own (non-standard) one.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Mar 8, 2021

We really want to serve Spyder in addition to Jupyter frontends, so if you want to get together for a "blackboard" session on this, we are happy to make ourselves available.

@blink1073
Copy link
Contributor

I cut a https://pypi.org/project/ipykernel/6.0.0a0/ so folks can try this out with pip install -U --pre ipykernel

@Carreau
Copy link
Member

Carreau commented Mar 9, 2021

I cut a https://pypi.org/project/ipykernel/6.0.0a0/ so folks can try this out with pip install -U --pre ipykernel

Thanks @blink1073 ! If we move toward a 6.x I'd love to have to also remove ipython_genutils as a dependencies, as it's starting to lead to packaging issues in linux (indirectly because of nose), and we had a couple of request for removing it.

Many use case are python 2/3 compat so should be easy; I've already send a PR with some removal but would appreciate some extra hands.

@blink1073
Copy link
Contributor

Agreed! I added #600 to the 6.0 milestone

@Carreau
Copy link
Member

Carreau commented Mar 9, 2021

hum, the alpha release seem to make the CI of jupyter_client fail. (also I've just push a commit to fix the version number that was ..dev

@SylvainCorlay
Copy link
Member

hum, the alpha release seem to make the CI of jupyter_client fail. (also I've just push a commit to fix the version number that was ..dev

That should be fixed. I think we can tag another pre-release as of the tip of the master branch.

@blink1073
Copy link
Contributor

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

Successfully merging this pull request may close these issues.

6 participants