From 0205d0afb53d549bb40dc6bf4216a71c0a7c05f9 Mon Sep 17 00:00:00 2001 From: Chris LaFreniere <40371649+chlafreniere@users.noreply.github.com> Date: Tue, 19 Feb 2019 15:01:47 -1000 Subject: [PATCH] Change default max table rows returned in notebook to 5000, make it user configurable (#4084) --- extensions/notebook/package.json | 5 ++++ extensions/notebook/package.nls.json | 3 ++- .../notebook/common/sqlSessionManager.ts | 24 ++++++++++++++----- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index f317acde2c..12e6966c85 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -31,6 +31,11 @@ "type": "boolean", "default": true, "description": "%notebook.overrideEditorTheming.description%" + }, + "notebook.maxTableRows": { + "type": "number", + "default": 5000, + "description": "%notebook.maxTableRows.description%" } } }, diff --git a/extensions/notebook/package.nls.json b/extensions/notebook/package.nls.json index b362246006..83e6df9a17 100644 --- a/extensions/notebook/package.nls.json +++ b/extensions/notebook/package.nls.json @@ -3,8 +3,9 @@ "description": "Defines the Data-procotol based Notebook contribution and many Notebook commands and contributions.", "notebook.configuration.title": "Notebook configuration", "notebook.pythonPath.description": "Local path to python installation used by Notebooks.", - "notebook.sqlKernelEnabled.description": "Enable SQL kernel in notebook editor (Preview). Requires reloading this window to take effect", + "notebook.sqlKernelEnabled.description": "Enable SQL kernel in Notebook editor (Preview). Requires reloading this window to take effect", "notebook.overrideEditorTheming.description": "Override editor default settings in the Notebook editor. Settings include background color, current line color and border", + "notebook.maxTableRows.description": "Maximum number of rows returned per table in the Notebook editor", "notebook.command.new": "New Notebook", "notebook.command.open": "Open Notebook", "notebook.analyzeJupyterNotebook": "Analyze in Notebook", diff --git a/src/sql/workbench/services/notebook/common/sqlSessionManager.ts b/src/sql/workbench/services/notebook/common/sqlSessionManager.ts index 9b67a4bf09..6d16fa5f83 100644 --- a/src/sql/workbench/services/notebook/common/sqlSessionManager.ts +++ b/src/sql/workbench/services/notebook/common/sqlSessionManager.ts @@ -13,15 +13,18 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import Severity from 'vs/base/common/severity'; import * as Utils from 'sql/platform/connection/common/utils'; import { Deferred } from 'sql/base/common/promise'; -import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable } from 'vs/base/common/lifecycle'; import { IErrorMessageService } from 'sql/platform/errorMessage/common/errorMessageService'; import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile'; import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; import { escape } from 'sql/base/common/strings'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; export const sqlKernel: string = localize('sqlKernel', 'SQL'); export const sqlKernelError: string = localize("sqlKernelError", "SQL kernel error"); -export const MAX_ROWS = 2000; +export const MAX_ROWS = 5000; +export const NotebookConfigSectionName = 'notebook'; +export const MaxTableRowsConfigName = 'maxTableRows'; let sqlKernelSpec: nb.IKernelSpec = ({ name: sqlKernel, @@ -135,7 +138,8 @@ class SqlKernel extends Disposable implements nb.IKernel { constructor( @IConnectionManagementService private _connectionManagementService: IConnectionManagementService, @IInstantiationService private _instantiationService: IInstantiationService, - @IErrorMessageService private _errorMessageService: IErrorMessageService) { + @IErrorMessageService private _errorMessageService: IErrorMessageService, + @IConfigurationService private _configurationService: IConfigurationService) { super(); } @@ -217,7 +221,7 @@ class SqlKernel extends Disposable implements nb.IKernel { // TODO verify this is "canonical" behavior let count = canRun ? ++this._executionCount : undefined; - this._future = new SQLFuture(this._queryRunner, count); + this._future = new SQLFuture(this._queryRunner, count, this._configurationService); if (!canRun) { // Complete early this._future.handleDone(new Error(localize('connectionRequired', 'A connection must be chosen to run notebook cells'))); @@ -270,9 +274,17 @@ export class SQLFuture extends Disposable implements FutureInternal { private ioHandler: nb.MessageHandler; private doneHandler: nb.MessageHandler; private doneDeferred = new Deferred(); + private configuredMaxRows: number = MAX_ROWS; - constructor(private _queryRunner: QueryRunner, private _executionCount: number | undefined) { + constructor(private _queryRunner: QueryRunner, private _executionCount: number | undefined, private configurationService: IConfigurationService) { super(); + let config = configurationService.getValue(NotebookConfigSectionName); + if (config) { + let maxRows = config[MaxTableRowsConfigName] ? config[MaxTableRowsConfigName] : undefined; + if (maxRows && maxRows > 0) { + this.configuredMaxRows = maxRows; + } + } } get inProgress(): boolean { @@ -337,7 +349,7 @@ export class SQLFuture extends Disposable implements FutureInternal { public handleBatchEnd(batch: BatchSummary): void { if (this.ioHandler) { for (let resultSet of batch.resultSetSummaries) { - let rowCount = resultSet.rowCount > MAX_ROWS ? MAX_ROWS : resultSet.rowCount; + let rowCount = resultSet.rowCount > this.configuredMaxRows ? this.configuredMaxRows : resultSet.rowCount; this._queryRunner.getQueryRows(0, rowCount, resultSet.batchId, resultSet.id).then(d => { let msg: nb.IIOPubMessage = {