Fix interactive window debugging with ipykernel6 #6691
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6691 +/- ##
======================================
- Coverage 69% 68% -1%
======================================
Files 412 412
Lines 28473 28599 +126
Branches 4241 4269 +28
======================================
- Hits 19825 19715 -110
- Misses 6991 7206 +215
- Partials 1657 1678 +21
|
| @@ -88,7 +88,8 @@ export class Kernel implements IKernel { | |||
| private readonly kernelExecution: KernelExecution; | |||
| private startCancellation = new CancellationTokenSource(); | |||
| constructor( | |||
| public readonly uri: Uri, | |||
| public readonly notebookUri: Uri, | |||
| public readonly resourceUri: Uri, | |||
There was a problem hiding this comment.
Today in our notebook provider we have the distinction between IdentityUri & ResourceUri.
This change merely ensures we support that in the IKernel object as well.
| identity: this.uri, | ||
| resource: this.uri, | ||
| identity: this.notebookUri, | ||
| resource: this.resourceUri, |
There was a problem hiding this comment.
Now, we're passing the right resource, else we'd be passing the Uri of the Interactive Notebook Uri instead of the the Python file Uri (i.e. works the way it did with inteactive webview)
| */ | ||
| get(uri: Uri): IKernel | undefined; | ||
| get(notebook: NotebookDocument): IKernel | undefined; |
There was a problem hiding this comment.
Didn't want to keep the Uri, its too ambiguous,
this way we're very explicit.
| @@ -257,9 +259,10 @@ export class NativeInteractiveWindow implements IInteractiveWindowLoadable { | |||
| } | |||
|
|
|||
| private registerKernel(notebookDocument: NotebookDocument, controller: VSCodeNotebookController) { | |||
| const kernel = this.kernelProvider.getOrCreate(notebookDocument.uri, { | |||
| const kernel = this.kernelProvider.getOrCreate(notebookDocument, { | |||
There was a problem hiding this comment.
No ambiguity with the argument now, we pass the document, else if it was uri, one could accidentally provide the resource Uri (owning uri)
src/client/datascience/interactive-window/nativeInteractiveWindow.ts
Outdated
Show resolved
Hide resolved
|
Just waiting on the suggestion that Joyce made. Otherwise looks good to me |
@joyceerhl thanks for the explanation, initially i didn't quite understand that. It makes perfect sense. Updated the PR. Please check |
For #6534