Notebooks: Support User-Installed Kernels (#10194)

* Fixes for attach to. Still need to update cache

* Don't have jupyter hardcoded

* Handle DOTNET_ROOT env var

* Fix on load

* Put behind feature flag

* Cleanup

* Error check

* PR feedback
This commit is contained in:
Chris LaFreniere
2020-04-29 10:34:27 -07:00
committed by GitHub
parent 5b57b13005
commit 0a3a0ec80f
7 changed files with 78 additions and 39 deletions

View File

@@ -332,6 +332,12 @@ export class JupyterServerInstallation implements IJupyterServerInstallation {
let env = Object.assign({}, process.env); let env = Object.assign({}, process.env);
delete env['Path']; // Delete extra 'Path' variable for Windows, just in case. delete env['Path']; // Delete extra 'Path' variable for Windows, just in case.
env['PATH'] = this.pythonEnvVarPath; env['PATH'] = this.pythonEnvVarPath;
// We don't want Jupyter to know about DOTNET_ROOT, as it's been modified by the liveshare experience
// Without this, won't be able to find the ASP.NET bits
if (process.env['DOTNET_ROOT']) {
delete env['DOTNET_ROOT'];
}
this.execOptions = { this.execOptions = {
env: env env: env
}; };

View File

@@ -108,6 +108,11 @@ export class JupyterSessionManager implements nb.SessionManager {
// TODO add more info to kernels // TODO add more info to kernels
return kernel; return kernel;
}); });
// For now, need to remove PySpark3, as it's been deprecated
// May want to have a formalized deprecated kernels mechanism in the future
kernels = kernels.filter(k => k.name !== 'pyspark3kernel');
let allKernels: nb.IAllKernels = { let allKernels: nb.IAllKernels = {
defaultKernel: specs.default, defaultKernel: specs.default,
kernels: kernels kernels: kernels
@@ -343,6 +348,11 @@ export class JupyterSession implements nb.ISession {
} }
for (let i = 0; i < Object.keys(process.env).length; i++) { for (let i = 0; i < Object.keys(process.env).length; i++) {
let key = Object.keys(process.env)[i]; let key = Object.keys(process.env)[i];
// DOTNET_ROOT gets set as part of the liveshare experience, but confuses the dotnet interactive kernel
// Not setting this environment variable for notebooks removes this issue
if (key.toLowerCase() === 'dotnet_root') {
continue;
}
if (key.toLowerCase() === 'path' && this._pythonEnvVarPath) { if (key.toLowerCase() === 'path' && this._pythonEnvVarPath) {
allCode += `%set_env ${key}=${this._pythonEnvVarPath}${EOL}`; allCode += `%set_env ${key}=${this._pythonEnvVarPath}${EOL}`;
} else { } else {

View File

@@ -396,7 +396,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
protected initActionBar(): void { protected initActionBar(): void {
let kernelContainer = document.createElement('div'); let kernelContainer = document.createElement('div');
let kernelDropdown = new KernelsDropdown(kernelContainer, this.contextViewService, this.modelReady); let kernelDropdown = this.instantiationService.createInstance(KernelsDropdown, kernelContainer, this.contextViewService, this.modelReady);
kernelDropdown.render(kernelContainer); kernelDropdown.render(kernelContainer);
attachSelectBoxStyler(kernelDropdown, this.themeService); attachSelectBoxStyler(kernelDropdown, this.themeService);

View File

@@ -197,6 +197,19 @@ configurationRegistry.registerConfiguration({
} }
}); });
configurationRegistry.registerConfiguration({
'id': 'notebook',
'title': 'Notebook',
'type': 'object',
'properties': {
'notebook.showAllKernels': {
'type': 'boolean',
'default': false,
'description': localize('notebook.showAllKernels', "(Preview) show all kernels for the current notebook provider.")
}
}
});
/* *************** Output components *************** */ /* *************** Output components *************** */
// Note: most existing types use the same component to render. In order to // Note: most existing types use the same component to render. In order to
// preserve correct rank order, we register it once for each different rank of // preserve correct rank order, we register it once for each different rank of

View File

@@ -27,6 +27,7 @@ import { IObjectExplorerService } from 'sql/workbench/services/objectExplorer/br
import { TreeUpdateUtils } from 'sql/workbench/services/objectExplorer/browser/treeUpdateUtils'; import { TreeUpdateUtils } from 'sql/workbench/services/objectExplorer/browser/treeUpdateUtils';
import { find, firstIndex } from 'vs/base/common/arrays'; import { find, firstIndex } from 'vs/base/common/arrays';
import { INotebookEditor } from 'sql/workbench/services/notebook/browser/notebookService'; import { INotebookEditor } from 'sql/workbench/services/notebook/browser/notebookService';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
const msgLoading = localize('loading', "Loading kernels..."); const msgLoading = localize('loading', "Loading kernels...");
const msgChanging = localize('changing', "Changing kernel..."); const msgChanging = localize('changing', "Changing kernel...");
@@ -208,9 +209,12 @@ export class CollapseCellsAction extends ToggleableAction {
} }
} }
const ShowAllKernelsConfigName = 'notebook.showAllKernels';
const WorkbenchPreviewConfigName = 'workbench.enablePreviewFeatures';
export class KernelsDropdown extends SelectBox { export class KernelsDropdown extends SelectBox {
private model: NotebookModel; private model: NotebookModel;
constructor(container: HTMLElement, contextViewProvider: IContextViewProvider, modelReady: Promise<INotebookModel>) { private _showAllKernels: boolean = false;
constructor(container: HTMLElement, contextViewProvider: IContextViewProvider, modelReady: Promise<INotebookModel>, @IConfigurationService private _configurationService: IConfigurationService) {
super([msgLoading], msgLoading, contextViewProvider, container, { labelText: kernelLabel, labelOnTop: false, ariaLabel: kernelLabel } as ISelectBoxOptionsWithLabel); super([msgLoading], msgLoading, contextViewProvider, container, { labelText: kernelLabel, labelOnTop: false, ariaLabel: kernelLabel } as ISelectBoxOptionsWithLabel);
if (modelReady) { if (modelReady) {
@@ -222,6 +226,12 @@ export class KernelsDropdown extends SelectBox {
} }
this.onDidSelect(e => this.doChangeKernel(e.selected)); this.onDidSelect(e => this.doChangeKernel(e.selected));
this.getAllKernelConfigValue();
this._register(this._configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(ShowAllKernelsConfigName) || e.affectsConfiguration(WorkbenchPreviewConfigName)) {
this.getAllKernelConfigValue();
}
}));
} }
updateModel(model: INotebookModel): void { updateModel(model: INotebookModel): void {
@@ -235,12 +245,23 @@ export class KernelsDropdown extends SelectBox {
// Update SelectBox values // Update SelectBox values
public updateKernel(kernel: azdata.nb.IKernel) { public updateKernel(kernel: azdata.nb.IKernel) {
let kernels: string[] = this.model.standardKernelsDisplayName(); let kernels: string[] = this._showAllKernels ? [...new Set(this.model.specs.kernels.map(a => a.display_name).concat(this.model.standardKernelsDisplayName()))]
: this.model.standardKernelsDisplayName();
if (kernel && kernel.isReady) { if (kernel && kernel.isReady) {
let standardKernel = this.model.getStandardKernelFromName(kernel.name); let standardKernel = this.model.getStandardKernelFromName(kernel.name);
if (kernels) {
if (kernels && standardKernel) { let index;
let index = firstIndex(kernels, kernel => kernel === standardKernel.displayName); if (standardKernel) {
index = firstIndex(kernels, kernel => kernel === standardKernel.displayName);
} else {
let kernelSpec = this.model.specs.kernels.find(k => k.name === kernel.name);
index = firstIndex(kernels, k => k === kernelSpec.display_name);
}
// This is an error case that should never happen
// Just in case, setting index to 0
if (index < 0) {
index = 0;
}
this.setOptions(kernels, index); this.setOptions(kernels, index);
} }
} else if (this.model.clientSession.isInErrorState) { } else if (this.model.clientSession.isInErrorState) {
@@ -254,6 +275,10 @@ export class KernelsDropdown extends SelectBox {
this.setOptions([msgChanging], 0); this.setOptions([msgChanging], 0);
this.model.changeKernel(displayName); this.model.changeKernel(displayName);
} }
private getAllKernelConfigValue(): void {
this._showAllKernels = !!this._configurationService.getValue(ShowAllKernelsConfigName) && !!this._configurationService.getValue(WorkbenchPreviewConfigName);
}
} }
export class AttachToDropdown extends SelectBox { export class AttachToDropdown extends SelectBox {

View File

@@ -402,20 +402,6 @@ suite('notebook model', function (): void {
assert.deepEqual(model.clientSession, mockClientSession.object); assert.deepEqual(model.clientSession, mockClientSession.object);
}); });
test('Should sanitize kernel display name when IP is included', async function (): Promise<void> {
let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
let displayName = 'PySpark (1.1.1.1)';
let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
assert.equal(sanitizedDisplayName, 'PySpark');
});
test('Should sanitize kernel display name properly when IP is not included', async function (): Promise<void> {
let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
let displayName = 'PySpark';
let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
assert.equal(sanitizedDisplayName, 'PySpark');
});
test('Should notify on trust set', async function () { test('Should notify on trust set', async function () {
// Given a notebook that's been loaded // Given a notebook that's been loaded
let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);

View File

@@ -464,7 +464,9 @@ export class NotebookModel extends Disposable implements INotebookModel {
public async startSession(manager: INotebookManager, displayName?: string, setErrorStateOnFail?: boolean): Promise<void> { public async startSession(manager: INotebookManager, displayName?: string, setErrorStateOnFail?: boolean): Promise<void> {
if (displayName && this._standardKernels) { if (displayName && this._standardKernels) {
let standardKernel = find(this._standardKernels, kernel => kernel.displayName === displayName); let standardKernel = find(this._standardKernels, kernel => kernel.displayName === displayName);
this._defaultKernel = { name: standardKernel.name, display_name: standardKernel.displayName }; if (standardKernel) {
this._defaultKernel = { name: standardKernel.name, display_name: standardKernel.displayName };
}
} }
if (this._defaultKernel) { if (this._defaultKernel) {
let clientSession = this._notebookOptions.factory.createClientSession({ let clientSession = this._notebookOptions.factory.createClientSession({
@@ -519,6 +521,13 @@ export class NotebookModel extends Disposable implements INotebookModel {
let provider = this._kernelDisplayNameToNotebookProviderIds.get(this._savedKernelInfo.display_name); let provider = this._kernelDisplayNameToNotebookProviderIds.get(this._savedKernelInfo.display_name);
if (provider && provider !== this._providerId) { if (provider && provider !== this._providerId) {
this._providerId = provider; this._providerId = provider;
} else if (!provider) {
this.notebookOptions.notebookManagers.forEach(m => {
if (m.providerId !== SQL_NOTEBOOK_PROVIDER) {
// We don't know which provider it is before that provider is chosen to query its specs. Choosing the "last" one registered.
this._providerId = m.providerId;
}
});
} }
this._defaultKernel = this._savedKernelInfo; this._defaultKernel = this._savedKernelInfo;
} else if (this._defaultKernel) { } else if (this._defaultKernel) {
@@ -616,10 +625,12 @@ export class NotebookModel extends Disposable implements INotebookModel {
} else if (language.toLowerCase() === 'ipython') { } else if (language.toLowerCase() === 'ipython') {
// Special case ipython because in many cases this is defined as the code mirror mode for python notebooks // Special case ipython because in many cases this is defined as the code mirror mode for python notebooks
language = 'python'; language = 'python';
} else if (language.toLowerCase() === 'c#') {
language = 'cs';
} }
} }
this._language = language; this._language = language.toLowerCase();
} }
public changeKernel(displayName: string): void { public changeKernel(displayName: string): void {
@@ -750,7 +761,6 @@ export class NotebookModel extends Disposable implements INotebookModel {
} }
private getKernelSpecFromDisplayName(displayName: string): nb.IKernelSpec { private getKernelSpecFromDisplayName(displayName: string): nb.IKernelSpec {
displayName = this.sanitizeDisplayName(displayName);
let kernel: nb.IKernelSpec = find(this.specs.kernels, k => k.display_name.toLowerCase() === displayName.toLowerCase()); let kernel: nb.IKernelSpec = find(this.specs.kernels, k => k.display_name.toLowerCase() === displayName.toLowerCase());
if (!kernel) { if (!kernel) {
return undefined; // undefined is handled gracefully in the session to default to the default kernel return undefined; // undefined is handled gracefully in the session to default to the default kernel
@@ -762,7 +772,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
private sanitizeSavedKernelInfo() { private sanitizeSavedKernelInfo() {
if (this._savedKernelInfo) { if (this._savedKernelInfo) {
let displayName = this.sanitizeDisplayName(this._savedKernelInfo.display_name); let displayName = this._savedKernelInfo.display_name;
if (this._savedKernelInfo.display_name !== displayName) { if (this._savedKernelInfo.display_name !== displayName) {
this._savedKernelInfo.display_name = displayName; this._savedKernelInfo.display_name = displayName;
@@ -843,21 +853,6 @@ export class NotebookModel extends Disposable implements INotebookModel {
} }
} }
/**
* Sanitizes display name to remove IP address in order to fairly compare kernels
* In some notebooks, display name is in the format <kernel> (<ip address>)
* example: PySpark (25.23.32.4)
* @param displayName Display Name for the kernel
*/
public sanitizeDisplayName(displayName: string): string {
let name = displayName;
if (name) {
let index = name.indexOf('(');
name = (index > -1) ? name.substr(0, index - 1).trim() : name;
}
return name;
}
private async updateKernelInfo(kernel: nb.IKernel): Promise<void> { private async updateKernelInfo(kernel: nb.IKernel): Promise<void> {
if (kernel) { if (kernel) {
try { try {
@@ -912,6 +907,10 @@ export class NotebookModel extends Disposable implements INotebookModel {
if (alwaysReturnId || (!this._oldKernel || this._oldKernel.name !== standardKernel.name)) { if (alwaysReturnId || (!this._oldKernel || this._oldKernel.name !== standardKernel.name)) {
return providerId; return providerId;
} }
} else {
if (this.notebookManagers?.length) {
return this.notebookManagers.map(m => m.providerId).find(p => p !== DEFAULT_NOTEBOOK_PROVIDER && p !== SQL_NOTEBOOK_PROVIDER);
}
} }
return undefined; return undefined;
} }