Simplify starting how we start local Jupyter, remote Jupyter and raw kernels #13445
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DonJayamanne
force-pushed
the
refactorRemotes
branch
from
7b98f23
to
3cb7d1a
Compare
DonJayamanne
changed the title
amunger
reviewed
May 4, 2023
| const kernelConnection = options.kernelConnection; | ||
| const isLocal = isLocalConnection(kernelConnection); | ||
|
|
||
| if (this.rawKernelSessionCreator?.isSupported && isLocal) { | ||
| return this.rawKernelSessionCreator.create( | ||
| return this.createRawKernelSession(this.rawKernelSessionCreator, options); |
There was a problem hiding this comment.
nit: no need to pass the member rawKernelSessionCreator to a private method
amunger
reviewed
May 4, 2023
amunger
reviewed
May 4, 2023
| NotebookCreationOptions | ||
| } from '../../types'; | ||
| import { Cancellation } from '../../../platform/common/cancellation'; | ||
| import { ConnectNotebookProviderOptions, IJupyterConnection } from '../../types'; | ||
| import { IJupyterNotebookProvider, IJupyterServerProvider } from '../types'; | ||
|
|
||
| // When the NotebookProvider looks to create a notebook it uses this class to create a Jupyter notebook | ||
| @injectable() | ||
| export class JupyterNotebookProvider implements IJupyterNotebookProvider { |
There was a problem hiding this comment.
Could other classes just use the jupyterServerConnector or just the ServerProvider directly so that this class can be dropped? It's not providing notebooks anymore as its name suggests.
amunger
approved these changes
May 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #12832, #12831
Before (steps to start a kernel)
After (we now have clear and distinct classes with specific responsibilities)