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

debugging work #6505

Merged
merged 8 commits into from
Jul 6, 2021
Merged

debugging work #6505

merged 8 commits into from
Jul 6, 2021

Conversation

DavidKutu
Copy link

For #6467
For #6391 its the following:

  • don't make up a random session ids
  • delete temp files (the kernel is not doing this)
  • use IDisposableRegistry
  • check if we need the vscode.debug.breakpoints on activate
  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).

@DavidKutu DavidKutu requested a review from a team as a code owner July 1, 2021 22:53
@DavidKutu DavidKutu changed the title debugging2 debugging work Jul 1, 2021
this.cellToFile.forEach((tempPath) => {
const norm = path.normalize(tempPath);
const dir = path.dirname(norm);
void this.fs.deleteLocalFile(norm);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to try catch on any of these?

Copy link
Author

Choose a reason for hiding this comment

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

good idea

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2021

Codecov Report

Merging #6505 (67c6f64) into main (76f30dc) will increase coverage by 0%.
The diff coverage is 29%.

❗ Current head 67c6f64 differs from pull request most recent head a2fb384. Consider uploading reports for the commit a2fb384 to get more accurate results

@@          Coverage Diff          @@
##            main   #6505   +/-   ##
=====================================
  Coverage     69%     69%           
=====================================
  Files        410     410           
  Lines      28248   28239    -9     
  Branches    4190    4185    -5     
=====================================
+ Hits       19619   19621    +2     
+ Misses      6963    6958    -5     
+ Partials    1666    1660    -6     
Impacted Files Coverage Δ
src/client/debugger/jupyter/kernelDebugAdapter.ts 5% <0%> (-1%) ⬇️
src/client/debugger/jupyter/debuggingManager.ts 35% <71%> (-2%) ⬇️
src/client/datascience/raw-kernel/rawSocket.ts 84% <0%> (-2%) ⬇️
...t/datascience/notebook/vscodeNotebookController.ts 79% <0%> (-1%) ⬇️
src/client/common/application/notebook.ts 88% <0%> (ø)
...ient/datascience/interactive-ipynb/nativeEditor.ts 69% <0%> (+<1%) ⬆️
src/client/datascience/jupyter/jupyterNotebook.ts 76% <0%> (+<1%) ⬆️
...rc/client/datascience/jupyter/debuggerVariables.ts 79% <0%> (+2%) ⬆️
src/client/common/process/pythonDaemonPool.ts 80% <0%> (+4%) ⬆️


const debugRequest = (message: DebugProtocol.Request): KernelMessage.IDebugRequestMsg => {
const sessionId = message.arguments?.__sessionId ? message.arguments.__sessionId : randomBytes(8).toString('hex');
Copy link
Member

Choose a reason for hiding this comment

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

Hang on a second, shouldn't this be the Jupyter session ID?

Copy link
Author

Choose a reason for hiding this comment

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

this will be sent to jupyter, and if the message came from jupyter the casting should put it there. But I'll test and make sure.

Copy link
Author

Choose a reason for hiding this comment

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

so, sessionId only comes in the attach message. I think I should save it and send the same one but I'm not sure. I'll investigate.

@DavidKutu DavidKutu requested a review from IanMatthewHuff July 6, 2021 20:04
@DavidKutu DavidKutu merged commit 54d2ebe into main Jul 6, 2021
@DavidKutu DavidKutu deleted the david/debugging2 branch July 6, 2021 23:49
DavidKutu pushed a commit that referenced this pull request Jul 6, 2021
* don't fetch breakpoints on activate

* implement IDisposable on the debugging manager

* pass on session id's

* delete temp files

* lint

* add try catch

* store sessionId

* pass jupyter session id
DavidKutu pushed a commit that referenced this pull request Jul 7, 2021
* don't fetch breakpoints on activate

* implement IDisposable on the debugging manager

* pass on session id's

* delete temp files

* lint

* add try catch

* store sessionId

* pass jupyter session id
DavidKutu pushed a commit that referenced this pull request Jul 8, 2021
* prepare release (#6403)

* Port jupyter kernel fix to release branch (#6486)

* Fix some problems running kernels with 'jupyter' or no raw kernel (#6464)

* Fix for 6409

* Jupyter server specific case for kernel data not being used

* Update changelog

* Fix interactive window debug cell, codelens tracking, go to cell, expand/collapse all from toolbar when focus is not on notebook editor (#6469) (#6493)

* fix #6423

* Debug cell fixes

* update execution count.

* Expand/collapse cell should not rely on `activeNotebookEditor`

* Format

* Refactor add/debug code codepaths

* Prettier

Co-authored-by: rebornix <penn.lv@gmail.com>

Co-authored-by: rebornix <penn.lv@gmail.com>

* enble the debugging setting (#6492)

* Port #6503 and #6504 to release (#6507)

* Don't pass registrationData twice when registering notebook serializer (#6503)

* If in Python daily insiders and VS Code Insiders opt into native interactive window (#6504)

* If in Python daily insiders and VS Code Insiders opt into native interactive window

* Oops

* Remove unused import

* Changelog

* Format

* debugging work (#6505) (#6558)

* don't fetch breakpoints on activate

* implement IDisposable on the debugging manager

* pass on session id's

* delete temp files

* lint

* add try catch

* store sessionId

* pass jupyter session id

* Remove start page in favor of a message (#6576)

* prepare release (#6590)

* prepare

* use same version in package.jpsn and changelog

* leave version at 2021.8.0

* publish release

* update package-lock

Co-authored-by: Rich Chiodo <rchiodo@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: rebornix <penn.lv@gmail.com>
Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>
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.

4 participants