taskiq icon indicating copy to clipboard operation
taskiq copied to clipboard

feat: set/get progress

Open Sobes76rus opened this issue 1 year ago • 13 comments

properly created PR instead of https://github.com/taskiq-python/taskiq/pull/121

Sobes76rus avatar May 12 '23 17:05 Sobes76rus

@s3rius Ok, i did it =)

Sobes76rus avatar May 12 '23 18:05 Sobes76rus

For example

await def task(progress: ProgressTracker[int] = Depends()): 
    await progress.set_progress('progress', 123)

Sobes76rus avatar May 12 '23 19:05 Sobes76rus

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (d708468) 77.40% compared to head (8d3c27f) 77.67%.

Files Patch % Lines
taskiq/depends/progress_tracker.py 89.65% 3 Missing :warning:
taskiq/brokers/inmemory_broker.py 88.88% 1 Missing :warning:
taskiq/task.py 83.33% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #130      +/-   ##
===========================================
+ Coverage    77.40%   77.67%   +0.27%     
===========================================
  Files           60       61       +1     
  Lines         1748     1792      +44     
===========================================
+ Hits          1353     1392      +39     
- Misses         395      400       +5     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar May 12 '23 21:05 codecov-commenter

@s3rius Hello, I made the changes as you suggested. Could you please take a look at them

Sobes76rus avatar May 15 '23 15:05 Sobes76rus

@s3rius Hi! Sorry to be annoying, but could you give me some feedback please. =)

Sobes76rus avatar May 24 '23 14:05 Sobes76rus

@s3rius Hello there! I have resolved conflicts with latest version and make some changes. Could u pls take another look =)

Sobes76rus avatar Feb 01 '24 20:02 Sobes76rus

~~I was looking for a progress report feature to show a progress bar, and found this. This feature name should change to "Task state reporting"~~

My very one-sided opinion on this feature is: pulling states from worker instead of pushing to result backend.

In case of way too many tasks, I think state reporting will easily over overwhelmed result backend. This might be solved by using reporting to a broker instead of a result backend, or have a limiter built-in.

For me, I might report what percentage the task is finished, which I should report as often as possible; and if there is not that much states, it is not necessary to report.

Also, I think state reporting needs to be one-way communication, or it will confuse people: should I use "state reporting" to handle error and retry, or use a middleware.

~~Maybe the end goal is to make a worker monitoring for taskiq.~~

kice avatar Feb 22 '24 18:02 kice

@s3rius hello there! Could you please take another look at this request

Sobes76rus avatar Apr 22 '24 14:04 Sobes76rus

Is the idea of this PR just to know the overall progress of a task as started/failure/success/retry or can an actual status message be passed from the worker task and fetched from the backend/broker (e.g. progress percentage or stage to then be displayed to user)?

kyboi avatar Apr 30 '24 11:04 kyboi

@kyboi, yes. Mainly for progress.

s3rius avatar Apr 30 '24 11:04 s3rius

@s3rius I think it should be up to the user to decide how to use the progress tracker. The overall status should be implemented with some ProgressMiddleware, not in the receiver. Or there should be a way to disable it. In other cases, the user can pass custom progress messages using the ProgressTracker dependency.

Sobes76rus avatar Apr 30 '24 11:04 Sobes76rus

@kyboi my intension was to use custom status messages. overall status should be optional middleware

Sobes76rus avatar Apr 30 '24 11:04 Sobes76rus

@Sobes76rus, I guess the progress middleware can be implemented in subsequent request, so it's fine to not set statuses here.

s3rius avatar Apr 30 '24 11:04 s3rius