elyra icon indicating copy to clipboard operation
elyra copied to clipboard

Script editor debugger - experimental

Open karlaspuldaro opened this issue 3 years ago • 4 comments

Closes #975 - Integrate Script Editor with JupyterLab debugger

What changes were proposed in this pull request?

  • [x] Subclass ScriptEditor (R + Python): PR #2388 ~~Add 'Run and Debug' button~~ ~~Add debugger code editor handler~~
  • [x] Get debugger service from activate plugin + create file debug handler
  • [x] Enable/disable debug button on kernel selection change
  • [x] Start debugger session for handler
  • [x] Set breakpoints automatically as per supported kernel selection
  • [x] Fix kernel spec object data inconsistencies on get specs (ScriptEditorController)
  • [x] Fix console errors triggered by getKernelSelection
  • [x] Update/restart debug session on kernelspec change
  • [X] Handle session on page reload
  • [x] Keep session connection open open after run when debug mode is on and selected kernel supports debugging
  • [ ] Session connection shouldn't shutdown after a run
  • [ ] Stop button should stop a run, not a session connection
  • [ ] Tests
  • [ ] Documentation
  • (Follow up) Refactor toolbar usage (move to @jupyterlab/ui-components library v4.0.0-alpha): issue #2375
  • (Follow up - known issue) Fix toolbar style in safari

How was this pull request tested?

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

karlaspuldaro avatar Aug 31 '21 15:08 karlaspuldaro

Thanks for making a pull request to Elyra!

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

elyra-bot[bot] avatar Aug 31 '21 15:08 elyra-bot[bot]

What would be in script-editor-debugger-extension?

lresende avatar Jun 01 '22 20:06 lresende

@lresende for now just for POC purposes, we needed to get a debugger service which is available on the activate function of new plugins. Since the script editor package is not built to be an actual extension, and we don't want to do that for each specific editor, we decided to add a new extension for the script editor debugger that can access our custom handler.

karlaspuldaro avatar Jun 03 '22 15:06 karlaspuldaro

@ajbozarth,

I plan to check this out and try it locally next, but given I've had issues installing Zeus kernel in the past it might not be easy

ipykernel supports debugging now as well. If the option isn't displayed, you might have an older package or kernel.json file. I'm running the following package versions for reference...

ipykernel==6.15.0
ipython==8.4.0

and the kernel.json must include at least the following metadata stanza (and sibling to argv):

 "metadata": {
  "debugger": true
 }

kevin-bates avatar Jul 19 '22 20:07 kevin-bates

Setting a break point in line one causes a fatal error image

ptitzler avatar Aug 15 '22 08:08 ptitzler

Nit (usability): If we have control over the behavior:

  • Enable debugger

  • Set break point

  • Run to break point

  • Click 'stop'

    image

Stop button should stop a run and not a session connection

Can you clarify if this is the reason why one cannot re-enable the debugger and cannot run the code again? image

Since we are labeling the status as 'experimental', can we also create a document for end-users containing list of functionality that is known to be limited and what, if any, workarounds are available? The goal is to avoid getting issues raised about known limitations. We can link to the document from the release notes, which are now conveniently accessible via the what's new link in the launcher.

For example 'Stop button should stop a run and not a session connection' would be too technical because it requires that the user understand what the difference is between a 'run' and a 'session'. A user-friendly description might state something like

  • Clicking STOP while the debugger is enabled prevents the user from running the script using the RUN button. As a workaround, close the Python editor and re-open it.

ptitzler avatar Aug 15 '22 09:08 ptitzler

Setting a break point in line one causes a fatal error !

It works as expected on my env :/ @ptitzler What versions do you have for your local python and ipython kernel? I have Python v3.10.4 and ipykernel v6.13.1

karlaspuldaro avatar Aug 15 '22 20:08 karlaspuldaro

As another data point, I can reproduce the "breakpoint on line 1" issue as well. Just to clarify, the breakpoint "sets" fine, but the script gets the error when run. I'm using ipykernel == 6.15.0 and python == 3.9.13.

kevin-bates avatar Aug 15 '22 21:08 kevin-bates

@kevin-bates thx for checking! I will try it again with your config, it runs with no error on my env.

karlaspuldaro avatar Aug 15 '22 21:08 karlaspuldaro

FWIW - I see the same issue from within a Python-based notebook as well - so the line-1-breakpoint issue is probably an upstream issue.

kevin-bates avatar Aug 15 '22 21:08 kevin-bates

Can you clarify if this is the reason why one cannot re-enable the debugger and cannot run the code again?

Yes and no. The user is supposed to still run the code again after shutting down the session, but you're correct, one workaround to re-enable debugging after a clicking the shutdown button is to close and reopen the editor. I agree to list the limitations in the docs. Coming soon!

karlaspuldaro avatar Aug 15 '22 21:08 karlaspuldaro

@kevin-bates @ptitzler I upgraded my ipykernel version and still didn't see the issue, although I was able to reproduce it using python 3.9. I guess we can open an issue about this upstream.

karlaspuldaro avatar Aug 15 '22 22:08 karlaspuldaro

This has already been fixed in debugpy - there just isn't a release built for it yet: https://github.com/microsoft/debugpy/issues/995

I was able to reproduce it using python 3.9.

It's interesting that you're finding it related to the Python version because the code changes just look like "off-by-one" handling kinds of things.

kevin-bates avatar Aug 15 '22 22:08 kevin-bates

What versions do you have for your local python and ipython kernel? I have Python v3.10.4 and ipykernel v6.13.1

  • Python 3.7.12
  • ipykernel 6.15.1

ptitzler avatar Aug 16 '22 05:08 ptitzler

This has already been fixed in debugpy - there just isn't a release built for it yet: microsoft/debugpy#995

Coincidence or not, but an hour after your comment https://github.com/microsoft/debugpy/releases/tag/v1.6.3 was released, containing the fix. I confirmed that no error is reported with 1.6.3 installed.

ptitzler avatar Aug 16 '22 08:08 ptitzler

re: documentation We should probably include a link to https://jupyterlab.readthedocs.io/en/stable/user/debugger.html (most useful link I was able to find) and verbiage that makes it clear that the debugger is a JupyterLab feature. Without a clear separation between Elyra's focus on integrating the debugger with the Python editor and not the debugger itself, we might end up receiving bug/feature reports for issues that need to be handled upstream.

ptitzler avatar Aug 16 '22 08:08 ptitzler

A nice-to-have usability feature, perhaps for the future:

  • If one enables the debugger, currently the debugger side panel is expanded. image
  • However, if one disables the debugger the panel is currently not minimized, forcing the user to do that manually.

ptitzler avatar Aug 18 '22 09:08 ptitzler

A nice-to-have usability feature, perhaps for the future:

  • If one enables the debugger, currently the debugger side panel is expanded.
  • However, if one disables the debugger the panel is currently not minimized, forcing the user to do that manually.

Agreed, the same behavior happens in notebooks so we can implement the enhancement upstream

karlaspuldaro avatar Aug 18 '22 15:08 karlaspuldaro

@ptitzler thx for reviewing! Comments are addressed.

karlaspuldaro avatar Aug 19 '22 15:08 karlaspuldaro

@kiersten-stokes thanks for the feedback!

karlaspuldaro avatar Aug 19 '22 18:08 karlaspuldaro

@akchinSTC thank you for the suggestions, doc updates addressed

karlaspuldaro avatar Aug 19 '22 21:08 karlaspuldaro