From c874eb6b11927481c34cf8ee9bfbf60957b602e8 Mon Sep 17 00:00:00 2001 From: Eric Radman Date: Tue, 14 May 2024 22:06:45 -0400 Subject: [PATCH] Revert changes to job status (#6969) "Query in queue" should switch to "Executing query", but does not. Commands: git revert --no-commit bd17662005248c087a8d6aeadb866ae94d3d3465 git revert --no-commit 5ac5d86f5e4ae0adb0c296e53f95cb79e612237a vim tests/handlers/test_query_results.py git add tests/handlers/test_query_results.py Co-authored-by: Justin Clift --- .../dashboard-widget/VisualizationWidget.jsx | 5 +- client/app/pages/queries/QuerySource.jsx | 4 +- client/app/pages/queries/QueryView.jsx | 2 +- .../components/QueryExecutionStatus.jsx | 26 +++--- client/app/services/query-result.js | 64 +++++++-------- client/app/services/query.js | 5 +- redash/handlers/api.py | 6 +- redash/handlers/query_results.py | 17 ++-- redash/serializers/__init__.py | 48 ++++++----- redash/tasks/queries/execution.py | 14 ++-- redash/tasks/worker.py | 7 +- tests/handlers/test_query_results.py | 10 ++- tests/serializers/test_job.py | 81 ------------------- tests/tasks/test_queries.py | 5 +- 14 files changed, 106 insertions(+), 188 deletions(-) delete mode 100644 tests/serializers/test_job.py diff --git a/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx b/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx index 58657e4c6df..9a021cc8bdc 100644 --- a/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx +++ b/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx @@ -19,7 +19,6 @@ import PlainButton from "@/components/PlainButton"; import ExpandedWidgetDialog from "@/components/dashboards/ExpandedWidgetDialog"; import EditParameterMappingsDialog from "@/components/dashboards/EditParameterMappingsDialog"; import VisualizationRenderer from "@/components/visualizations/VisualizationRenderer"; -import { ExecutionStatus } from "@/services/query-result"; import Widget from "./Widget"; @@ -279,7 +278,7 @@ class VisualizationWidget extends React.Component { const widgetQueryResult = widget.getQueryResult(); const widgetStatus = widgetQueryResult && widgetQueryResult.getStatus(); switch (widgetStatus) { - case ExecutionStatus.FAILED: + case "failed": return (
{widgetQueryResult.getError() && ( @@ -289,7 +288,7 @@ class VisualizationWidget extends React.Component { )}
); - case ExecutionStatus.FINISHED: + case "done": return (
Cancelling… : null; switch (status) { - case ExecutionStatus.QUEUED: + case "waiting": if (!isCancelling) { message = Query in queue…; } break; - case ExecutionStatus.STARTED: + case "processing": if (!isCancelling) { message = Executing query…; } break; - case ExecutionStatus.LOADING_RESULT: + case "loading-result": message = Loading results…; break; - case ExecutionStatus.FAILED: + case "failed": message = ( Error running query: {error} ); break; - case ExecutionStatus.CANCELED: - message = Query was canceled; - break; // no default } @@ -74,7 +66,7 @@ QueryExecutionStatus.propTypes = { }; QueryExecutionStatus.defaultProps = { - status: ExecutionStatus.QUEUED, + status: "waiting", updatedAt: null, error: null, isCancelling: true, diff --git a/client/app/services/query-result.js b/client/app/services/query-result.js index 6fb10fffa4c..7f50228fa65 100644 --- a/client/app/services/query-result.js +++ b/client/app/services/query-result.js @@ -50,15 +50,18 @@ const QueryResultResource = { }; export const ExecutionStatus = { - QUEUED: "queued", - STARTED: "started", - FINISHED: "finished", + WAITING: "waiting", + PROCESSING: "processing", + DONE: "done", FAILED: "failed", LOADING_RESULT: "loading-result", - CANCELED: "canceled", - DEFERRED: "deferred", - SCHEDULED: "scheduled", - STOPPED: "stopped", +}; + +const statuses = { + 1: ExecutionStatus.WAITING, + 2: ExecutionStatus.PROCESSING, + 3: ExecutionStatus.DONE, + 4: ExecutionStatus.FAILED, }; function handleErrorResponse(queryResult, error) { @@ -77,7 +80,7 @@ function handleErrorResponse(queryResult, error) { queryResult.update({ job: { error: "cached query result unavailable, please execute again.", - status: ExecutionStatus.FAILED, + status: 4, }, }); return; @@ -88,7 +91,7 @@ function handleErrorResponse(queryResult, error) { queryResult.update({ job: { error: get(error, "response.data.message", "Unknown error occurred. Please try again later."), - status: ExecutionStatus.FAILED, + status: 4, }, }); } @@ -99,19 +102,11 @@ function sleep(ms) { export function fetchDataFromJob(jobId, interval = 1000) { return axios.get(`api/jobs/${jobId}`).then(data => { - const status = data.job.status; - if ( - [ExecutionStatus.QUEUED, ExecutionStatus.STARTED, ExecutionStatus.SCHEDULED, ExecutionStatus.DEFERRED].includes( - status - ) - ) { + const status = statuses[data.job.status]; + if (status === ExecutionStatus.WAITING || status === ExecutionStatus.PROCESSING) { return sleep(interval).then(() => fetchDataFromJob(data.job.id)); - } else if (status === ExecutionStatus.FINISHED) { - return data.job.result_id; - } else if (status === ExecutionStatus.CANCELED) { - return Promise.reject("Job was canceled"); - } else if (status === ExecutionStatus.STOPPED) { - return Promise.reject("Job was stopped"); + } else if (status === ExecutionStatus.DONE) { + return data.job.result; } else if (status === ExecutionStatus.FAILED) { return Promise.reject(data.job.error); } @@ -127,7 +122,7 @@ class QueryResult { this.deferred = defer(); this.job = {}; this.query_result = {}; - this.status = ExecutionStatus.QUEUED; + this.status = "waiting"; this.updatedAt = moment(); @@ -143,8 +138,8 @@ class QueryResult { extend(this, props); if ("query_result" in props) { - this.status = ExecutionStatus.FINISHED; - this.deferred.onStatusChange(ExecutionStatus.FINISHED); + this.status = ExecutionStatus.DONE; + this.deferred.onStatusChange(ExecutionStatus.DONE); const columnTypes = {}; @@ -188,10 +183,11 @@ class QueryResult { }); this.deferred.resolve(this); - } else if (this.job.status === ExecutionStatus.STARTED || this.job.status === ExecutionStatus.FINISHED) { - this.status = ExecutionStatus.STARTED; - } else if (this.job.status === ExecutionStatus.FAILED) { - this.status = this.job.status; + } 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); @@ -215,7 +211,7 @@ class QueryResult { if (this.isLoadingResult) { return ExecutionStatus.LOADING_RESULT; } - return this.status || this.job.status; + return this.status || statuses[this.job.status]; } getError() { @@ -378,7 +374,7 @@ class QueryResult { this.isLoadingResult = true; this.deferred.onStatusChange(ExecutionStatus.LOADING_RESULT); - QueryResultResource.get({ id: this.job.result_id }) + QueryResultResource.get({ id: this.job.query_result_id }) .then(response => { this.update(response); this.isLoadingResult = false; @@ -393,7 +389,7 @@ class QueryResult { this.update({ job: { error: "failed communicating with server. Please check your Internet connection and try again.", - status: ExecutionStatus.FAILED, + status: 4, }, }); this.isLoadingResult = false; @@ -417,9 +413,9 @@ class QueryResult { .then(jobResponse => { this.update(jobResponse); - if (this.getStatus() === ExecutionStatus.STARTED && this.job.result_id && this.job.result_id !== "None") { + if (this.getStatus() === "processing" && this.job.query_result_id && this.job.query_result_id !== "None") { loadResult(); - } else if (this.getStatus() !== ExecutionStatus.FAILED) { + } else if (this.getStatus() !== "failed") { const waitTime = tryNumber > 10 ? 3000 : 500; setTimeout(() => { this.refreshStatus(query, parameters, tryNumber + 1); @@ -432,7 +428,7 @@ class QueryResult { this.update({ job: { error: "failed communicating with server. Please check your Internet connection and try again.", - status: ExecutionStatus.FAILED, + status: 4, }, }); }); diff --git a/client/app/services/query.js b/client/app/services/query.js index 76c8cf858ae..a8cf624cb87 100644 --- a/client/app/services/query.js +++ b/client/app/services/query.js @@ -2,7 +2,6 @@ import moment from "moment"; import debug from "debug"; import Mustache from "mustache"; import { axios } from "@/services/axios"; -import { ExecutionStatus } from "@/services/query-result"; import { zipObject, isEmpty, @@ -104,7 +103,7 @@ export class Query { return new QueryResult({ job: { error: `missing ${valuesWord} for ${missingParams.join(", ")} ${paramsWord}.`, - status: ExecutionStatus.FAILED, + status: 4, }, }); } @@ -361,7 +360,7 @@ export class QueryResultError { // eslint-disable-next-line class-methods-use-this getStatus() { - return ExecutionStatus.FAILED; + return "failed"; } // eslint-disable-next-line class-methods-use-this diff --git a/redash/handlers/api.py b/redash/handlers/api.py index 76de2f5a2d5..48428daf0f3 100644 --- a/redash/handlers/api.py +++ b/redash/handlers/api.py @@ -236,11 +236,11 @@ def json_representation(data, code, headers=None): ) api.add_org_resource( QueryResultResource, - "/api/query_results/.", - "/api/query_results/", + "/api/query_results/.", + "/api/query_results/", "/api/queries//results", "/api/queries//results.", - "/api/queries//results/.", + "/api/queries//results/.", endpoint="query_result", ) api.add_org_resource( diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py index 22a7a8631e7..bfc4371d085 100644 --- a/redash/handlers/query_results.py +++ b/redash/handlers/query_results.py @@ -5,7 +5,6 @@ from flask import make_response, request from flask_login import current_user from flask_restful import abort -from rq.job import JobStatus from redash import models, settings from redash.handlers.base import BaseResource, get_object_or_404, record_event @@ -39,7 +38,7 @@ def error_response(message, http_status=400): - return {"job": {"status": JobStatus.FAILED, "error": message}}, http_status + return {"job": {"status": 4, "error": message}}, http_status error_messages = { @@ -226,7 +225,7 @@ def add_cors_headers(headers): headers["Access-Control-Allow-Credentials"] = str(settings.ACCESS_CONTROL_ALLOW_CREDENTIALS).lower() @require_any_of_permission(("view_query", "execute_query")) - def options(self, query_id=None, result_id=None, filetype="json"): + def options(self, query_id=None, query_result_id=None, filetype="json"): headers = {} self.add_cors_headers(headers) @@ -286,12 +285,12 @@ def post(self, query_id): return error_messages["no_permission"] @require_any_of_permission(("view_query", "execute_query")) - def get(self, query_id=None, result_id=None, filetype="json"): + def get(self, query_id=None, query_result_id=None, filetype="json"): """ Retrieve query results. :param number query_id: The ID of the query whose results should be fetched - :param number result_id: the ID of the query result to fetch + :param number query_result_id: the ID of the query result to fetch :param string filetype: Format to return. One of 'json', 'xlsx', or 'csv'. Defaults to 'json'. :