jprdonnelly/redash
1
0
Fork 0
mirror of https://github.com/getredash/redash.git synced 2025-12-19 17:37:19 -05:00
Code Issues Projects Releases Wiki Activity

Revert changes to job status (#6969)

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

Commands:

git revert --no-commit bd17662005
git revert --no-commit 5ac5d86f5e
vim tests/handlers/test_query_results.py
git add tests/handlers/test_query_results.py

Co-authored-by: Justin Clift <justin@postgresql.org>
This commit is contained in:
Eric Radman
2024-05-14 22:06:45 -04:00
committed by GitHub
parent f3a323695f
commit c874eb6b11
14 changed files with 106 additions and 188 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx
View File

@@ -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

4
client/app/pages/queries/QuerySource.jsx
View File

@@ -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}

2
client/app/pages/queries/QueryView.jsx
View File

@@ -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}

26
client/app/pages/queries/components/QueryExecutionStatus.jsx
View File

@@ -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,

64
client/app/services/query-result.js
View File

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

5
client/app/services/query.js
View File

@@ -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

6
redash/handlers/api.py
View File

@@ -236,11 +236,11 @@ api.add_org_resource(
)
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(

17
redash/handlers/query_results.py
View File

@@ -5,7 +5,6 @@ import regex
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 @@ from redash.utils import (
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 @@ class QueryResultResource(BaseResource):
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 @@ class QueryResultResource(BaseResource):
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 @@ class QueryResultResource(BaseResource):
# 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 @@ class QueryResultResource(BaseResource):
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)

48
redash/serializers/__init__.py
View File

@@ -7,7 +7,6 @@ separation of concerns.
from flask_login import current_user
from funcy import project
from rq.job import JobStatus
from rq.results import Result
from rq.timeouts import JobTimeoutException
from redash import models
@@ -272,33 +271,46 @@ class DashboardSerializer(Serializer):
def serialize_job(job):
# TODO: this is mapping to the old Job class statuses. Need to update the client side and remove this
STATUSES = {
JobStatus.QUEUED: 1,
JobStatus.STARTED: 2,
JobStatus.FINISHED: 3,
JobStatus.FAILED: 4,
JobStatus.CANCELED: 5,
JobStatus.DEFERRED: 6,
JobStatus.SCHEDULED: 7,
}
job_status = job.get_status()
if job.is_started:
updated_at = job.started_at or 0
else:
updated_at = 0
status = job.get_status()
error = result_id = None
job_result = job.latest_result()
if job_result:
if job_result.type == Result.Type.SUCCESSFUL:
result = job_result.return_value
if isinstance(result, Exception):
error = str(result)
status = JobStatus.FAILED
elif isinstance(result, dict) and "error" in result:
error = result["error"]
status = JobStatus.FAILED
else:
result_id = result
else:
error = job_result.exc_string
status = STATUSES[job_status]
result = query_result_id = None
if job.is_cancelled:
error = "Query cancelled by user."
status = 4
elif isinstance(job.result, Exception):
error = str(job.result)
status = 4
elif isinstance(job.result, dict) and "error" in job.result:
error = job.result["error"]
status = 4
else:
error = ""
result = query_result_id = job.result
return {
"job": {
"id": job.id,
"updated_at": updated_at,
"status": status,
"error": error,
"result_id": result_id,
"result": result,
"query_result_id": query_result_id,
}
}

14
redash/tasks/queries/execution.py
View File

@@ -43,24 +43,24 @@ def enqueue_query(query, data_source, user_id, is_api_key=False, scheduled_query
if job_id:
logger.info("[%s] Found existing job: %s", query_hash, job_id)
job_complete = None
job_cancelled = None
try:
job = Job.fetch(job_id)
job_exists = True
status = job.get_status()
job_complete = status in [
JobStatus.FINISHED,
JobStatus.FAILED,
JobStatus.STOPPED,
JobStatus.CANCELED,
]
job_complete = status in [JobStatus.FINISHED, JobStatus.FAILED]
job_cancelled = job.is_cancelled
if job_complete:
message = "job found is complete (%s)" % status
elif job_cancelled:
message = "job found has ben cancelled"
except NoSuchJobError:
message = "job found has expired"
job_exists = False
lock_is_irrelevant = job_complete or not job_exists
lock_is_irrelevant = job_complete or job_cancelled or not job_exists
if lock_is_irrelevant:
logger.info("[%s] %s, removing lock", query_hash, message)

7
redash/tasks/worker.py
View File

@@ -65,7 +65,10 @@ class StatsdRecordingWorker(BaseWorker):
super().execute_job(job, queue)
finally:
statsd_client.decr("rq.jobs.running.{}".format(queue.name))
statsd_client.incr("rq.jobs.{}.{}".format(job.get_status(), queue.name))
if job.get_status() == JobStatus.FINISHED:
statsd_client.incr("rq.jobs.finished.{}".format(queue.name))
else:
statsd_client.incr("rq.jobs.failed.{}".format(queue.name))
class HardLimitingWorker(BaseWorker):
@@ -151,7 +154,7 @@ class HardLimitingWorker(BaseWorker):
job_status = job.get_status()
if job_status is None: # Job completed and its ttl has expired
return
if job_status not in [JobStatus.FINISHED, JobStatus.FAILED, JobStatus.STOPPED, JobStatus.CANCELED]:
if job_status not in [JobStatus.FINISHED, JobStatus.FAILED]:
if not job.ended_at:
job.ended_at = utcnow()

10
tests/handlers/test_query_results.py
View File

@@ -1,5 +1,3 @@
from rq.job import JobStatus
from redash.handlers.query_results import error_messages, run_query
from redash.models import db
from tests import BaseTestCase
@@ -436,6 +434,9 @@ class TestQueryResultExcelResponse(BaseTestCase):
class TestJobResource(BaseTestCase):
def test_cancels_queued_queries(self):
QUEUED = 1
FAILED = 4
query = self.factory.create_query()
job_id = self.make_request(
"post",
@@ -446,9 +447,10 @@ class TestJobResource(BaseTestCase):
]["id"]
status = self.make_request("get", f"/api/jobs/{job_id}").json["job"]["status"]
self.assertEqual(status, JobStatus.QUEUED)
self.assertEqual(status, QUEUED)
self.make_request("delete", f"/api/jobs/{job_id}")
job = self.make_request("get", f"/api/jobs/{job_id}").json["job"]
self.assertEqual(job["status"], JobStatus.CANCELED)
self.assertEqual(job["status"], FAILED)
self.assertTrue("cancelled" in job["error"])

81
tests/serializers/test_job.py
View File

@@ -1,81 +0,0 @@
from unittest.mock import MagicMock
from rq.job import JobStatus
from rq.results import Result
from redash.serializers import (
serialize_job,
)
from redash.tasks.queries.execution import QueryExecutionError
from tests import BaseTestCase
class JobSerializationTest(BaseTestCase):
def test_serializes_job_with_exception_in_result(self):
job = MagicMock()
job.id = 0
job.is_started = False
job.get_status = MagicMock(return_value=JobStatus.FINISHED)
result = MagicMock()
result.type = Result.Type.SUCCESSFUL
result.return_value = QueryExecutionError("test")
job.latest_result = MagicMock(return_value=result)
result = serialize_job(job)
self.assertDictEqual(
result,
{
"job": {
"id": 0,
"updated_at": 0,
"status": JobStatus.FAILED,
"error": str(QueryExecutionError("test")),
"result_id": None,
}
},
)
def test_serializes_job_with_dict_that_contains_error_in_result(self):
job = MagicMock()
job.id = 0
job.is_started = False
job.get_status = MagicMock(return_value=JobStatus.FINISHED)
result = MagicMock()
result.type = Result.Type.SUCCESSFUL
result.return_value = {"error": "test error"}
job.latest_result = MagicMock(return_value=result)
result = serialize_job(job)
self.assertDictEqual(
result,
{
"job": {
"id": 0,
"updated_at": 0,
"status": JobStatus.FAILED,
"error": "test error",
"result_id": None,
}
},
)
def test_serializes_job_with_dict_that_finished_successfully(self):
job = MagicMock()
job.id = 0
job.is_started = False
job.get_status = MagicMock(return_value=JobStatus.FINISHED)
result = MagicMock()
result.type = Result.Type.SUCCESSFUL
result.return_value = 1
job.latest_result = MagicMock(return_value=result)
result = serialize_job(job)
self.assertDictEqual(
result,
{
"job": {
"id": 0,
"updated_at": 0,
"status": JobStatus.FINISHED,
"error": None,
"result_id": 1,
}
},
)

5
tests/tasks/test_queries.py
View File

@@ -1,7 +1,6 @@
from mock import Mock, patch
from rq import Connection
from rq.exceptions import NoSuchJobError
from rq.job import JobStatus
from redash import models, rq_redis_connection
from redash.query_runner.pg import PostgreSQL
@@ -22,7 +21,7 @@ def fetch_job(*args, **kwargs):
result = Mock()
result.id = job_id
result.get_status = lambda: JobStatus.STARTED
result.is_cancelled = False
return result
@@ -108,7 +107,7 @@ class TestEnqueueTask(BaseTestCase):
# "cancel" the previous job
def cancel_job(*args, **kwargs):
job = fetch_job(*args, **kwargs)
job.get_status = lambda: JobStatus.CANCELED
job.is_cancelled = True
return job
my_fetch_job.side_effect = cancel_job