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

Implement restart #11668

Merged
merged 13 commits into from
Oct 21, 2022
Merged

Implement restart #11668

merged 13 commits into from
Oct 21, 2022

Conversation

roblourens
Copy link
Member

Towards #7670

Also includes some refactoring so that this flow reads more nicely.
I had to

  • Flip around the flow of starting a cell debug session- previously it did init work in the command, starting a debug session, instantiating the DA then passing the session object back to the command handler. Now the command only kicks off a debug session, and all the interesting work happens inside the debug adapter factory, which is more straightforward
  • Modify the initialize request to add the capability flag that the DA will implement the "restart" request
  • Intercept the restart request, and start a new debug session. The reason we have to implement the restart request is because the index of the notebook cell may have changed since the original request, and we will start a launch config using the new index.

Remaining: This is not hooked up for IW debugging yet

@roblourens roblourens marked this pull request as draft October 14, 2022 21:27
@roblourens roblourens force-pushed the roblou/gentle-turkey branch from 6685086 to 25ca6ff Compare October 14, 2022 21:31
@roblourens roblourens marked this pull request as ready for review October 14, 2022 21:32
Copy link
Member

@IanMatthewHuff IanMatthewHuff left a comment

Choose a reason for hiding this comment

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

Awesome. Looks great.

@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2022

Codecov Report

Merging #11668 (fbc6769) into main (543900b) will decrease coverage by 17%.
The diff coverage is 35%.

❗ Current head fbc6769 differs from pull request most recent head 705f8b6. Consider uploading reports for the commit 705f8b6 to get more accurate results

@@           Coverage Diff            @@
##            main   #11668     +/-   ##
========================================
- Coverage     59%      41%    -18%     
========================================
  Files        483      486      +3     
  Lines      34509    34480     -29     
  Branches    5609     5592     -17     
========================================
- Hits       20401    14359   -6042     
- Misses     12128    18908   +6780     
+ Partials    1980     1213    -767     
Impacted Files Coverage Δ
src/notebooks/debugger/debugger.ts 20% <0%> (+3%) ⬆️
src/notebooks/debugger/kernelDebugAdapterBase.ts 6% <0%> (-1%) ⬇️
src/notebooks/debugger/debuggingManager.ts 31% <11%> (+10%) ⬆️
src/notebooks/debugger/debuggingManagerBase.ts 24% <17%> (-12%) ⬇️
...otebooks/debugger/controllers/restartController.ts 21% <21%> (ø)
...ive-window/debugger/jupyter/debugCellController.ts 22% <25%> (ø)
...active-window/debugger/jupyter/debuggingManager.ts 33% <26%> (+2%) ⬆️
src/notebooks/debugger/commandRegistry.ts 37% <37%> (ø)
...rc/interactive-window/debugger/jupyter/debugger.ts 42% <42%> (ø)
src/notebooks/debugger/helper.ts 19% <42%> (+<1%) ⬆️
... and 274 more

@roblourens roblourens merged commit 251bcc7 into main Oct 21, 2022
@roblourens roblourens deleted the roblou/gentle-turkey branch October 21, 2022 19:33
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 this pull request may close these issues.

5 participants