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

Implemented variable inspection when the debugger has started #10025

Merged
merged 3 commits into from
Apr 9, 2021
Merged

Implemented variable inspection when the debugger has started #10025

merged 3 commits into from
Apr 9, 2021

Conversation

JohanMabille
Copy link
Member

@JohanMabille JohanMabille commented Mar 30, 2021

References

#9114

Code changes

This implementation is specific to the debugger package. The frontned now sends an inspectVariables request to the backend when the debugger has started and the code has not hit a breakpoint.

This can change can be tested with xeus-python 0.12.2 or with this branch of ipykernel.

There are still some issues (thus the WIP status):

  • infinite loop of sending inspectVariables when the user restars the kernel
  • defining variables such as dict breaks the behavior (nothing is displayed anymore, but the backend doe snot emit any error).

User-facing changes

When the debugger has started, the user can now see the variables that have been defined so far in the variables panel of the debugger sidebar.

inspect_variables

Backwards-incompatible changes

The kernel must support the inspectVariable request. This means upgrading to xeus-python 0.12.3 in the tests (this can be done in another PR). There is no impact on ipykernel since the debugger will be supported in the next release only, and the support for inspectVariables has been implemented.

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@JohanMabille JohanMabille marked this pull request as draft March 30, 2021 16:39
@jtpio
Copy link
Member

jtpio commented Mar 30, 2021

Thanks!

This can change can be tested with xeus-python 0.12.2

Does it mean we should also consider updating the version of xeus-python on CI here (used for the tests)?

# The debugger tests require a kernel that supports debugging
if [[ $GROUP == js-debugger ]]; then
pip install xeus-python">=0.9.0,<0.10.0"
fi

Or is the new 0.12.2 mostly about adding support for filtering variables for the inspectVariables request, and thus is a nice-to-have but not a strict requirement?

@JohanMabille
Copy link
Member Author

Second option, xeus-python 0.12.2 is mostly about adding support for the inspectVariables requesr.

@JohanMabille JohanMabille changed the title WIP - Implemented variable inspection when the debugger has started Implemented variable inspection when the debugger has started Apr 7, 2021
@JohanMabille JohanMabille marked this pull request as ready for review April 7, 2021 09:43
@JohanMabille
Copy link
Member Author

JohanMabille commented Apr 7, 2021

xeus-python 0.12.3 is not available yet for all platforms on conda-forge, thus the failure.

EDIT: except that we install xeus-python from pypi ...

@jtpio
Copy link
Member

jtpio commented Apr 7, 2021

Looks like the new wheels will have to be published to PyPI, since xeus-python is installed via pip:

# The debugger tests require a kernel that supports debugging
if [[ $GROUP == js-debugger ]]; then
pip install xeus-python">=0.9.0,<0.10.0"
fi

In the meantime the PR can be tested on Binder: https://mybinder.org/v2/gh/JohanMabille/jupyterlab/inspect?urlpath=lab-dev

@JohanMabille
Copy link
Member Author

@jtpio I've changed the approach here: a new signal handler is connected to the iopubMessage signal, and asked for the defined variables after an idel message whose parent message type is execute_request has been received.

I'm afraid we may miss updates of variables due to widgets, but this avoids storing an internal state.

@JohanMabille
Copy link
Member Author

JohanMabille commented Apr 7, 2021

Back to old version of xeus-python, since the upgrade makes the tests fail. We can upgrade in a dedicated PR.

@JohanMabille JohanMabille marked this pull request as draft April 8, 2021 07:07
@JohanMabille JohanMabille marked this pull request as ready for review April 8, 2021 07:23
@JohanMabille
Copy link
Member Author

JohanMabille commented Apr 8, 2021

I have commented out the "restart" test since it now requires xeus-python 0.12.3. However, upgrading xeus requires fixing other tests. I will upgrade and fix all the tests in another PR.

@JohanMabille
Copy link
Member Author

@blink1073
Copy link
Contributor

Super green!

@blink1073 blink1073 added this to the 3.1 milestone Apr 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.

Thanks!

@SylvainCorlay SylvainCorlay merged commit 7e682ed into jupyterlab:master Apr 9, 2021
@JohanMabille JohanMabille deleted the inspect branch April 10, 2021 20:27
@lawrence-z-arabii
Copy link

I suppose it's off topic but any ideas why 'table view' is only partly of variable's window height and has its own scrollbar?

[jupyterlab 3.0.14]

image

@jtpio
Copy link
Member

jtpio commented Apr 23, 2021

@lawrence-z-arabii do you mind opening a separate issue to track this? (if none already exists)

The variable inspection from this PR is not in 3.0.14 so it's probably not directly related to this change. But it is in 3.1.0a5 if you want to try it out.

Thanks!

@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Jan 19, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement pkg:debugger pkg:services status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants