-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Kernel selection UI #8866
Kernel selection UI #8866
Changes from all commits
55bf0f4
3ef83f8
5e22dc5
a89eb8d
6293ef1
5a4a6aa
7aee062
bc0d9f7
5cc2346
ee1169a
a86da06
2f1238b
02cbba0
0b5dac3
e25eb72
22efccd
bbbae0a
23accfc
5f0bb15
25e2d19
2f758f4
f2caf13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Added kernel status and selection toolbar to the notebook editor. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import { ConfigurationTarget, Event, EventEmitter, Position, Range, Selection, T | |
import { Disposable } from 'vscode-jsonrpc'; | ||
import * as vsls from 'vsls/vscode'; | ||
|
||
import { ServerStatus } from '../../../datascience-ui/interactive-common/mainState'; | ||
import { | ||
IApplicationShell, | ||
IDocumentManager, | ||
|
@@ -31,8 +32,9 @@ import { IInterpreterService, PythonInterpreter } from '../../interpreter/contra | |
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; | ||
import { generateCellRangesFromDocument } from '../cellFactory'; | ||
import { CellMatcher } from '../cellMatcher'; | ||
import { Identifiers, Telemetry } from '../constants'; | ||
import { Identifiers, Settings, Telemetry } from '../constants'; | ||
import { ColumnWarningSize } from '../data-viewing/types'; | ||
import { DataScience } from '../datascience'; | ||
import { | ||
IAddedSysInfo, | ||
ICopyCode, | ||
|
@@ -48,6 +50,7 @@ import { | |
import { JupyterInstallError } from '../jupyter/jupyterInstallError'; | ||
import { JupyterSelfCertsError } from '../jupyter/jupyterSelfCertsError'; | ||
import { JupyterKernelPromiseFailedError } from '../jupyter/kernels/jupyterKernelPromiseFailedError'; | ||
import { KernelSpecInterpreter } from '../jupyter/kernels/kernelSelector'; | ||
import { CssMessages } from '../messages'; | ||
import { | ||
CellState, | ||
|
@@ -110,6 +113,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp | |
@unmanaged() private jupyterVariables: IJupyterVariables, | ||
@unmanaged() private jupyterDebugger: IJupyterDebugger, | ||
@unmanaged() protected ipynbProvider: INotebookEditorProvider, | ||
@unmanaged() private dataScience: DataScience, | ||
@unmanaged() protected errorHandler: IDataScienceErrorHandler, | ||
@unmanaged() rootPath: string, | ||
@unmanaged() scripts: string[], | ||
|
@@ -261,6 +265,14 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp | |
this.handleMessage(message, payload, this.requestOnigasm); | ||
break; | ||
|
||
case InteractiveWindowMessages.SelectKernel: | ||
this.handleMessage(message, payload, this.selectKernel); | ||
break; | ||
|
||
case InteractiveWindowMessages.SelectJupyterServer: | ||
this.handleMessage(message, payload, this.selectServer); | ||
break; | ||
|
||
default: | ||
break; | ||
} | ||
|
@@ -1068,6 +1080,25 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp | |
if (this.notebook) { | ||
const uri: Uri = await this.getNotebookIdentity(); | ||
this.postMessage(InteractiveWindowMessages.NotebookExecutionActivated, uri.toString()).ignoreErrors(); | ||
|
||
const statusChangeHandler = async (status: ServerStatus) => { | ||
if (this.notebook) { | ||
const settings = this.configuration.getSettings(); | ||
const kernelSpec = this.notebook.getKernelSpec(); | ||
|
||
if (kernelSpec) { | ||
const name = kernelSpec.display_name; | ||
|
||
await this.postMessage(InteractiveWindowMessages.UpdateKernel, { | ||
jupyterServerStatus: status, | ||
localizedUri: settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch ? | ||
localize.DataScience.localJupyterServer() : settings.datascience.jupyterServerURI, | ||
displayName: name | ||
}); | ||
} | ||
} | ||
}; | ||
this.notebook.onSessionStatusChanged(statusChangeHandler); | ||
} | ||
|
||
traceInfo('Connected to jupyter server.'); | ||
|
@@ -1265,4 +1296,32 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp | |
this.postMessage(InteractiveWindowMessages.LoadOnigasmAssemblyResponse, undefined).ignoreErrors(); | ||
} | ||
} | ||
|
||
private async selectServer() { | ||
await this.dataScience.selectJupyterURI(); | ||
} | ||
|
||
private async selectKernel() { | ||
const settings = this.configuration.getSettings(); | ||
|
||
let kernel: KernelSpecInterpreter | undefined; | ||
|
||
if (settings.datascience.jupyterServerURI.toLowerCase() === Settings.JupyterServerLocalLaunch) { | ||
kernel = await this.dataScience.selectLocalJupyterKernel(); | ||
} else if (this.notebook) { | ||
const connInfo = this.notebook.server.getConnectionInfo(); | ||
|
||
if (connInfo) { | ||
kernel = await this.dataScience.selectRemoteJupyterKernel(connInfo); | ||
} | ||
} | ||
|
||
if (kernel && kernel.kernelSpec && this.notebook) { | ||
// Tell the kernel. A status update should fire that changes our display | ||
await this.notebook.setKernelSpec(kernel.kernelSpec); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should't this be done first, before we update the UI? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that's probably better. Let me try that. I think I can eliminate the status update then too. In reply to: 356777545 [](ancestors = 356777545) |
||
|
||
// Add in a new sys info | ||
await this.addSysInfo(SysInfoReason.New); | ||
} | ||
} | ||
} |
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.
This is a dup, we need to use the existing setting
Settings.JupyterServerLocalLaunch
.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.
I'm going to keep this here, but use it only for display