-
Notifications
You must be signed in to change notification settings - Fork 358
Additional logging and cleanup of Kernel resources #17220
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds additional logging capabilities for kernel messages and improves resource cleanup of kernel execution objects. The changes introduce a new debug setting logKernelMessages that enables detailed kernel message tracing, and ensure proper disposal of NotebookKernelExecution instances when kernels are disposed.
Key changes:
- New
logKernelMessagessetting to enable detailed kernel message logging for debugging purposes INotebookKernelExecutioninterface now extendsIDisposableto support proper cleanup- Kernel providers now properly dispose execution instances when kernels are disposed
- Test improvements including better error messages and test naming consistency
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/platform/common/types.ts |
Added logKernelMessages boolean property to IJupyterSettings interface |
src/platform/common/constants.ts |
Added global logKernelMessages flag and setter function |
src/platform/common/configSettings.ts |
Added logKernelMessages property to JupyterSettings class |
src/kernels/types.ts |
Extended INotebookKernelExecution interface with IDisposable |
src/kernels/kernel.ts |
Added kernel message logging when logKernelMessages is enabled |
src/kernels/kernelExecution.ts |
Changed NotebookKernelExecution to extend DisposableStore for proper cleanup |
src/kernels/kernelProvider.base.ts |
Added execution disposal when kernel is disposed |
src/kernels/kernelProvider.node.ts |
Updated to pass execution instance to disposal handler |
src/kernels/kernelProvider.web.ts |
Updated to pass execution instance to disposal handler |
src/kernels/raw/session/rawKernelConnection.node.ts |
Added subshellId to kernel interrupt request for protocol compliance |
src/extension.common.ts |
Added initialization of logKernelMessages global from settings |
src/test/standardTest.node.ts |
Enabled logKernelMessages setting in test configuration |
src/test/common.node.ts |
Added console logging when capturing screenshots |
src/test/datascience/notebook/helper.ts |
Improved error messages with more context (notebook URI, cell count, content) |
src/test/datascience/notebook/executionService.vscode.test.ts |
Removed unused test stub code |
src/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.ts |
Added test suite suffix to test names for better identification in test output |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
No description provided.