Fix #3937 Create new notebook (Mac) and receive TypeError (#3965)

- Handles empty file scenario, with fixes along the way for missing metadata (bonus win)
- In non-empty file still shows error and kernel stuck in loading state. #3964 opened to track this issue and fix later
This commit is contained in:
Kevin Cunnane
2019-02-07 10:35:19 -08:00
committed by GitHub
parent 40e0d5cfbf
commit 69dff5a2cb
3 changed files with 49 additions and 23 deletions

View File

@@ -523,7 +523,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
// Get default language if saved in notebook file
// Otherwise, default to python
private getDefaultLanguageInfo(notebook: nb.INotebookContents): nb.ILanguageInfo {
return notebook!.metadata!.language_info || {
return (notebook && notebook.metadata && notebook.metadata.language_info) ? notebook.metadata.language_info : {
name: this._providerId === SQL_NOTEBOOK_PROVIDER ? 'sql' : 'python',
version: '',
mimetype: this._providerId === SQL_NOTEBOOK_PROVIDER ? 'x-sql' : 'x-python'
@@ -532,7 +532,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
// Get default kernel info if saved in notebook file
private getSavedKernelInfo(notebook: nb.INotebookContents): nb.IKernelInfo {
return notebook!.metadata!.kernelspec;
return (notebook && notebook.metadata && notebook.metadata.kernelspec) ? notebook.metadata.kernelspec : undefined;
}
private getKernelSpecFromDisplayName(displayName: string): nb.IKernelSpec {
@@ -645,28 +645,39 @@ export class NotebookModel extends Disposable implements INotebookModel {
* @param kernelSpec KernelSpec for new kernel
*/
private setProviderIdForKernel(kernelSpec: nb.IKernelSpec): void {
let sessionManagerFound: boolean = false;
for (let i = 0; i < this.notebookManagers.length; i++) {
if (this.notebookManagers[i].sessionManager && this.notebookManagers[i].sessionManager.specs && this.notebookManagers[i].sessionManager.specs.kernels) {
let index = this.notebookManagers[i].sessionManager.specs.kernels.findIndex(kernel => kernel.name === kernelSpec.name);
if (index >= 0) {
this._activeClientSession = this._clientSessions[i];
if (this.notebookManagers[i].providerId !== this._providerId) {
this._providerId = this.notebookManagers[i].providerId;
this._onProviderIdChanged.fire(this._providerId);
if (!kernelSpec) {
// Just use the 1st non-default provider, we don't have a better heuristic
let notebookManagers = this.notebookOptions.notebookManagers.filter(manager => manager.providerId !== DEFAULT_NOTEBOOK_PROVIDER);
if (!notebookManagers.length) {
notebookManagers = this.notebookOptions.notebookManagers;
}
if (notebookManagers.length > 0) {
this._providerId = notebookManagers[0].providerId;
}
} else {
let sessionManagerFound: boolean = false;
for (let i = 0; i < this.notebookManagers.length; i++) {
if (this.notebookManagers[i].sessionManager && this.notebookManagers[i].sessionManager.specs && this.notebookManagers[i].sessionManager.specs.kernels) {
let index = this.notebookManagers[i].sessionManager.specs.kernels.findIndex(kernel => kernel.name === kernelSpec.name);
if (index >= 0) {
this._activeClientSession = this._clientSessions[i];
if (this.notebookManagers[i].providerId !== this._providerId) {
this._providerId = this.notebookManagers[i].providerId;
this._onProviderIdChanged.fire(this._providerId);
}
sessionManagerFound = true;
break;
}
sessionManagerFound = true;
break;
}
}
}
// If no SessionManager exists, utilize passed in StandardKernels to see if we can intelligently set _providerId
if (!sessionManagerFound) {
let provider = this._kernelDisplayNameToNotebookProviderIds.get(kernelSpec.display_name);
if (provider) {
this._providerId = provider;
this._onProviderIdChanged.fire(this._providerId);
// If no SessionManager exists, utilize passed in StandardKernels to see if we can intelligently set _providerId
if (!sessionManagerFound) {
let provider = this._kernelDisplayNameToNotebookProviderIds.get(kernelSpec.display_name);
if (provider) {
this._providerId = provider;
this._onProviderIdChanged.fire(this._providerId);
}
}
}
}

View File

@@ -101,7 +101,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
}
private updateProfile(): void {
this.profile = this.notebookParams!.profile;
this.profile = this.notebookParams ? this.notebookParams.profile : undefined;
if (!this.profile) {
// use global connection if possible
let profile = TaskUtilities.getCurrentGlobalConnection(this.objectExplorerService, this.connectionManagementService, this.editorService);

View File

@@ -17,6 +17,7 @@ import { localize } from 'vs/nls';
import { JSONObject } from 'sql/parts/notebook/models/jsonext';
import { OutputTypes } from 'sql/parts/notebook/models/contracts';
import { nbversion } from 'sql/parts/notebook/notebookConstants';
import { nbformat } from 'sql/parts/notebook/models/nbformat';
type MimeBundle = { [key: string]: string | string[] | undefined };
@@ -29,7 +30,8 @@ export class LocalContentManager implements nb.ContentManager {
let path = notebookUri.fsPath;
// Note: intentionally letting caller handle exceptions
let notebookFileBuffer = await pfs.readFile(path);
let contents: JSONObject = json.parse(notebookFileBuffer.toString());
let stringContents = notebookFileBuffer.toString();
let contents: JSONObject = json.parse(stringContents);
if (contents) {
if (contents.nbformat === 4) {
@@ -40,9 +42,13 @@ export class LocalContentManager implements nb.ContentManager {
if (contents.nbformat) {
throw new TypeError(localize('nbformatNotRecognized', 'nbformat v{0}.{1} not recognized', contents.nbformat, contents.nbformat_minor));
}
} else if (stringContents === '' || stringContents === undefined) {
// Empty?
return v4.createEmptyNotebook();
}
// else, fallthrough condition
throw new TypeError(localize('nbNotSupported', 'This notebook format is not supported'));
throw new TypeError(localize('nbNotSupported', 'This file does not have a valid notebook format'));
}
@@ -72,6 +78,15 @@ namespace v4 {
return notebook;
}
export function createEmptyNotebook(): nb.INotebookContents {
return {
cells: [],
metadata: undefined,
nbformat: nbformat.MAJOR_VERSION,
nbformat_minor: nbformat.MINOR_VERSION
};
}
function readCell(cell: nb.ICellContents): nb.ICellContents {
switch (cell.cell_type) {
case 'markdown':