search

celery / django-celery-results

Celery result back end with django
Other
696 stars 206 forks source link

Store current time when TaskResult started #432

Closed yktakaha4 closed 5 months ago

yktakaha4 commented 5 months ago

Thank you to all maintainers of this great OSS.

Referring to the issue below, I generate TaskResult when Task is PENDING. ref: https://github.com/celery/django-celery-results/issues/286#issuecomment-1789094153

This code works fine, but I can't use date_created as the start time of the Task because the TaskResult already exists when STARTED is recorded. Additionally, in my use case, I want to measure the time from when a task is added to the message broker until the worker starts processing it.

By recording date_started as the time associated with STARTED, we can calculate the waiting time in the queue or the actual processing time of the worker. Use django.db.models.functions.Now to get the current time to avoid being affected by the server's time settings. ref: https://docs.djangoproject.com/en/5.0/ref/models/database-functions/#now

This PR is backward compatible because it simply adds a column and sets a value. For users who do not create TaskResults themselves, date_created and date_started have the same meaning.

Testing

We can check the value of date_started in django admin.

admin

For testing, I implemented a 5 second sleep on @shared_task. The time increases in the order date_created < date_started < date_done.

select id, status, date_created, date_started, date_done
from django_celery_results_taskresult
where id = 35;
id status date_created date_started date_done
35 SUCCESS 2024-06-08 06:14:37.631030 +00:00 2024-06-08 06:14:37.650764 +00:00 2024-06-08 06:14:43.334453 +00:00

Please let me know if there is anything else that should be tested.

auvipy commented 5 months ago

in the mean time, can you please review this PR? https://github.com/celery/django-celery-results/pull/407

yktakaha4 commented 5 months ago

@auvipy Thank you for your quick review.

I have confirmed the fix for #407. I noticed that no tests were written and no documentation was provided, but I couldn't think of any points that would functionally conflict with my PR. (Should I point this out in the review?)

In my personal opinion, TaskResult.state is just for storing celery.states, and I thought it might make state customization possible in Celery and then change it, but I'm not confident.