Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions flower/api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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.
Comment on lines +645 to +647
Copy link

Copilot AI Jan 7, 2026

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.

Suggested change
Get task info and reapply the task with the same arguments.
:param taskid: ID of the task to reapply.
Reapply a previously executed task using the same arguments.
This endpoint retrieves an existing task by its ID, extracts its original
positional and keyword arguments, and submits a new task with the same
name and arguments. A new task ID is returned for the re-applied task.
**Example request**:
.. sourcecode:: http
POST /api/task/reapply/7b3b9f52-1af3-4e0c-8a8a-5a5f9c2f8c64 HTTP/1.1
Host: localhost
Accept: application/json
Cookie: user=...
**Example response**:
.. sourcecode:: http
HTTP/1.1 200 OK
Content-Type: application/json
{
"task-id": "c1a2b3d4-5678-90ab-cdef-1234567890ab",
"state": "PENDING"
}
:param str taskid: ID of the original task to reapply.
:statuscode 200: task successfully reapplied; returns new task ID and state (if backend configured)
:statuscode 400: invalid task arguments or original task has no name
:statuscode 401: unauthorized request
:statuscode 404: unknown task ID or unknown task name
:statuscode 500: internal error while reapplying the task

Copilot uses AI. Check for mistakes.
"""
# Get original task info
task = tasks.get_task_by_id(self.application.events, taskid)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation: The taskid parameter from the URL is used directly without validation. Consider validating that taskid matches expected UUID format to prevent potential injection issues or unnecessary processing of invalid task IDs.

Copilot uses AI. Check for mistakes.
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:
Comment thread
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
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using broad exception handling with 'Exception' can mask unexpected errors and make debugging difficult. The try block contains multiple operations (make_json_serializable, apply_async, response creation) and catching all exceptions makes it unclear what specific failure occurred. Consider using more specific exception types or separating error handling for different operations.

Copilot uses AI. Check for mistakes.
33 changes: 33 additions & 0 deletions flower/static/js/flower.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling for empty or undefined response. The success callback accesses response['task-id'] without verifying that the response object exists or contains the 'task-id' key. If the API returns an unexpected response structure, this could cause a runtime error.

Suggested change
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 uses AI. Check for mistakes.
// Optionally reload the page after success
setTimeout(() => location.reload(), 1500);
},
error: function (response) {
show_alert(response.responseText || 'Failed to retry task', 'danger');
Copy link

Copilot AI Jan 7, 2026

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.

Suggested change
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
}

Copilot uses AI. Check for mistakes.
// Reset button state on error
$button.prop('disabled', false);
$spinner.addClass('d-none');
}
});
});


}(jQuery));
7 changes: 6 additions & 1 deletion flower/templates/task.html
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect indentation: The 'elif' statement is not aligned with the previous 'if' statement on line 17. The indentation should be at the same level as the 'if' on line 15 to maintain proper HTML structure and logical flow.

Suggested change
<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>

Copilot uses AI. Check for mistakes.
{% end %}
</h2>
</div>
Expand Down Expand Up @@ -89,4 +94,4 @@ <h2>{{ getattr(task, 'name', None) }}
</div>
</div>
</div>
{% end %}
{% end %}
52 changes: 52 additions & 0 deletions flower/utils/tasks.py
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

Expand Down Expand Up @@ -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
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic in lines 83-85 is unreachable. After json.loads succeeds on line 82, parsed_args is the result of parsing JSON. If the original string was a tuple like '(1, 2, 3)', json.loads would fail (tuples aren't valid JSON), so this code would be in the exception handler. If parsed_args is already parsed from JSON, it won't be a string that starts with '(' unless the JSON itself was a string literal like '"(1, 2)"', which is unlikely. This check should be removed or the logic should be restructured.

Suggested change
if isinstance(parsed_args, str) and parsed_args.startswith('(') and parsed_args.endswith(')'):
return ast.literal_eval(parsed_args) # Handle stringified tuples safely

Copilot uses AI. Check for mistakes.
return parsed_args
except (json.JSONDecodeError, SyntaxError):
# Fallback for stringified tuples or ellipsis
if args == '...':
return [...]
Copy link

Copilot AI Jan 7, 2026

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.

Suggested change
return [...]
return []

Copilot uses AI. Check for mistakes.
if args.startswith('(') and args.endswith(')'):
return ast.literal_eval(args)
Comment on lines +90 to +91
Copy link

Copilot AI Jan 7, 2026

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 uses AI. Check for mistakes.
return [args]

def parse_kwargs(kwargs):
"""
Parse and process the `kwargs` of the task.
"""
if not kwargs:
return {}
try:
# Attempt to parse JSON
return json.loads(kwargs)
except json.JSONDecodeError:
try:
# Fallback for stringified dictionaries
if kwargs.startswith('{') and kwargs.endswith('}'):
return ast.literal_eval(kwargs)
except (ValueError, SyntaxError):
return {}
Copy link

Copilot AI Jan 7, 2026

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.

Suggested change
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 uses AI. Check for mistakes.
return {}
Comment thread
auvipy marked this conversation as resolved.
Outdated

def make_json_serializable(obj):
"""
Recursively replace non-serializable types with JSON-serializable alternatives.
"""
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()}
Comment on lines +116 to +119
Copy link

Copilot AI Jan 7, 2026

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.

Suggested change
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()

Copilot uses AI. Check for mistakes.
elif obj is Ellipsis:
return None # Replace `...` with `null`
return obj # Return the object if it's already serializable