Skip to content

Commit

Permalink
fix: query blank issue (#1438)
Browse files Browse the repository at this point in the history
* fix: query blank issue

* fix: send telemetry event

* chore: cleanup

* fix: review comments

* storybook crypto

* fix: catch only wasm error

---------

Co-authored-by: anandgupta42 <[email protected]>
  • Loading branch information
saravmajestic and anandgupta42 authored Oct 18, 2024
1 parent cd0ca48 commit 85c423f
Show file tree
Hide file tree
Showing 17 changed files with 298 additions and 111 deletions.
9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@
"description": "Enable AltimateAI teammates feature.",
"default": false
},
"dbt.enableQueryBookmarks": {
"dbt.disableQueryHistory": {
"type": "boolean",
"description": "Enable Query history and bookmarks feature",
"default": true
"description": "Disable Query history and bookmarks",
"default": false
},
"dbt.enableNotebooks": {
"type": "boolean",
Expand Down Expand Up @@ -701,8 +701,7 @@
"menus": {
"file/newFile": [
{
"command": "dbtPowerUser.createSqlFile",
"when": "dbt.enableQueryBookmarks == true"
"command": "dbtPowerUser.createSqlFile"
},
{
"command": "dbtPowerUser.createDatapilotNotebook",
Expand Down
2 changes: 2 additions & 0 deletions src/telemetry/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,6 @@ export enum TelemetryEvents {
"Notebook/Launch" = "Notebook/Launch",
"Notebook/LaunchError" = "Notebook/LaunchError",
"Notebook/StoreDataInKernelError" = "Notebook/StoreDataInKernelError",
"QueryHistory/Disabled" = "QueryHistory/Disabled",
"QueryHistory/Cleared" = "QueryHistory/Cleared",
}
18 changes: 18 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,21 @@ export function getFormattedDateTime(): string {

return `${date}-${time}`;
}

export const getStringSizeInMb = (str: string): number => {
let sizeInBytes = 0;
for (let i = 0; i < str.length; i++) {
const charCode = str.charCodeAt(i);
if (charCode <= 0x7f) {
sizeInBytes += 1;
} else if (charCode <= 0x7ff) {
sizeInBytes += 2;
} else if (charCode <= 0xffff) {
sizeInBytes += 3;
} else {
sizeInBytes += 4;
}
}
const sizeInMB = sizeInBytes / (1024 * 1024);
return sizeInMB;
};
90 changes: 49 additions & 41 deletions src/webview_provider/queryResultPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import {
workspace,
} from "vscode";

import { readFileSync } from "fs";
import { PythonException } from "python-bridge";
import { DBTProjectContainer } from "../manifest/dbtProjectContainer";
import {
extendErrorWithSupportLinks,
getFormattedDateTime,
getStringSizeInMb,
provideSingleton,
} from "../utils";
import { TelemetryService } from "../telemetry";
Expand All @@ -38,6 +38,7 @@ import {
import { DBTTerminal } from "../dbt_client/dbtTerminal";
import { QueryManifestService } from "../services/queryManifestService";
import { UsersService } from "../services/usersService";
import { TelemetryEvents } from "../telemetry/events";

interface JsonObj {
[key: string]: string | number | undefined;
Expand Down Expand Up @@ -94,6 +95,7 @@ enum InboundCommand {
RunAdhocQuery = "runAdhocQuery",
ViewResultSet = "viewResultSet",
OpenCodeInEditor = "openCodeInEditor",
ClearQueryHistory = "clearQueryHistory",
}

interface RecInfo {
Expand Down Expand Up @@ -124,7 +126,7 @@ interface QueryHistory {
duration: number;
adapter: string;
projectName: string;
data: JsonObj[];
data?: JsonObj[];
columnNames: string[];
columnTypes: string[];
modelName: string;
Expand Down Expand Up @@ -165,13 +167,11 @@ export class QueryResultPanel extends AltimateWebviewProvider {
this._disposables.push(
workspace.onDidChangeConfiguration(
(e) => {
if (e.affectsConfiguration("dbt.enableQueryBookmarks")) {
this.updateEnableBookmarksInContext();
if (e.affectsConfiguration("dbt.disableQueryHistory")) {
if (this._panel) {
this.renderWebviewView(this._panel.webview);
}
}

if (e.affectsConfiguration("dbt.enableNotebooks")) {
this.updateEnableNotebooksInContext();
const event = workspace
Expand All @@ -187,7 +187,6 @@ export class QueryResultPanel extends AltimateWebviewProvider {
),
);

this.updateEnableBookmarksInContext();
this.updateEnableNotebooksInContext();
this._disposables.push(
commands.registerCommand(
Expand All @@ -205,17 +204,6 @@ export class QueryResultPanel extends AltimateWebviewProvider {
});
}

private updateEnableBookmarksInContext() {
// Setting this here to access it in package.json for enabling new file command
commands.executeCommand(
"setContext",
"dbt.enableQueryBookmarks",
workspace
.getConfiguration("dbt")
.get<boolean>("enableQueryBookmarks", false),
);
}

private updateEnableNotebooksInContext() {
// Setting this here to access it in package.json for enabling new file command
commands.executeCommand(
Expand Down Expand Up @@ -378,6 +366,20 @@ export class QueryResultPanel extends AltimateWebviewProvider {
this._panel!.webview.onDidReceiveMessage(
async (message) => {
switch (message.command) {
// Incase of error in rendering perspective viewer query results, user can click button
// to disable query history and retry
case InboundCommand.ClearQueryHistory:
this.telemetry.sendTelemetryError(
TelemetryEvents["QueryHistory/Cleared"],
message.error,
);
this._queryHistory = [];
this.sendResponseToWebview({
command: "response",
data: {},
syncRequestId: message.syncRequestId,
});
break;
case InboundCommand.OpenCodeInEditor:
this.handleOpenCodeInEditor(message);
break;
Expand Down Expand Up @@ -430,22 +432,24 @@ export class QueryResultPanel extends AltimateWebviewProvider {
}
break;
case InboundCommand.GetQueryPanelContext:
const perspectiveTheme = workspace
.getConfiguration("dbt")
.get("perspectiveTheme", "Vintage");
const queryBookmarksEnabled = workspace
.getConfiguration("dbt")
.get("enableQueryBookmarks", false);
{
const perspectiveTheme = workspace
.getConfiguration("dbt")
.get("perspectiveTheme", "Vintage");
const queryHistoryDisabled = workspace
.getConfiguration("dbt")
.get("disableQueryHistory", false);

const limit = workspace
.getConfiguration("dbt")
.get<number>("queryLimit");
await this._panel!.webview.postMessage({
command: OutboundCommand.GetContext,
limit,
perspectiveTheme,
queryBookmarksEnabled,
});
const limit = workspace
.getConfiguration("dbt")
.get<number>("queryLimit");
await this._panel!.webview.postMessage({
command: OutboundCommand.GetContext,
limit,
perspectiveTheme,
queryHistoryDisabled,
});
}
break;
case InboundCommand.CancelQuery:
if (this.queryExecution) {
Expand Down Expand Up @@ -665,8 +669,8 @@ export class QueryResultPanel extends AltimateWebviewProvider {
duration: number,
modelName: string,
) {
// Do not update query history if bookmarks are disabled
if (!workspace.getConfiguration("dbt").get("enableQueryBookmarks", false)) {
// Do not update query history if disabled
if (workspace.getConfiguration("dbt").get("disableQueryHistory", false)) {
return;
}
const project = projectName
Expand All @@ -679,6 +683,17 @@ export class QueryResultPanel extends AltimateWebviewProvider {
);
return;
}
const queryHistoryCurrentSize = getStringSizeInMb(
JSON.stringify(this._queryHistory),
);
// if current history size > 3MB, remove the oldest entry
if (queryHistoryCurrentSize > 3) {
this._queryHistory.pop();
this.dbtTerminal.info(
"updateQueryHistory",
"Query history size exceeded 3MB, cleared oldest entry",
);
}
this._queryHistory.unshift({
rawSql: query,
compiledSql: result.compiled_sql,
Expand All @@ -692,13 +707,6 @@ export class QueryResultPanel extends AltimateWebviewProvider {
modelName,
});
this._queryHistory = this._queryHistory.splice(0, 10);
this._bottomPanel?.webview.postMessage({
command: "queryHistory",
args: {
body: this._queryHistory,
status: true,
},
});
}

/** Runs a query transmitting appropriate notifications to webview */
Expand Down
10 changes: 2 additions & 8 deletions webview_panels/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
"plugin:you-dont-need-lodash-underscore/compatible",
"plugin:storybook/recommended",
],
ignorePatterns: ["dist", ".eslintrc.cjs"],
ignorePatterns: ["dist", ".eslintrc.cjs", "src/lib/altimate/*"],
parser: "@typescript-eslint/parser",
settings: {
react: {
Expand Down Expand Up @@ -107,12 +107,6 @@ module.exports = {
},
],
},
},
{
"files": ["src/lib/altimate/altimate-components.d.ts"],
"rules": {
"@typescript-eslint/naming-convention": "off"
}
}
}
],
};
73 changes: 73 additions & 0 deletions webview_panels/src/assets/icons/error.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions webview_panels/src/assets/icons/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export { default as NoHistoryIcon } from "./no-history.svg?react";
export { default as NoNotebooksIcon } from "./notebook.svg?react";
export { default as ThinkingIcon } from "./thinking.svg?react";
export { default as CoachAIIcon } from "./coachAi.svg?react";
export { default as ErrorIcon } from "./error.svg?react";
import LoadingSpinnerUrl from "./spinner.gif";
import "./styles.css";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const DefaultQueryPanelView = {
if (request.command === "getQueryPanelContext") {
window.postMessage({
command: "getContext",
queryBookmarksEnabled: true,
queryHistoryDisabled: false,
});

window.postMessage({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import PreTag from "@modules/markdown/PreTag";
import { QueryPanelTitleTabState } from "./types";
import QueryPanelHistory from "../queryPanelQueryHistory/QueryPanelHistory";
import QueryPanelBookmarks from "../queryPanelBookmarks/QueryPanelBookmarks";
import PerspectiveErrorBoundary from "../perspective/PerspectiveErrorBoundary";

const QueryPanelContent = ({
tabState,
Expand Down Expand Up @@ -41,11 +42,13 @@ const QueryPanelContent = ({

if (queryResults) {
return (
<PerspectiveErrorBoundary>
<PerspectiveViewer
data={queryResults.data}
columnNames={queryResults.columnNames}
columnTypes={queryResults.columnTypes}
/>
</PerspectiveErrorBoundary>
);
}

Expand Down
Loading

0 comments on commit 85c423f

Please sign in to comment.