Fixed when messages are acked.

s3rius commented 8 months ago

This PR fixes the acknowledgements behavior. If worker goes down when executing tasks, message won't be acknowledged untill it executed.

codecov[bot] commented 8 months ago

Codecov Report

Merging #222 (fac8905) into develop (e66f3aa) will increase coverage by 5.80%. Report is 140 commits behind head on develop. The diff coverage is 73.96%.

@@             Coverage Diff             @@
##           develop     #222      +/-   ##
===========================================
+ Coverage    67.62%   73.42%   +5.80%     
===========================================
  Files           37       55      +18     
  Lines          942     1656     +714     
===========================================
+ Hits           637     1216     +579     
- Misses         305      440     +135

Files	Coverage Δ
taskiq/__init__.py	`100.00% <100.00%> (ø)`
taskiq/abc/formatter.py	`100.00% <100.00%> (ø)`
taskiq/abc/middleware.py	`100.00% <ø> (ø)`
taskiq/abc/result_backend.py	`100.00% <ø> (ø)`
taskiq/abc/serializer.py	`100.00% <100.00%> (ø)`
taskiq/acks.py	`100.00% <100.00%> (ø)`
taskiq/api/__init__.py	`100.00% <100.00%> (ø)`
taskiq/cli/common_args.py	`100.00% <100.00%> (ø)`
taskiq/cli/watcher.py	`0.00% <ø> (ø)`
taskiq/cli/worker/log_collector.py	`100.00% <ø> (ø)`
... and 43 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

thebalaa commented 7 months ago

hi @s3rius,

this change caused an issue for us recently, in celery there is this option: https://docs.celeryq.dev/en/latest/userguide/configuration.html#task-acks-late

Would you be open to making when a task is acked configurable in a similar fashion?

s3rius commented 7 months ago

Sure. Can you please clarify what issues have you ran into?

thebalaa commented 7 months ago

In our Matrix powered TaskIQ broker a slow task is being run multiple times because we expected the worker to ack task before running them.

I was under the impression that if you were concerned with the outcome of a task you could await the result from the result backend. Does it make sense?

s3rius commented 7 months ago

Okay. I see as a possible solution allowing users to choose when to ack tasks. Here're possible variants for the option:

Tasks are acked when received;
Tasks are acked right after they was executed;
Tasks are acked when results are stored in the result backend;

What do you think?

And let's set the third one as default.

taskiq-python / taskiq

Fixed when messages are acked. #222

Codecov Report