-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enhancement: Add feature to manually retry failed task #1417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 6 commits
81c2836
cf3b790
012e958
bc0b8e3
6966ae8
323474e
a550f13
598c274
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
|
|
||
| from ..utils import tasks | ||
| from ..utils.broker import Broker | ||
| from ..utils.tasks import parse_args, parse_kwargs, make_json_serializable | ||
| from . import BaseApiHandler | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
@@ -636,3 +637,50 @@ def get(self, taskid): | |
| response['worker'] = task.worker.hostname | ||
|
|
||
| self.write(response) | ||
|
|
||
| class TaskReapply(BaseTaskHandler): | ||
| @web.authenticated | ||
| async def post(self, taskid): | ||
| """ | ||
| Get task info and reapply the task with the same arguments. | ||
|
|
||
| :param taskid: ID of the task to reapply. | ||
| """ | ||
| # Get original task info | ||
| task = tasks.get_task_by_id(self.application.events, taskid) | ||
|
||
| if not task: | ||
| raise HTTPError(404, f"Unknown task '{taskid}'") | ||
|
|
||
| # Get task name | ||
| taskname = task.name | ||
| if not taskname: | ||
| raise HTTPError(400, "Cannot reapply task with no name") | ||
|
|
||
| try: | ||
| # Get the task object from registered tasks | ||
| task_obj = self.capp.tasks[taskname] | ||
| except KeyError as exc: | ||
| raise HTTPError(404, f"Unknown task '{taskname}'") from exc | ||
|
|
||
| # Parse args and kwargs from the original task | ||
| try: | ||
| args = parse_args(task.args) | ||
| kwargs = parse_kwargs(task.kwargs) | ||
| except Exception as exc: | ||
|
auvipy marked this conversation as resolved.
Outdated
|
||
| logger.error("Error parsing task arguments: %s", exc) | ||
| raise HTTPError(400, f"Invalid task arguments: {str(exc)}") from exc | ||
|
|
||
| # Apply the task with original arguments | ||
| try: | ||
| # Ensure args and kwargs are JSON serializable | ||
| args = make_json_serializable(args) | ||
| kwargs = make_json_serializable(kwargs) | ||
|
|
||
| result = task_obj.apply_async(args=args, kwargs=kwargs) | ||
| response = {'task-id': result.task_id} | ||
| if self.backend_configured(result): | ||
| response.update(state=result.state) | ||
| self.write(response) | ||
| except Exception as exc: | ||
| logger.error("Error reapplying task with args=%s, kwargs=%s: %s", args, kwargs, str(exc)) | ||
| raise HTTPError(500, f"Error reapplying task: {str(exc)}") from exc | ||
|
Comment on lines
+684
to
+686
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -690,4 +690,37 @@ var flower = (function () { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $('#task-retry').click(function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const $button = $(this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const $spinner = $button.find('.spinner-border'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const taskId = $('#taskid').text(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!taskId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| show_alert('Task ID is missing. Cannot proceed.', 'danger'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Show loading state | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $button.prop('disabled', true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $spinner.removeClass('d-none'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Reapply the task using the reapply endpoint | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $.ajax({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: 'POST', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: url_prefix() + '/api/task/reapply/' + taskId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| success: function (response) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| show_alert(`Task ${taskId} has been retried (new task ID: ${response['task-id']})`, 'success'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| show_alert(`Task ${taskId} has been retried (new task ID: ${response['task-id']})`, 'success'); | |
| var newTaskId = response && typeof response === 'object' ? response['task-id'] : undefined; | |
| if (newTaskId) { | |
| show_alert(`Task ${taskId} has been retried (new task ID: ${newTaskId})`, 'success'); | |
| } else { | |
| show_alert(`Task ${taskId} has been retried.`, 'success'); | |
| } |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message displayed to users may contain the full response text which could include sensitive information or overly technical details. Consider parsing the error response JSON to extract a user-friendly message, or provide a generic error message with technical details logged separately.
| show_alert(response.responseText || 'Failed to retry task', 'danger'); | |
| var userMessage = 'Failed to retry task'; | |
| try { | |
| if (response && response.responseJSON) { | |
| if (typeof response.responseJSON.message === 'string') { | |
| userMessage = response.responseJSON.message; | |
| } else if (typeof response.responseJSON.error === 'string') { | |
| userMessage = response.responseJSON.error; | |
| } | |
| } else if (response && typeof response.responseText === 'string') { | |
| var parsed = JSON.parse(response.responseText); | |
| if (parsed && typeof parsed.message === 'string') { | |
| userMessage = parsed.message; | |
| } else if (parsed && typeof parsed.error === 'string') { | |
| userMessage = parsed.error; | |
| } | |
| } | |
| } catch (e) { | |
| // Ignore JSON parsing errors and fall back to generic message | |
| } | |
| if (!userMessage) { | |
| userMessage = 'Failed to retry task'; | |
| } | |
| show_alert(userMessage, 'danger'); | |
| // Log full technical details for debugging without exposing them to users | |
| try { | |
| console.error('Task retry failed for task', taskId, response); | |
| } catch (logError) { | |
| // Ignore logging errors | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,11 @@ <h2>{{ getattr(task, 'name', None) }} | |||||||||||||||||||||||||||||||||
| <button class="btn btn-danger float-end" id="task-terminate">Terminate</button> | ||||||||||||||||||||||||||||||||||
| {% elif task.state == "RECEIVED" or task.state == "RETRY" %} | ||||||||||||||||||||||||||||||||||
| <button class="btn btn-danger float-end" id="task-revoke">Revoke</button> | ||||||||||||||||||||||||||||||||||
| {% elif task.state == "FAILURE" %} | ||||||||||||||||||||||||||||||||||
| <button class="btn btn-warning float-end" id="task-retry" data-bs-toggle="button"> | ||||||||||||||||||||||||||||||||||
| <span class="spinner-border spinner-border-sm d-none" role="status" aria-hidden="true"></span> | ||||||||||||||||||||||||||||||||||
| Retry | ||||||||||||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||||||||||||
|
Comment on lines
16
to
+23
|
||||||||||||||||||||||||||||||||||
| <button class="btn btn-danger float-end" id="task-terminate">Terminate</button> | |
| {% elif task.state == "RECEIVED" or task.state == "RETRY" %} | |
| <button class="btn btn-danger float-end" id="task-revoke">Revoke</button> | |
| {% elif task.state == "FAILURE" %} | |
| <button class="btn btn-warning float-end" id="task-retry" data-bs-toggle="button"> | |
| <span class="spinner-border spinner-border-sm d-none" role="status" aria-hidden="true"></span> | |
| Retry | |
| </button> | |
| <button class="btn btn-danger float-end" id="task-terminate">Terminate</button> | |
| {% elif task.state == "RECEIVED" or task.state == "RETRY" %} | |
| <button class="btn btn-danger float-end" id="task-revoke">Revoke</button> | |
| {% elif task.state == "FAILURE" %} | |
| <button class="btn btn-warning float-end" id="task-retry" data-bs-toggle="button"> | |
| <span class="spinner-border spinner-border-sm d-none" role="status" aria-hidden="true"></span> | |
| Retry | |
| </button> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||||||||||||||||||||||||||||
| import datetime | ||||||||||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||
| import ast | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| from .search import parse_search_terms, satisfies_search_terms | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
@@ -68,3 +70,53 @@ def get_task_by_id(events, task_id): | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| def as_dict(task): | ||||||||||||||||||||||||||||||||
| return task.as_dict() | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| def parse_args(args): | ||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||
| Parse and process the `args` of the task. | ||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||
| if not args: | ||||||||||||||||||||||||||||||||
| return [] | ||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||
| # Attempt to parse JSON | ||||||||||||||||||||||||||||||||
| parsed_args = json.loads(args) | ||||||||||||||||||||||||||||||||
| if isinstance(parsed_args, str) and parsed_args.startswith('(') and parsed_args.endswith(')'): | ||||||||||||||||||||||||||||||||
| return ast.literal_eval(parsed_args) # Handle stringified tuples safely | ||||||||||||||||||||||||||||||||
|
Comment on lines
+83
to
+84
|
||||||||||||||||||||||||||||||||
| if isinstance(parsed_args, str) and parsed_args.startswith('(') and parsed_args.endswith(')'): | |
| return ast.literal_eval(parsed_args) # Handle stringified tuples safely |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning a list containing an Ellipsis object is problematic. The Ellipsis object (...) is typically used as a placeholder but returning it wrapped in a list can cause issues downstream. Consider returning an empty list or raising a ValueError to indicate that '...' is not a valid argument format.
| return [...] | |
| return [] |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Using ast.literal_eval on user-provided input (task.args) could be dangerous. While ast.literal_eval is safer than eval, it still executes Python code parsing. If an attacker can control the stored task.args value, they might craft malicious input. Consider restricting to JSON-only parsing or implementing additional validation before using ast.literal_eval.
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Using ast.literal_eval on user-provided input (task.kwargs) could be dangerous. While ast.literal_eval is safer than eval, it still executes Python code parsing. If an attacker can control the stored task.kwargs value, they might craft malicious input. Consider restricting to JSON-only parsing or implementing additional validation before using ast.literal_eval.
| try: | |
| # Fallback for stringified dictionaries | |
| if kwargs.startswith('{') and kwargs.endswith('}'): | |
| return ast.literal_eval(kwargs) | |
| except (ValueError, SyntaxError): | |
| return {} | |
| # Fallback for stringified dictionaries that are close to JSON | |
| if isinstance(kwargs, str) and kwargs.startswith('{') and kwargs.endswith('}'): | |
| # Best-effort conversion of legacy dict-string formats to JSON | |
| # Example: "{'foo': 'bar'}" -> "{"foo": "bar"}" | |
| tentative = kwargs.replace("'", '"') | |
| try: | |
| return json.loads(tentative) | |
| except json.JSONDecodeError: | |
| return {} |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function has incomplete handling of non-serializable types. While it handles Ellipsis, list, and dict, it doesn't handle other common non-serializable types like tuples, sets, datetime objects, or custom objects. Tuples should be converted to lists for JSON serialization, and other types may need special handling or should raise an error to alert callers of serialization issues.
| if isinstance(obj, list): | |
| return [make_json_serializable(item) for item in obj] | |
| elif isinstance(obj, dict): | |
| return {key: make_json_serializable(value) for key, value in obj.items()} | |
| if isinstance(obj, (list, tuple, set)): | |
| # Convert tuples and sets to lists, and recurse into all sequence elements | |
| return [make_json_serializable(item) for item in obj] | |
| elif isinstance(obj, dict): | |
| return {key: make_json_serializable(value) for key, value in obj.items()} | |
| elif isinstance(obj, (datetime.datetime, datetime.date, datetime.time)): | |
| # Represent datetime-like objects as ISO 8601 strings | |
| return obj.isoformat() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing API documentation: The TaskReapply endpoint lacks comprehensive API documentation that other endpoints in this file have (such as TaskInfo). Consider adding proper docstring documentation including HTTP method, example request/response, parameters description, and status codes.