From 83a6ee0a22df4b0d342fe9549859a5aab10673ab Mon Sep 17 00:00:00 2001 From: Chris LaFreniere <40371649+chlafreniere@users.noreply.github.com> Date: Wed, 30 Jan 2019 17:29:08 -0800 Subject: [PATCH] Change feature flag for SQL kernel to be user preference (#3838) * Change feature flag for SQL kernel to be user preference * fix test that was broken * Tweak package.nls.json to show "(Preview)" --- extensions/notebook/package.json | 5 +++++ extensions/notebook/package.nls.json | 1 + src/sql/parts/notebook/notebook.component.ts | 4 ++-- src/sql/parts/notebook/notebookUtils.ts | 7 ++++--- .../services/notebook/common/notebookServiceImpl.ts | 6 ++++-- src/sqltest/workbench/api/mainThreadNotebook.test.ts | 3 ++- 6 files changed, 18 insertions(+), 8 deletions(-) diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index e883fe6acc..272d13f4e4 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -26,6 +26,11 @@ "type": "string", "default": "", "description": "%notebook.pythonPath.description%" + }, + "notebook.sqlKernelEnabled": { + "type": "boolean", + "default": false, + "description": "%notebook.sqlKernelEnabled.description%" } } }, diff --git a/extensions/notebook/package.nls.json b/extensions/notebook/package.nls.json index f3453c39d1..00117d89c0 100644 --- a/extensions/notebook/package.nls.json +++ b/extensions/notebook/package.nls.json @@ -4,6 +4,7 @@ "notebook.configuration.title": "Notebook configuration", "notebook.enabled.description": "Enable viewing notebook files using built-in notebook editor.", "notebook.pythonPath.description": "Local path to python installation used by Notebooks.", + "notebook.sqlKernelEnabled.description": "Enable SQL kernel in notebook editor (Preview)", "notebook.command.new": "New Notebook", "notebook.command.open": "Open Notebook" } \ No newline at end of file diff --git a/src/sql/parts/notebook/notebook.component.ts b/src/sql/parts/notebook/notebook.component.ts index 19d22e6768..95b218f3f0 100644 --- a/src/sql/parts/notebook/notebook.component.ts +++ b/src/sql/parts/notebook/notebook.component.ts @@ -234,7 +234,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe private async loadModel(): Promise { await this.awaitNonDefaultProvider(); - let providerId = notebookUtils.sqlNotebooksEnabled() ? 'sql' : this._notebookParams.providers.find(provider => provider !== DEFAULT_NOTEBOOK_PROVIDER); // this is tricky; really should also depend on the connection profile + let providerId = notebookUtils.sqlNotebooksEnabled(this.contextKeyService) ? 'sql' : this._notebookParams.providers.find(provider => provider !== DEFAULT_NOTEBOOK_PROVIDER); // this is tricky; really should also depend on the connection profile this.setContextKeyServiceWithProviderId(providerId); this.fillInActionsForCurrentContext(); for (let providerId of this._notebookParams.providers) { @@ -248,7 +248,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe notificationService: this.notificationService, notebookManagers: this.notebookManagers, standardKernels: this._notebookParams.input.standardKernels, - providerId: notebookUtils.sqlNotebooksEnabled() ? 'sql' : 'jupyter', // this is tricky; really should also depend on the connection profile + providerId: notebookUtils.sqlNotebooksEnabled(this.contextKeyService) ? 'sql' : 'jupyter', // this is tricky; really should also depend on the connection profile defaultKernel: this._notebookParams.input.defaultKernel, capabilitiesService: this.capabilitiesService }, false, this.profile); diff --git a/src/sql/parts/notebook/notebookUtils.ts b/src/sql/parts/notebook/notebookUtils.ts index 3a63bd68a3..1f7dab4f8e 100644 --- a/src/sql/parts/notebook/notebookUtils.ts +++ b/src/sql/parts/notebook/notebookUtils.ts @@ -12,6 +12,7 @@ import * as pfs from 'vs/base/node/pfs'; import { localize } from 'vs/nls'; import { IOutputChannel } from 'vs/workbench/parts/output/common/output'; import { DEFAULT_NOTEBOOK_PROVIDER, DEFAULT_NOTEBOOK_FILETYPE, INotebookService } from 'sql/workbench/services/notebook/common/notebookService'; +import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; /** @@ -73,9 +74,9 @@ export function getStandardKernelsForProvider(providerId: string, notebookServic return (standardKernels); } -// Private feature flag to enable Sql Notebook experience -export function sqlNotebooksEnabled() { - return process.env['SQLOPS_SQL_NOTEBOOK'] !== undefined; +// Feature flag to enable Sql Notebook experience +export function sqlNotebooksEnabled(contextKeyService: IContextKeyService) { + return contextKeyService.contextMatchesRules(ContextKeyExpr.equals('config.notebook.sqlKernelEnabled', true)); } export interface IStandardKernelWithProvider { diff --git a/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts index a0c7c2086c..ae1457085f 100644 --- a/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts @@ -30,6 +30,7 @@ import { Deferred } from 'sql/base/common/promise'; import { SqlSessionManager } from 'sql/workbench/services/notebook/common/sqlSessionManager'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { sqlNotebooksEnabled } from 'sql/parts/notebook/notebookUtils'; +import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; export interface NotebookProviderProperties { provider: string; @@ -88,7 +89,8 @@ export class NotebookService extends Disposable implements INotebookService { @IStorageService private _storageService: IStorageService, @IExtensionService extensionService: IExtensionService, @IExtensionManagementService extensionManagementService: IExtensionManagementService, - @IInstantiationService private _instantiationService: IInstantiationService + @IInstantiationService private _instantiationService: IInstantiationService, + @IContextKeyService private _contextKeyService: IContextKeyService ) { super(); this._register(notebookRegistry.onNewRegistration(this.updateRegisteredProviders, this)); @@ -351,7 +353,7 @@ export class NotebookService extends Disposable implements INotebookService { } private registerBuiltInProvider() { - if (!sqlNotebooksEnabled()) { + if (!sqlNotebooksEnabled(this._contextKeyService)) { let defaultProvider = new BuiltinProvider(); this.registerProvider(defaultProvider.providerId, defaultProvider); notebookRegistry.registerNotebookProvider({ diff --git a/src/sqltest/workbench/api/mainThreadNotebook.test.ts b/src/sqltest/workbench/api/mainThreadNotebook.test.ts index 9cdc8d150a..8ed3472498 100644 --- a/src/sqltest/workbench/api/mainThreadNotebook.test.ts +++ b/src/sqltest/workbench/api/mainThreadNotebook.test.ts @@ -18,6 +18,7 @@ import { NotebookService } from 'sql/workbench/services/notebook/common/notebook import { INotebookProvider } from 'sql/workbench/services/notebook/common/notebookService'; import { INotebookManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; import { LocalContentManager } from 'sql/workbench/services/notebook/node/localContentManager'; +import { ContextKeyServiceStub } from 'sqltest/stubs/contextKeyServiceStub'; suite('MainThreadNotebook Tests', () => { @@ -31,7 +32,7 @@ suite('MainThreadNotebook Tests', () => { let extContext = { getProxy: proxyType => mockProxy.object }; - mockNotebookService = TypeMoq.Mock.ofType(NotebookService); + mockNotebookService = TypeMoq.Mock.ofType(NotebookService, undefined, undefined, undefined, undefined, undefined, new ContextKeyServiceStub()); notebookUri = URI.parse('file:/user/default/my.ipynb'); mainThreadNotebook = new MainThreadNotebook(extContext, mockNotebookService.object); });