SQL Kernel Improvements/Removing Spark Code from Core/Attach to Changes (#3790)

* Scenarios work besides loading saved kernel

* Fix compilation issue

* Save and load functional

* Fix loading kernesl issue when sql kernel is not enabled

* Fix language mapping to not be hardcoded any longer

* Remove unnecessary comment

* PR Comments vol. 1

* Code cleanup, use ConnectionProfile instead of IConnectionProfile when accessing serverName

* PR changes vol. 2

* One final comment for PR

* Fix linting issue
This commit is contained in:
Chris LaFreniere
2019-01-25 18:54:04 -08:00
committed by GitHub
parent ea67859de7
commit 43be88a37c
29 changed files with 768 additions and 627 deletions

View File

@@ -47,36 +47,42 @@ export class NotebookModelStub implements INotebookModel {
get contentChanged(): Event<NotebookContentChange> {
throw new Error('method not implemented.');
}
get specs(): nb.IAllKernels {
throw new Error('method not implemented.');
}
get contexts(): IDefaultConnection {
throw new Error('method not implemented.');
}
get providerId(): string {
throw new Error('method not implemented.');
}
changeKernel(displayName: string): void {
throw new Error('Method not implemented.');
}
changeContext(host: string, connection?: IConnectionProfile): void {
throw new Error('Method not implemented.');
}
findCellIndex(cellModel: ICellModel): number {
throw new Error('Method not implemented.');
}
addCell(cellType: CellType, index?: number): void {
throw new Error('Method not implemented.');
}
deleteCell(cellModel: ICellModel): void {
throw new Error('Method not implemented.');
}
saveModel(): Promise<boolean> {
throw new Error('Method not implemented.');
}
pushEditOperations(edits: ISingleNotebookEditOperation[]): void {
throw new Error('Method not implemented.');
}
get specs(): nb.IAllKernels {
throw new Error('method not implemented.');
}
get contexts(): IDefaultConnection {
throw new Error('method not implemented.');
}
get providerId(): string {
throw new Error('method not implemented.');
}
get applicableConnectionProviderIds(): string[] {
throw new Error('method not implemented.');
}
changeKernel(displayName: string): void {
throw new Error('Method not implemented.');
}
changeContext(host: string, connection?: IConnectionProfile): void {
throw new Error('Method not implemented.');
}
findCellIndex(cellModel: ICellModel): number {
throw new Error('Method not implemented.');
}
addCell(cellType: CellType, index?: number): void {
throw new Error('Method not implemented.');
}
deleteCell(cellModel: ICellModel): void {
throw new Error('Method not implemented.');
}
saveModel(): Promise<boolean> {
throw new Error('Method not implemented.');
}
pushEditOperations(edits: ISingleNotebookEditOperation[]): void {
throw new Error('Method not implemented.');
}
getApplicableConnectionProviderIds(kernelName: string): string[] {
throw new Error('Method not implemented.');
}
}
export class NotebookManagerStub implements INotebookManager {

View File

@@ -25,6 +25,8 @@ import { Deferred } from 'sql/base/common/promise';
import { ConnectionManagementService } from 'sql/platform/connection/common/connectionManagementService';
import { Memento } from 'vs/workbench/common/memento';
import { Emitter } from 'vs/base/common/event';
import { CapabilitiesTestService } from 'sqltest/stubs/capabilitiesTestService';
import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService';
let expectedNotebookContent: nb.INotebookContents = {
cells: [{
@@ -71,182 +73,187 @@ let mockClientSession: TypeMoq.Mock<IClientSession>;
let sessionReady: Deferred<void>;
let mockModelFactory: TypeMoq.Mock<ModelFactory>;
let notificationService: TypeMoq.Mock<INotificationService>;
let capabilitiesService: TypeMoq.Mock<ICapabilitiesService>;
describe('notebook model', function (): void {
let notebookManagers = [new NotebookManagerStub()];
let memento: TypeMoq.Mock<Memento>;
let queryConnectionService: TypeMoq.Mock<ConnectionManagementService>;
let defaultModelOptions: INotebookModelOptions;
beforeEach(() => {
sessionReady = new Deferred<void>();
notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose);
memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, '');
memento.setup(x => x.getMemento(TypeMoq.It.isAny())).returns(() => void 0);
queryConnectionService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, memento.object, undefined);
queryConnectionService.callBase = true;
defaultModelOptions = {
notebookUri: defaultUri,
factory: new ModelFactory(),
notebookManagers,
notificationService: notificationService.object,
connectionService: queryConnectionService.object,
providerId: 'jupyter'
};
mockClientSession = TypeMoq.Mock.ofType(ClientSession, undefined, defaultModelOptions);
mockClientSession.setup(c => c.initialize(TypeMoq.It.isAny())).returns(() => {
return Promise.resolve();
});
mockClientSession.setup(c => c.ready).returns(() => sessionReady.promise);
mockModelFactory = TypeMoq.Mock.ofType(ModelFactory);
mockModelFactory.callBase = true;
mockModelFactory.setup(f => f.createClientSession(TypeMoq.It.isAny())).returns(() => {
return mockClientSession.object;
});
});
describe('notebook model', function(): void {
let notebookManagers = [new NotebookManagerStub()];
let memento: TypeMoq.Mock<Memento>;
let queryConnectionService: TypeMoq.Mock<ConnectionManagementService>;
let defaultModelOptions: INotebookModelOptions;
beforeEach(() => {
sessionReady = new Deferred<void>();
notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose);
capabilitiesService = TypeMoq.Mock.ofType(CapabilitiesTestService);
memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, '');
memento.setup(x => x.getMemento(TypeMoq.It.isAny())).returns(() => void 0);
queryConnectionService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, memento.object, undefined);
queryConnectionService.callBase = true;
defaultModelOptions = {
notebookUri: defaultUri,
factory: new ModelFactory(),
notebookManagers,
notificationService: notificationService.object,
connectionService: queryConnectionService.object,
providerId: 'SQL',
standardKernels: [{ name: 'SQL', connectionProviderIds: ['MSSQL'], notebookProvider: 'sql' }],
defaultKernel: undefined,
capabilitiesService: capabilitiesService.object
};
mockClientSession = TypeMoq.Mock.ofType(ClientSession, undefined, defaultModelOptions);
mockClientSession.setup(c => c.initialize(TypeMoq.It.isAny())).returns(() => {
return Promise.resolve();
});
mockClientSession.setup(c => c.ready).returns(() => sessionReady.promise);
mockModelFactory = TypeMoq.Mock.ofType(ModelFactory);
mockModelFactory.callBase = true;
mockModelFactory.setup(f => f.createClientSession(TypeMoq.It.isAny())).returns(() => {
return mockClientSession.object;
});
});
it('Should create no cells if model has no contents', async function (): Promise<void> {
// Given an empty notebook
let emptyNotebook: nb.INotebookContents = {
cells: [],
metadata: {
kernelspec: {
name: 'mssql',
language: 'sql'
}
},
nbformat: 4,
nbformat_minor: 5
};
it('Should create no cells if model has no contents', async function(): Promise<void> {
// Given an empty notebook
let emptyNotebook: nb.INotebookContents = {
cells: [],
metadata: {
kernelspec: {
name: 'mssql',
language: 'sql'
}
},
nbformat: 4,
nbformat_minor: 5
};
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(emptyNotebook));
notebookManagers[0].contentManager = mockContentManager.object;
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(emptyNotebook));
notebookManagers[0].contentManager = mockContentManager.object;
// When I initialize the model
let model = new NotebookModel(defaultModelOptions);
await model.requestModelLoad();
// When I initialize the model
let model = new NotebookModel(defaultModelOptions);
await model.requestModelLoad();
// Then I expect to have 0 code cell as the contents
should(model.cells).have.length(0);
});
// Then I expect to have 0 code cell as the contents
should(model.cells).have.length(0);
});
it('Should throw if model load fails', async function (): Promise<void> {
// Given a call to get Contents fails
let error = new Error('File not found');
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).throws(error);
notebookManagers[0].contentManager = mockContentManager.object;
it('Should throw if model load fails', async function(): Promise<void> {
// Given a call to get Contents fails
let error = new Error('File not found');
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).throws(error);
notebookManagers[0].contentManager = mockContentManager.object;
// When I initalize the model
// Then it should throw
let model = new NotebookModel(defaultModelOptions);
should(model.inErrorState).be.false();
await testUtils.assertThrowsAsync(() => model.requestModelLoad(), error.message);
should(model.inErrorState).be.true();
});
// When I initalize the model
// Then it should throw
let model = new NotebookModel(defaultModelOptions);
should(model.inErrorState).be.false();
await testUtils.assertThrowsAsync(() => model.requestModelLoad(), error.message);
should(model.inErrorState).be.true();
});
it('Should convert cell info to CellModels', async function (): Promise<void> {
// Given a notebook with 2 cells
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedNotebookContent));
notebookManagers[0].contentManager = mockContentManager.object;
it('Should convert cell info to CellModels', async function(): Promise<void> {
// Given a notebook with 2 cells
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedNotebookContent));
notebookManagers[0].contentManager = mockContentManager.object;
// When I initalize the model
let model = new NotebookModel(defaultModelOptions);
await model.requestModelLoad();
// When I initalize the model
let model = new NotebookModel(defaultModelOptions);
await model.requestModelLoad();
// Then I expect all cells to be in the model
should(model.cells).have.length(2);
should(model.cells[0].source).be.equal(expectedNotebookContent.cells[0].source);
should(model.cells[1].source).be.equal(expectedNotebookContent.cells[1].source);
});
// Then I expect all cells to be in the model
should(model.cells).have.length(2);
should(model.cells[0].source).be.equal(expectedNotebookContent.cells[0].source);
should(model.cells[1].source).be.equal(expectedNotebookContent.cells[1].source);
});
it('Should load contents but then go to error state if client session startup fails', async function (): Promise<void> {
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedNotebookContentOneCell));
notebookManagers[0].contentManager = mockContentManager.object;
it('Should load contents but then go to error state if client session startup fails', async function(): Promise<void> {
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedNotebookContentOneCell));
notebookManagers[0].contentManager = mockContentManager.object;
// Given I have a session that fails to start
mockClientSession.setup(c => c.isInErrorState).returns(() => true);
mockClientSession.setup(c => c.errorMessage).returns(() => 'Error');
sessionReady.resolve();
let sessionFired = false;
// Given I have a session that fails to start
mockClientSession.setup(c => c.isInErrorState).returns(() => true);
mockClientSession.setup(c => c.errorMessage).returns(() => 'Error');
sessionReady.resolve();
let sessionFired = false;
let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, <Partial<INotebookModelOptions>>{
factory: mockModelFactory.object
});
let model = new NotebookModel(options);
model.onClientSessionReady((session) => sessionFired = true);
await model.requestModelLoad();
model.backgroundStartSession();
let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, <Partial<INotebookModelOptions>> {
factory: mockModelFactory.object
});
let model = new NotebookModel(options);
model.onClientSessionReady((session) => sessionFired = true);
await model.requestModelLoad();
model.backgroundStartSession();
// Then I expect load to succeed
shouldHaveOneCell(model);
should(model.clientSession).not.be.undefined();
// but on server load completion I expect error state to be set
// Note: do not expect serverLoad event to throw even if failed
await model.sessionLoadFinished;
should(model.inErrorState).be.true();
should(sessionFired).be.false();
});
// Then I expect load to succeed
shouldHaveOneCell(model);
should(model.clientSession).not.be.undefined();
// but on server load completion I expect error state to be set
// Note: do not expect serverLoad event to throw even if failed
await model.sessionLoadFinished;
should(model.inErrorState).be.true();
should(sessionFired).be.false();
});
it('Should not be in error state if client session initialization succeeds', async function (): Promise<void> {
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedNotebookContentOneCell));
notebookManagers[0].contentManager = mockContentManager.object;
let kernelChangedEmitter: Emitter<nb.IKernelChangedArgs> = new Emitter<nb.IKernelChangedArgs>();
it('Should not be in error state if client session initialization succeeds', async function(): Promise<void> {
let mockContentManager = TypeMoq.Mock.ofType(LocalContentManager);
mockContentManager.setup(c => c.getNotebookContents(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedNotebookContentOneCell));
notebookManagers[0].contentManager = mockContentManager.object;
let kernelChangedEmitter: Emitter<nb.IKernelChangedArgs> = new Emitter<nb.IKernelChangedArgs>();
mockClientSession.setup(c => c.isInErrorState).returns(() => false);
mockClientSession.setup(c => c.isReady).returns(() => true);
mockClientSession.setup(c => c.kernelChanged).returns(() => kernelChangedEmitter.event);
mockClientSession.setup(c => c.isInErrorState).returns(() => false);
mockClientSession.setup(c => c.isReady).returns(() => true);
mockClientSession.setup(c => c.kernelChanged).returns(() => kernelChangedEmitter.event);
queryConnectionService.setup(c => c.getActiveConnections(TypeMoq.It.isAny())).returns(() => null);
queryConnectionService.setup(c => c.getActiveConnections(TypeMoq.It.isAny())).returns(() => null);
sessionReady.resolve();
let actualSession: IClientSession = undefined;
sessionReady.resolve();
let actualSession: IClientSession = undefined;
let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, <Partial<INotebookModelOptions>>{
factory: mockModelFactory.object
});
let model = new NotebookModel(options, false);
model.onClientSessionReady((session) => actualSession = session);
await model.requestModelLoad();
model.backgroundStartSession();
let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, <Partial<INotebookModelOptions>> {
factory: mockModelFactory.object
});
let model = new NotebookModel(options, false);
model.onClientSessionReady((session) => actualSession = session);
await model.requestModelLoad();
model.backgroundStartSession();
// Then I expect load to succeed
should(model.clientSession).not.be.undefined();
// but on server load completion I expect error state to be set
// Note: do not expect serverLoad event to throw even if failed
let kernelChangedArg: nb.IKernelChangedArgs = undefined;
model.kernelChanged((kernel) => kernelChangedArg = kernel);
await model.sessionLoadFinished;
should(model.inErrorState).be.false();
should(actualSession).equal(mockClientSession.object);
should(model.clientSession).equal(mockClientSession.object);
});
// Then I expect load to succeed
should(model.clientSession).not.be.undefined();
// but on server load completion I expect error state to be set
// Note: do not expect serverLoad event to throw even if failed
let kernelChangedArg: nb.IKernelChangedArgs = undefined;
model.kernelChanged((kernel) => kernelChangedArg = kernel);
await model.sessionLoadFinished;
should(model.inErrorState).be.false();
should(actualSession).equal(mockClientSession.object);
should(model.clientSession).equal(mockClientSession.object);
});
it('Should sanitize kernel display name when IP is included', async function (): Promise<void> {
let model = new NotebookModel(defaultModelOptions);
let displayName = 'PySpark (1.1.1.1)';
let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
should(sanitizedDisplayName).equal('PySpark');
});
it('Should sanitize kernel display name when IP is included', async function(): Promise<void> {
let model = new NotebookModel(defaultModelOptions);
let displayName = 'PySpark (1.1.1.1)';
let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
should(sanitizedDisplayName).equal('PySpark');
});
it('Should sanitize kernel display name properly when IP is not included', async function (): Promise<void> {
let model = new NotebookModel(defaultModelOptions);
let displayName = 'PySpark';
let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
should(sanitizedDisplayName).equal('PySpark');
});
it('Should sanitize kernel display name properly when IP is not included', async function(): Promise<void> {
let model = new NotebookModel(defaultModelOptions);
let displayName = 'PySpark';
let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
should(sanitizedDisplayName).equal('PySpark');
});
function shouldHaveOneCell(model: NotebookModel): void {
should(model.cells).have.length(1);
verifyCellModel(model.cells[0], { cell_type: CellTypes.Code, source: 'insert into t1 values (c1, c2)', metadata: { language: 'python' }, execution_count: 1 });
}
function shouldHaveOneCell(model: NotebookModel): void {
should(model.cells).have.length(1);
verifyCellModel(model.cells[0], { cell_type: CellTypes.Code, source: 'insert into t1 values (c1, c2)', metadata: { language: 'python' }, execution_count: 1 });
}
function verifyCellModel(cellModel: ICellModel, expected: nb.ICellContents): void {
should(cellModel.cellType).equal(expected.cell_type);
should(cellModel.source).equal(expected.source);
}
function verifyCellModel(cellModel: ICellModel, expected: nb.ICellContents): void {
should(cellModel.cellType).equal(expected.cell_type);
should(cellModel.source).equal(expected.source);
}
});

View File

@@ -122,7 +122,7 @@ suite('ExtHostNotebook Tests', () => {
class NotebookProviderStub implements sqlops.nb.NotebookProvider {
providerId: string = 'TestProvider';
standardKernels: string[] = ['fakeKernel'];
standardKernels: sqlops.nb.IStandardKernel[] = [{name: 'fakeKernel', connectionProviderIds: ['MSSQL']}];
getNotebookManager(notebookUri: vscode.Uri): Thenable<sqlops.nb.NotebookManager> {
throw new Error('Method not implemented.');

View File

@@ -149,6 +149,12 @@ class ExtHostNotebookStub implements ExtHostNotebookShape {
$changeKernel(sessionId: number, kernelInfo: sqlops.nb.IKernelSpec): Thenable<INotebookKernelDetails> {
throw new Error('Method not implemented.');
}
$configureKernel(sessionId: number, kernelInfo: sqlops.nb.IKernelSpec): Thenable<void> {
throw new Error('Method not implemented.');
}
$configureConnection(sessionId: number, conneection: sqlops.IConnectionProfile): Thenable<void> {
throw new Error('Method not implemented.');
}
$getKernelReadyStatus(kernelId: number): Thenable<sqlops.nb.IInfoReply> {
throw new Error('Method not implemented.');
}