Deprecating stop_task: A Discussion about celery-progress HOT 8 CLOSED

If we choose to keep stop_task, the only suitable solution that comes to mind would be something like this:

def stop_task(exc):
    raise (exc if isinstance(exc, Exception) else Exception(exc))

This would have the exact same effect as the what the previous implementation aimed to accomplish, with the caveat that the bar would still go to 100%. In order for us to maintain the current state of the progress bar vs filling it completely, we would have to make changes to the client-side JavaScript to somehow interpret that from the response, and we would have to modify how Progress.get_info sets the progress state for "completed" tasks to detect failures vs. successes, as right now both states just return the "completed" task state.

In the end, all of this is just extra work to offer a crutch for actually just raising the error yourself. Heck, it even takes more characters to type stop_task(exc=exc) instead of raise exc. ConsoleProgressRecorder's method doesn't even do anything! While I understand the valid concern for whether or not we should cater to entry-level programmers, I would think that if they've somehow managed to set up a Django webserver that requires progress bars, they would have at some point come across the try/except logic and exception raising.

from celery-progress.

Another thought that came to my mind, stop_task may only makes sense if a start_task method existed. However, I don't think it should be the focus of the celery-progress utility, instead make it as simple as possible to manage the progress/status of a running task. If the two start and end points (creating/activating and ending/killing) stay true to Celery, then one should have no problem implementing those using Celery's documentation.

from celery-progress.

I think you lay out a compelling case and I'm happy to deprecate/remove support for it. It's not a feature that I've ever had the need for, and I just merged in the enhancement since it seemed to be useful to others.

Re-reading that PR, it does seem like the core use case of providing input on a failure within the task is better handled through the ways you've outlined.

from celery-progress.

The first time I used celery-progress, I got confused with stop_task. I thought it would nicely handle the killing of the task (eg. raise the exception) and freeze the progress bar displaying the error message. But it did none of those things so I quickly move away from it.
If it can be made more useful (eg. my initial expectation and/or more) I think it could be nice for novices like me to have a quick hook to stop the on-going progress.

from celery-progress.

Although, for a possible counter-argument, keeping stop_task in the modified form would allow users to subclass the bar and add their own logic. So I guess the argument at that point becomes "it's a crutch for actual Python" vs. "user extendibility". Decisions, decisions...

from celery-progress.

@EJH2 you have my support on whatever you think makes sense on this 😄

Sorry if I'm acting a bit hands off - I just don't have any strong opinions on this particular topic. Plus, I think you're doing a great job stewarding the lib and trust your judgment.

from celery-progress.

I'm in favour of dropping stop_task and letting the user simply raise an exception.

The only use case for stop_task is if you want to ignore the task (which prevents state updates), but you also want to update the state and save the exception. Right? I'm not sure why you would want to do that.

from celery-progress.

It seems that the general consensus is fine with removing the method, so I'll put out a PR that corrects this.

from celery-progress.