From 33a37d46babba226007c580a3d290669db32a161 Mon Sep 17 00:00:00 2001 From: Arik Fraimovich Date: Thu, 23 Jan 2020 10:52:20 +0200 Subject: [PATCH 01/28] QueryResult: added support for getting status updates using a callback instead of polling on the query result status. This is a step towards bigger refacotr. We should completly rewrite this class and split it into two: 1. QueryExecution -- executes a query and tracks status. 2. QueryResult -- represents the actual QueryResult and has the different utilities to use the results (although those might be moved into helpers). --- client/app/services/query-result.js | 36 +++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/client/app/services/query-result.js b/client/app/services/query-result.js index 0659227bfdc..65015e32d8a 100644 --- a/client/app/services/query-result.js +++ b/client/app/services/query-result.js @@ -9,7 +9,7 @@ const logger = debug("redash:services:QueryResult"); const filterTypes = ["filter", "multi-filter", "multiFilter"]; function defer() { - const result = {}; + const result = { onStatusChange: status => {} }; result.promise = new Promise((resolve, reject) => { result.resolve = resolve; result.reject = reject; @@ -53,11 +53,19 @@ const QueryResultResource = { post: data => axios.post(createOrSaveUrl(data), data), }; +export const ExecutionStatus = { + WAITING: "waiting", + PROCESSING: "processing", + DONE: "done", + FAILED: "failed", + LOADING_RESULT: "loading-result", +}; + const statuses = { - 1: "waiting", - 2: "processing", - 3: "done", - 4: "failed", + 1: ExecutionStatus.WAITING, + 2: ExecutionStatus.PROCESSING, + 3: ExecutionStatus.DONE, + 4: ExecutionStatus.FAILED, }; function handleErrorResponse(queryResult, response) { @@ -104,7 +112,8 @@ class QueryResult { extend(this, props); if ("query_result" in props) { - this.status = "done"; + this.status = ExecutionStatus.DONE; + this.deferred.onStatusChange(ExecutionStatus.DONE); const columnTypes = {}; @@ -148,12 +157,14 @@ class QueryResult { }); this.deferred.resolve(this); - } else if (this.job.status === 3) { + } else if (this.job.status === 3 || this.job.status === 2) { + this.deferred.onStatusChange(ExecutionStatus.PROCESSING); this.status = "processing"; } else if (this.job.status === 4) { this.status = statuses[this.job.status]; this.deferred.reject(new QueryResultError(this.job.error)); } else { + this.deferred.onStatusChange(undefined); this.status = undefined; } } @@ -172,7 +183,7 @@ class QueryResult { getStatus() { if (this.isLoadingResult) { - return "loading-result"; + return ExecutionStatus.LOADING_RESULT; } return this.status || statuses[this.job.status]; } @@ -290,7 +301,10 @@ class QueryResult { return filters; } - toPromise() { + toPromise(statusCallback) { + if (statusCallback) { + this.deferred.onStatusChange = statusCallback; + } return this.deferred.promise; } @@ -298,6 +312,8 @@ class QueryResult { const queryResult = new QueryResult(); queryResult.isLoadingResult = true; + queryResult.deferred.onStatusChange(ExecutionStatus.LOADING); + axios .get(`api/queries/${queryId}/results/${id}.json`) .then(response => { @@ -327,6 +343,8 @@ class QueryResult { loadResult(tryCount) { this.isLoadingResult = true; + this.deferred.onStatusChange(ExecutionStatus.LOADING); + QueryResultResource.get({ id: this.job.query_result_id }) .then(response => { this.update(response); From 8e52b28f092040d61cb2ea60d93dc9827edee82e Mon Sep 17 00:00:00 2001 From: Arik Fraimovich Date: Thu, 23 Jan 2020 12:33:00 +0200 Subject: [PATCH 02/28] Refactor useQueryExecution. --- client/app/lib/getQueryResultData.js | 16 +++ client/app/lib/hooks/useQueryResult.js | 66 --------- client/app/pages/queries/QuerySource.jsx | 85 ++++++------ client/app/pages/queries/QueryView.jsx | 56 ++++---- .../pages/queries/hooks/useQueryExecute.js | 127 ++++++++++-------- .../EditVisualizationDialog.jsx | 5 +- .../visualizations/VisualizationRenderer.jsx | 4 +- 7 files changed, 157 insertions(+), 202 deletions(-) create mode 100644 client/app/lib/getQueryResultData.js delete mode 100644 client/app/lib/hooks/useQueryResult.js diff --git a/client/app/lib/getQueryResultData.js b/client/app/lib/getQueryResultData.js new file mode 100644 index 00000000000..18e74e7fe99 --- /dev/null +++ b/client/app/lib/getQueryResultData.js @@ -0,0 +1,16 @@ +import { get, invoke } from "lodash"; + +export default function getQueryResultData(queryResult) { + return { + status: invoke(queryResult, "getStatus") || null, + columns: invoke(queryResult, "getColumns") || [], + rows: invoke(queryResult, "getData") || [], + filters: invoke(queryResult, "getFilters") || [], + updatedAt: invoke(queryResult, "getUpdatedAt") || null, + retrievedAt: get(queryResult, "query_result.retrieved_at", null), + log: invoke(queryResult, "getLog") || [], + error: invoke(queryResult, "getError") || null, + runtime: invoke(queryResult, "getRuntime") || null, + metadata: get(queryResult, "query_result.data.metadata", {}), + }; +} diff --git a/client/app/lib/hooks/useQueryResult.js b/client/app/lib/hooks/useQueryResult.js deleted file mode 100644 index fba258f947f..00000000000 --- a/client/app/lib/hooks/useQueryResult.js +++ /dev/null @@ -1,66 +0,0 @@ -import { includes, get, invoke } from "lodash"; -import { useState, useEffect, useRef } from "react"; - -function getQueryResultStatus(queryResult) { - return invoke(queryResult, "getStatus") || null; -} - -function isFinalStatus(status) { - return includes(["done", "failed"], status); -} - -function getQueryResultData(queryResult) { - return { - status: getQueryResultStatus(queryResult), - columns: invoke(queryResult, "getColumns") || [], - rows: invoke(queryResult, "getData") || [], - filters: invoke(queryResult, "getFilters") || [], - updatedAt: invoke(queryResult, "getUpdatedAt") || null, - retrievedAt: get(queryResult, "query_result.retrieved_at", null), - log: invoke(queryResult, "getLog") || [], - error: invoke(queryResult, "getError") || null, - runtime: invoke(queryResult, "getRuntime") || null, - metadata: get(queryResult, "query_result.data.metadata", {}), - }; -} - -export default function useQueryResult(queryResult) { - const [data, setData] = useState(getQueryResultData(queryResult)); - const queryResultRef = useRef(queryResult); - const lastStatusRef = useRef(getQueryResultStatus(queryResult)); - - useEffect(() => { - // This check is needed to avoid unnecessary updates. - // `useState`/`useRef` hooks use their argument (initial value) only on the first run. - // When `useEffect` will run for the first time, that values will be already properly - // initialized, so we just need to start polling. - // If `queryResult` object will later change - `useState`/`useRef` will not update values; - // in that case this section will not be skipped and will update internal state properly. - if (queryResult !== queryResultRef.current) { - queryResultRef.current = queryResult; - lastStatusRef.current = getQueryResultStatus(queryResult); - setData(getQueryResultData(queryResult)); - } - - if (!isFinalStatus(lastStatusRef.current)) { - let timer = setInterval(() => { - const currentStatus = getQueryResultStatus(queryResultRef.current); - if (lastStatusRef.current !== currentStatus) { - lastStatusRef.current = currentStatus; - setData(getQueryResultData(queryResultRef.current)); - } - - if (isFinalStatus(currentStatus)) { - clearInterval(timer); - timer = null; - } - }, 200); - - return () => { - clearInterval(timer); - }; - } - }, [queryResult]); - - return data; -} diff --git a/client/app/pages/queries/QuerySource.jsx b/client/app/pages/queries/QuerySource.jsx index 6bb96b7bfe9..6c9f10f94c3 100644 --- a/client/app/pages/queries/QuerySource.jsx +++ b/client/app/pages/queries/QuerySource.jsx @@ -29,6 +29,7 @@ import useQuery from "./hooks/useQuery"; import useVisualizationTabHandler from "./hooks/useVisualizationTabHandler"; import useAutocompleteFlags from "./hooks/useAutocompleteFlags"; import useQueryExecute from "./hooks/useQueryExecute"; +import getQueryResultData from "@/lib/getQueryResultData"; import useQueryDataSources from "./hooks/useQueryDataSources"; import useDataSourceSchema from "./hooks/useDataSourceSchema"; import useQueryFlags from "./hooks/useQueryFlags"; @@ -66,14 +67,17 @@ function QuerySource(props) { const { queryResult, - queryResultData, - isQueryExecuting, - isExecutionCancelling, + isExecuting: isQueryExecuting, + executionStatus, executeQuery, - executeAdhocQuery, - cancelExecution, + error: executionError, + cancelCallback: cancelExecution, + isCancelling: isExecutionCancelling, + updatedAt, } = useQueryExecute(query); + const queryResultData = getQueryResultData(queryResult); + const editorRef = useRef(null); const [autocompleteAvailable, autocompleteEnabled, toggleAutocomplete] = useAutocompleteFlags(schema); @@ -82,6 +86,7 @@ function QuerySource(props) { }, 100); useEffect(() => { + // TODO: ignore new pages? recordEvent("view_source", "query", query.id); }, [query.id]); @@ -154,20 +159,14 @@ function QuerySource(props) { return; } if (isDirty || !isEmpty(selectedText)) { - executeAdhocQuery(selectedText); + executeQuery(null, () => { + return query.getQueryResultByText(0, selectedText); + }); } else { executeQuery(); } }, - [ - queryFlags.canExecute, - areParametersDirty, - isQueryExecuting, - isDirty, - selectedText, - executeAdhocQuery, - executeQuery, - ] + [query, queryFlags.canExecute, areParametersDirty, isQueryExecuting, isDirty, selectedText, executeQuery] ); const [isQuerySaving, setIsQuerySaving] = useState(false); @@ -347,46 +346,44 @@ function QuerySource(props) { /> )} - {queryResult && queryResultData.status !== "done" && ( + {(executionError || isQueryExecuting) && (
)} - {queryResultData.status === "done" && ( - - {queryResultData.log.length > 0 && ( -
-

Log Information:

- {map(queryResultData.log, (line, index) => ( -

- {line} -

- ))} -
- )} - - - )} + + {queryResultData.log.length > 0 && ( +
+

Log Information:

+ {map(queryResultData.log, (line, index) => ( +

+ {line} +

+ ))} +
+ )} + + - {queryResultData.status === "done" && ( + {queryResult && !queryResult.getError() && (
{!queryFlags.isNew && queryFlags.canEdit && ( diff --git a/client/app/pages/queries/QueryView.jsx b/client/app/pages/queries/QueryView.jsx index bf45801c4cc..2d20a98204d 100644 --- a/client/app/pages/queries/QueryView.jsx +++ b/client/app/pages/queries/QueryView.jsx @@ -3,6 +3,7 @@ import PropTypes from "prop-types"; import cx from "classnames"; import useMedia from "use-media"; import Button from "antd/lib/button"; +import Icon from "antd/lib/icon"; import AuthenticatedPageWrapper from "@/components/ApplicationArea/AuthenticatedPageWrapper"; import EditInPlace from "@/components/EditInPlace"; @@ -15,6 +16,7 @@ import { ErrorBoundaryContext } from "@/components/ErrorBoundary"; import { Query } from "@/services/query"; import DataSource from "@/services/data-source"; import { pluralize, durationHumanize } from "@/lib/utils"; +import getQueryResultData from "@/lib/getQueryResultData"; import QueryPageHeader from "./components/QueryPageHeader"; import QueryVisualizationTabs from "./components/QueryVisualizationTabs"; @@ -35,18 +37,10 @@ import useDeleteVisualization from "./hooks/useDeleteVisualization"; import "./QueryView.less"; -// ANGULAR_REMOVE_ME: Update with new Router -function updateUrlSearch(...params) {} - function useFullscreenHandler(available) { - // TODO: bring back URL sync const [fullscreen, setFullscreen] = useState(false); const toggleFullscreen = useCallback(() => setFullscreen(!fullscreen), [fullscreen]); - useEffect(() => { - updateUrlSearch("fullscreen", fullscreen ? true : null); - }, [fullscreen]); - return useMemo(() => [available && fullscreen, toggleFullscreen], [available, fullscreen, toggleFullscreen]); } @@ -62,13 +56,17 @@ function QueryView(props) { const { queryResult, - queryResultData, - isQueryExecuting, - isExecutionCancelling, + isExecuting, + executionStatus, executeQuery, - cancelExecution, + error: executionError, + cancelCallback: cancelExecution, + isCancelling: isExecutionCancelling, + updatedAt, } = useQueryExecute(query); + const queryResultData = getQueryResultData(queryResult); + const updateQueryDescription = useUpdateQueryDescription(query, setQuery); const openAddToDashboardDialog = useAddToDashboardDialog(query); const openEmbedDialog = useEmbedDialog(query); @@ -82,12 +80,12 @@ function QueryView(props) { const doExecuteQuery = useCallback( (skipParametersDirtyFlag = false) => { - if (!queryFlags.canExecute || (!skipParametersDirtyFlag && (areParametersDirty || isQueryExecuting))) { + if (!queryFlags.canExecute || (!skipParametersDirtyFlag && (areParametersDirty || isExecuting))) { return; } executeQuery(); }, - [areParametersDirty, executeQuery, isQueryExecuting, queryFlags.canExecute] + [areParametersDirty, executeQuery, isExecuting, queryFlags.canExecute] ); useEffect(() => { @@ -111,7 +109,7 @@ function QueryView(props) { className="m-r-5" type="primary" shortcut="mod+enter, alt+enter" - disabled={!queryFlags.canExecute || isQueryExecuting || areParametersDirty} + disabled={!queryFlags.canExecute || isExecuting || areParametersDirty} onClick={doExecuteQuery}> Refresh @@ -159,18 +157,18 @@ function QueryView(props) {
)}
- {queryResult && queryResultData.status !== "done" && ( -
+ {(executionError || isExecuting) && ( +
)} - {(!queryResult || queryResultData.status === "done") && ( + {true && ( - {!isQueryExecuting &&