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

Fix for issue #11396 #11436

Merged
merged 2 commits into from
Oct 4, 2022
Merged

Conversation

adam-zlatniczki
Copy link
Contributor

  • During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.

Fixes #11396

  • 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 feature-requests.
  • 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).

@adam-zlatniczki adam-zlatniczki force-pushed the notebook_debug branch 2 times, most recently from c3596e5 to 998fbac Compare September 23, 2022 12:43
src/notebooks/debugger/kernelDebugAdapter.ts Outdated Show resolved Hide resolved
src/notebooks/debugger/kernelDebugAdapter.ts Outdated Show resolved Hide resolved
src/notebooks/debugger/kernelDebugAdapter.ts Outdated Show resolved Hide resolved
* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.
@adam-zlatniczki
Copy link
Contributor Author

Hi Rob, sorry for the late response, I was quite occupied the last week or so. I've introduced some changes based on your comments. I was wondering a bit about where such a utility function would fit well. As I see it, this kind of functionality is only expected to be used in kernel debug adapters, so I decided to make it available as a static method of the KernelDebugAdapterBase class instead of putting it into the general path utilities. If you think it would be better the other way, then I can move the function and the unit tests to the path library.

@roblourens roblourens self-assigned this Oct 3, 2022
@roblourens roblourens added this to the October 2022 milestone Oct 4, 2022
@roblourens roblourens merged commit 0271863 into microsoft:main Oct 4, 2022
roblourens added a commit that referenced this pull request Oct 5, 2022
roblourens added a commit that referenced this pull request Oct 5, 2022
@jersonchua
Copy link

I see that the fix was reverted. I'm seeing the same issue reported in #11396
How can I address it? VSCode is running in windows and the kernel is running in a Linux container.

Jupyter Extension version: v2023.11.1003402403
ipykernel: 6.20.2
debugpy: 1.80
python: 3.10.2

Here are some relevant logs.

sourcePath in dumpCell has forward slash
2024-08-09 15:01:14.644 info: < {"header": {"msg_id": "491ec245-2cadc611fe3575525aa4b73a_338_103", "msg_type": "debug_reply", "username": "glue_user", "session": "491ec245-2cadc611fe3575525aa4b73a", "date": "2024-08-09T19:01:14.570370Z", "version": "5.3"}, "msg_id": "491ec245-2cadc611fe3575525aa4b73a_338_103", "msg_type": "debug_reply", "parent_header": {"date": "2024-08-09T19:01:14.093000Z", "msg_id": "21d47a00-7e31-4fea-b50b-b03e371a1bc5", "msg_type": "debug_request", "session": "bdc61c4a-6655-4165-bfc7-0e1360b7751e", "username": "", "version": "5.2"}, "metadata": {}, "content": {"type": "response", "request_seq": 15, "success": true, "command": "dumpCell", "body": {"sourcePath": "/tmp/ipykernel_338/2523026605.py"}}, "buffers": [], "channel": "control"}

path in setBreakpoints has back slash. I think this is caused by the path normalization logic in the extension.
2024-08-09 15:01:14.662 info: > {"buffers":[],"channel":"control","content":{"seq":10,"type":"request","command":"setBreakpoints","arguments":{"source":{"name":"Untitled-1.ipynb","path":"\tmp\ipykernel_338\2523026605.py"},"lines":[2],"breakpoints":[{"line":2}],"sourceModified":false}},"header":{"date":"2024-08-09T19:01:14.662Z","msg_id":"c5f0353e-a3d8-4cdf-b14c-df854ef4c0ae","msg_type":"debug_request","session":"bdc61c4a-6655-4165-bfc7-0e1360b7751e","username":"","version":"5.2"},"metadata":{},"parent_header":{}}

And because the path is the back slash, the debugger returned "Breakpoint in file that does not exist"
2024-08-09 15:01:14.737 info: < {"header": {"msg_id": "491ec245-2cadc611fe3575525aa4b73a_338_109", "msg_type": "debug_reply", "username": "glue_user", "session": "491ec245-2cadc611fe3575525aa4b73a", "date": "2024-08-09T19:01:14.669487Z", "version": "5.3"}, "msg_id": "491ec245-2cadc611fe3575525aa4b73a_338_109", "msg_type": "debug_reply", "parent_header": {"date": "2024-08-09T19:01:14.662000Z", "msg_id": "c5f0353e-a3d8-4cdf-b14c-df854ef4c0ae", "msg_type": "debug_request", "session": "bdc61c4a-6655-4165-bfc7-0e1360b7751e", "username": "", "version": "5.2"}, "metadata": {}, "content": {"seq": 25, "type": "response", "request_seq": 10, "success": true, "command": "setBreakpoints", "body": {"breakpoints": [{"verified": false, "id": 1, "message": "Breakpoint in file that does not exist.", "source": {"name": "Untitled-1.ipynb", "path": "\tmp\ipykernel_338\2523026605.py"}, "line": 2}]}}, "buffers": [], "channel": "control"}

Please let me know if there's any additional information I can provide. Thanks.

@adam-zlatniczki
Copy link
Contributor Author

If I recall correctly, Rob mentioned something about the change breaking the CI for the browser-based VSCode frontend, that's why it had to be reverted. Since I moved to WSL I didn't face the Windows-Unix cross-OS issue anymore and forgot about this. Unfortunately I don't have time to pick up the work right now, but you can open a new Issue and refer this one.

@jersonchua
Copy link

@adam-zlatniczki i just opened a new issue: #15934

Thanks.

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.

Cell debug doesn't stop at breakpoints when VSCode is run on Windows but a remote kernel runs on Unix
4 participants