alexandrusoare commented on code in PR #42305:
URL: https://github.com/apache/superset/pull/42305#discussion_r3664745023
##########
superset/async_events/api.py:
##########
@@ -99,3 +104,76 @@ def events(self) -> Response:
return self.response_401()
return self.response(200, result=events)
+
+ @expose("/<job_id>/cancel", methods=("POST",))
+ @event_logger.log_this
+ @protect()
+ @safe
+ @statsd_metrics
+ @permission_name("list")
Review Comment:
The cancel endpoint reuses @permission_name("list") from the read-only
events() method. What do you think of using a dedicated permission?
##########
superset/tasks/async_queries.py:
##########
@@ -84,6 +84,23 @@ def _load_user_from_job_metadata(job_metadata: dict[str,
Any]) -> User:
return user
+def _handle_soft_time_limit(
+ job_metadata: dict[str, Any], ex: Exception, activity: str
+) -> None:
+ """
+ SoftTimeLimitExceeded is raised both by a genuine timeout and by a
+ user-initiated cancel (revoke sends SIGUSR1). Emit the matching terminal
+ event so a cancelled job is reported as cancelled.
+ """
+ if async_query_manager.is_job_cancelled(job_metadata["job_id"]):
+ logger.info("Cancelled by the user while %s", activity)
+ async_query_manager.update_job(
+ job_metadata, async_query_manager.STATUS_CANCELLED
+ )
+ else:
Review Comment:
When is_job_cancelled returns False, no terminal event is emitted — the job
stays in RUNNING from the client's perspective. Worth adding an update_job(...,
STATUS_ERROR) in the else branch?
##########
superset-frontend/src/middleware/asyncEvent.ts:
##########
@@ -104,6 +104,15 @@ const fetchCachedData = async (
return { status, data };
};
+const cancelAsyncJob = (jobId: string) => {
+ // Best-effort server-side cancel; the request stops the running Celery task
+ // so it no longer consumes warehouse resources. Failures are non-fatal: the
+ // client has already stopped waiting on the job.
+ SupersetClient.post({
+ endpoint: `/api/v1/async_event/${jobId}/cancel`,
+ }).catch(() => {});
Review Comment:
Do you think a `console.warn` here with would help with debugging future
cancel failures?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
