Revert changes to job status (#6969)

"Query in queue" should switch to "Executing query", but does not.

Commands:

git revert --no-commit bd17662
git revert --no-commit 5ac5d86
vim tests/handlers/test_query_results.py
git add tests/handlers/test_query_results.py

Co-authored-by: Justin Clift <justin@postgresql.org>

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 (
           <div className="body-row-auto scrollbox">
             {widgetQueryResult.getError() && (
@@ -289,7 +288,7 @@ class VisualizationWidget extends React.Component {
             )}
           </div>
         );
-      case ExecutionStatus.FINISHED:
+      case "done":
         return (
           <div className="body-row-auto scrollbox">
             <VisualizationRenderer

client/app/pages/queries/QuerySource.jsx

@@ -380,9 +380,7 @@ function QuerySource(props) {
                     <QueryVisualizationTabs
                       queryResult={queryResult}
                       visualizations={query.visualizations}
-                      showNewVisualizationButton={
-                        queryFlags.canEdit && queryResultData.status === ExecutionStatus.FINISHED
-                      }
+                      showNewVisualizationButton={queryFlags.canEdit && queryResultData.status === ExecutionStatus.DONE}
                       canDeleteVisualizations={queryFlags.canEdit}
                       selectedTab={selectedVisualization}
                       onChangeTab={setSelectedVisualization}

client/app/pages/queries/QueryView.jsx

@@ -165,7 +165,7 @@ function QueryView(props) {
             <QueryVisualizationTabs
               queryResult={queryResult}
               visualizations={query.visualizations}
-              showNewVisualizationButton={queryFlags.canEdit && queryResultData.status === ExecutionStatus.FINISHED}
+              showNewVisualizationButton={queryFlags.canEdit && queryResultData.status === ExecutionStatus.DONE}
               canDeleteVisualizations={queryFlags.canEdit}
               selectedTab={selectedVisualization}
               onChangeTab={setSelectedVisualization}

client/app/pages/queries/components/QueryExecutionStatus.jsx

@@ -1,45 +1,37 @@
+import { includes } from "lodash";
 import React from "react";
 import PropTypes from "prop-types";
 import Alert from "antd/lib/alert";
 import Button from "antd/lib/button";
 import Timer from "@/components/Timer";
-import { ExecutionStatus } from "@/services/query-result";
 
 export default function QueryExecutionStatus({ status, updatedAt, error, isCancelling, onCancel }) {
-  const alertType = status === ExecutionStatus.FAILED ? "error" : "info";
-  const showTimer = status !== ExecutionStatus.FAILED && updatedAt;
-  const isCancelButtonAvailable = [
-    ExecutionStatus.SCHEDULED,
-    ExecutionStatus.QUEUED,
-    ExecutionStatus.STARTED,
-    ExecutionStatus.DEFERRED,
-  ].includes(status);
+  const alertType = status === "failed" ? "error" : "info";
+  const showTimer = status !== "failed" && updatedAt;
+  const isCancelButtonAvailable = includes(["waiting", "processing"], status);
   let message = isCancelling ? <React.Fragment>Cancelling&hellip;</React.Fragment> : null;
 
   switch (status) {
-    case ExecutionStatus.QUEUED:
+    case "waiting":
       if (!isCancelling) {
         message = <React.Fragment>Query in queue&hellip;</React.Fragment>;
       }
       break;
-    case ExecutionStatus.STARTED:
+    case "processing":
       if (!isCancelling) {
         message = <React.Fragment>Executing query&hellip;</React.Fragment>;
       }
       break;
-    case ExecutionStatus.LOADING_RESULT:
+    case "loading-result":
       message = <React.Fragment>Loading results&hellip;</React.Fragment>;
       break;
-    case ExecutionStatus.FAILED:
+    case "failed":
       message = (
         <React.Fragment>
           Error running query: <strong>{error}</strong>
         </React.Fragment>
       );
       break;
-    case ExecutionStatus.CANCELED:
-      message = <React.Fragment>Query was canceled</React.Fragment>;
-      break;
     // no default
   }
 
@@ -74,7 +66,7 @@ QueryExecutionStatus.propTypes = {
 };
 
 QueryExecutionStatus.defaultProps = {
-  status: ExecutionStatus.QUEUED,
+  status: "waiting",
   updatedAt: null,
   error: null,
   isCancelling: true,

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,
           },
         });
       });

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

redash/handlers/api.py

@@ -236,11 +236,11 @@ def json_representation(data, code, headers=None):
 )
 api.add_org_resource(
     QueryResultResource,
-    "/api/query_results/<result_id>.<filetype>",
-    "/api/query_results/<result_id>",
+    "/api/query_results/<query_result_id>.<filetype>",
+    "/api/query_results/<query_result_id>",
     "/api/queries/<query_id>/results",
     "/api/queries/<query_id>/results.<filetype>",
-    "/api/queries/<query_id>/results/<result_id>.<filetype>",
+    "/api/queries/<query_id>/results/<query_result_id>.<filetype>",
     endpoint="query_result",
 )
 api.add_org_resource(

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'.
 
         :<json number id: Query result ID
@@ -306,13 +305,13 @@ def get(self, query_id=None, result_id=None, filetype="json"):
         # This method handles two cases: retrieving result by id & retrieving result by query id.
         # They need to be split, as they have different logic (for example, retrieving by query id
         # should check for query parameters and shouldn't cache the result).
-        should_cache = result_id is not None
+        should_cache = query_result_id is not None
 
         query_result = None
         query = None
 
-        if result_id:
-            query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, result_id, self.current_org)
+        if query_result_id:
+            query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, query_result_id, self.current_org)
 
         if query_id is not None:
             query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
@@ -347,7 +346,7 @@ def get(self, query_id=None, result_id=None, filetype="json"):
                     event["object_id"] = query_id
                 else:
                     event["object_type"] = "query_result"
-                    event["object_id"] = result_id
+                    event["object_id"] = query_result_id
 
                 self.record_event(event)