Enhancement: Added feature to manually retry failed tasks #737
Conversation
…ueue" button to task detail view; requeues the task with original args and kwargs using the Flower async-apply API. Supports an existing feature request (mher#17).
|
@asfaltboy @mher @ask This is a follow-up against #158. I'd like you to review this PR. |
|
@jiffyjeff What do you think of this PR? |
|
Don't work, has problem with args parsing, because send tuple to json. |
|
@jheld Yes if you can! |
|
What is preventing this from being merged? |
|
@clintonb As far as I know, someone does not think this PR works well. |
|
Last time I tried to use it, it didn't work. But I'm willing to try again. |
|
One extra issue is that args or kwargs repr could be truncated. If the worker has an increased repr size, that can be a work around -- but out of control of this PR. Also, is there a way to remember/use any child tasks? Additionally, if the task result backend is enabled and the args/kwargs are there (newer version of celery, not yet released), then we could take from that if it is available. |
|
+1 |
|
Should I do something to get merged? |
|
@tnir yes please! |
|
If you do, please have a warning in docs or something that this does not re-queue child tasks, and depending on the size of args/kwargs, the re-queue itself might not run (repr may be truncated). The latter can be dealt with as part of running celery, but I don't think the former can. |
|
Another way to do this could be with use of the extended results feature on newer celery (4.3 I think), if all the settings are lined up. If the result backend has the tombstone and that extended setting is enabled, we should be able to get around the repr truncation issue at least. |
|
good feature, why can't it be merged? |
|
I think the idea is sound. But there are some caveats/documentation around it that I think would be required in this change should it be merged. If the truncation or extended results are not adjusted per the user, they may not be able to use the retry. That's effectively breakage from the user's perspective, unless there are prominently placed docs. Due to how celery/events construct tasks and the flow in them, it makes this feature harder. Additionally, what happens if this task is part of a chain? We would like to ensure we can make it possible to call the rest of the chain should this task succeed on manual retry. That's my take, and I feel it's important to get it right if we can. If not, then yes, it could be left off for now. |
|
@mher would you be able to add me as a collaborator/maintainer? I'd like to help merge this (once we agree on readiness/impact). |
|
Is there any update on this? @jheld |
|
I feel we're still blocked by at least the truncation issue. If the result backend extended setting support is merged then that will technically allow this to work better (so long as the key is still in the backend, not expired). So although this is a useful change, it's sitting in a boundary where a lot of folks are going to have breakage, so long as the other pieces aren't addressed. |
|
Closing this pull request because it will only work in limited cases |
This PR is based on @jiffyjeff's work #158 with follow-up to be ready to be merged.
Enhancement: Added feature to manually retry failed tasks. Added "Requeue" button to task detail view; requeues the task with original args and kwargs using the Flower async-apply API. Supports an existing feature request (#17).