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

Script editor debugger - experimental #2087

Merged
merged 98 commits into from
Aug 19, 2022
Merged

Conversation

karlaspuldaro
Copy link
Member

@karlaspuldaro karlaspuldaro commented Aug 31, 2021

Closes #975 - Integrate Script Editor with JupyterLab debugger

What changes were proposed in this pull request?

debugger

  • Subclass ScriptEditor (R + Python): PR Add Script Editor language-specific subclasses #2388
  • Get debugger service from activate plugin + create file debug handler
  • Enable/disable debug button on kernel selection change
  • Start debugger session for custom handler
  • Update/restart debug session on kernelspec change
  • Handle session on page reload
  • Keep session connection open open after run when debug mode is on and selected kernel supports debugging
  • Change stop button behavior to interrupt a kernel run and not shutdown a session connection
  • Fix kernel spec init
  • Fix/comment existing python editor / pipeline tests
  • Add/update documentation

Know issues / Follow ups:

How was this pull request tested?

No new test added for the experimental feature yet - debugger feature tested manually, tests to follow.
Existing script editor tests updated.

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.

@elyra-bot
Copy link

elyra-bot bot commented Aug 31, 2021

Thanks for making a pull request to Elyra!

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

@karlaspuldaro karlaspuldaro added the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Aug 31, 2021
@lresende lresende force-pushed the master branch 2 times, most recently from 13b7ffe to 9c6e8d9 Compare September 30, 2021 20:24
@ptitzler
Copy link
Member

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.

@karlaspuldaro
Copy link
Member Author

karlaspuldaro commented Aug 18, 2022

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

Copy link
Member

@ptitzler ptitzler left a comment

Choose a reason for hiding this comment

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

minor edits for consistency and relevancy

docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
@karlaspuldaro
Copy link
Member Author

@ptitzler thx for reviewing! Comments are addressed.

Copy link
Member

@kiersten-stokes kiersten-stokes left a comment

Choose a reason for hiding this comment

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

This is super cool! I just have a few documentation NITs below

docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
@karlaspuldaro
Copy link
Member Author

@kiersten-stokes thanks for the feedback!

docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
docs/source/user_guide/enhanced-script-support.md Outdated Show resolved Hide resolved
Fix integration tests calling dismissAssistant
@karlaspuldaro
Copy link
Member Author

@akchinSTC thank you for the suggestions, doc updates addressed

@akchinSTC akchinSTC removed the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Aug 19, 2022
Copy link
Member

@kiersten-stokes kiersten-stokes left a comment

Choose a reason for hiding this comment

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

This is exciting!

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.

Integrate Script Editor with JupyterLab debugger
7 participants