[Django] #37126: Make Task and TaskResult comparable

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+---------------------------------------
Reporter: Johannes Maron | Type: New feature
Status: new | Component: Tasks
Version: dev | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+---------------------------------------
[https://forum.djangoproject.com/t/tasks-framework-versatility-
performance/45035/4 Jake and I have been discussing changes to the Task
framework for 6.2 with a focus on performance and versatility.]

Python's [https://docs.python.org/3/library/queue.html#queue.PriorityQueue
queue.PriorityQueue] implementation requires objects to be comparable.

Since the base implementation of a `Task` implements a priority, it only
makes sense to provide basic comparability based on the priority and date.

Dataclasses have a neat `order=True` attribute to make this stupidly
simple.
--
Ticket URL: <https://code.djangoproject.com/ticket/37126>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+--------------------------------------

Reporter: Johannes Maron | Owner: (none)

Type: New feature | Status: new
Component: Tasks | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Johannes Maron):

I would also suggest adding a custom __eq__ method. We only need to
include `id` and a simple string comparison is much quicker. Benchmarks N
= 100_000_000:


{{{
Original == (equal) 18.614s (186.1 ns/call)
Original == (not equal) 7.452s (74.5 ns/call)
Id-only == (equal) 4.434s (44.3 ns/call)
Id-only == (not equal) 4.423s (44.2 ns/call)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:1>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: (none)
Type: New feature | Status: new
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+--------------------------------------
Comment (by Johannes Maron):

Missing benchmark sources for my previous comment:

{{{
import timeit
from dataclasses import dataclass, field
from datetime import datetime
from typing import Any

from django.utils.json import normalize_json
from django.tasks.base import Task, TaskResult, TaskResultStatus

# --- Real Task instance ---

def my_func():
pass

real_task = Task.__new__(Task)
object.__setattr__(real_task, "func", my_func)
object.__setattr__(real_task, "priority", 0)
object.__setattr__(real_task, "backend", "default")
object.__setattr__(real_task, "queue_name", "default")
object.__setattr__(real_task, "run_after", None)
object.__setattr__(real_task, "takes_context", False)

# --- Shared kwargs ---

now = datetime.now()

common_kwargs = dict(
task=real_task,
id="abc123",
status=TaskResultStatus.SUCCESSFUL,
enqueued_at=now,
started_at=now,
finished_at=now,
last_attempted_at=now,
args=[],
kwargs={},
backend="default",
errors=[],
worker_ids=["worker-1"],
)

# --- id-only equality variant ---

@dataclass(frozen=True, slots=True, kw_only=True)
class TaskResultIdEq:
task: Any = field(compare=False)
id: str
status: Any = field(compare=False)
enqueued_at: datetime | None = field(compare=False)
started_at: datetime | None = field(compare=False)
finished_at: datetime | None = field(compare=False)
last_attempted_at: datetime | None = field(compare=False)
args: list[Any] = field(compare=False)
kwargs: dict[str, Any] = field(compare=False)
backend: str = field(compare=False)
errors: list = field(compare=False)
worker_ids: list[str] = field(compare=False)
_return_value: Any | None = field(init=False, default=None,
compare=False)

def __post_init__(self):
object.__setattr__(self, "args", normalize_json(self.args))
object.__setattr__(self, "kwargs", normalize_json(self.kwargs))

# --- Instances to compare ---

a_full = TaskResult(**common_kwargs)
b_full = TaskResult(**common_kwargs)
# equal
c_full = TaskResult(**{**common_kwargs, "id": "other"}) #
not equal

a_id = TaskResultIdEq(**common_kwargs)
b_id = TaskResultIdEq(**common_kwargs)
# equal
c_id = TaskResultIdEq(**{**common_kwargs, "id": "other"}) #
not equal

N = 100_000_000

results = {
"Original == (equal)": timeit.timeit(lambda: a_full == b_full,
number=N),
"Original == (not equal)": timeit.timeit(lambda: a_full == c_full,
number=N),
"Id-only == (equal)": timeit.timeit(lambda: a_id == b_id,
number=N),
"Id-only == (not equal)": timeit.timeit(lambda: a_id == c_id,
number=N),
}

for label, t in results.items():
print(f"{label:<30} {t:.3f}s ({t/N*1e9:.1f} ns/call)")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:2>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: (none)
Type: New feature | Status: new
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by zky):

Hi Johannes, thanks for providing such clear benchmarks! The performance
gains for the task queue are very significant. I'm quite interested in
this and would love to help take the implementation work off your hands.
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:3>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------

Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned

Component: Tasks | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by zky):

* owner: (none) => zky
* stage: Unreviewed => Accepted
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:4>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Comment (by zky):

Hi Johannes,

Regarding the sorting implementation, while @dataclass(order=True) is
indeed stupidly simple, I noticed two potential issues.

First, it makes the sorting logic strictly dependent on the physical order
of the class attributes, which could cause regressions if someone
accidentally reorders them in the future.

More importantly, since run_after is typed as datetime | None, using
order=True will crash the queue with a TypeError (comparing NoneType and
datetime) whenever a task scheduled to run immediately (run_after=None) is
compared against a scheduled task.

To prevent the queue from crashing and to make the codebase more
defensive, would it be safer to explicitly implement __lt__ and __eq__? We
can safely handle the None fallback inside __lt__ by comparing tuples like
(self.run_after is not None, self.run_after).

Let me know your thoughts!
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:5>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Comment (by Johannes Maron):

Hi there,

Good call. The dataclasses factory utilities are mainly supposed to reduce
boilerplate. They just create classes with the very same methods for you.
If it gets in the way, as it seems to do here, it is an excellent choice
to do things “manually.”

Idioms and tools should never get in your way. That sentiment speaks much
to the core of Django and its success. In other words, I believe you are
on the right path :)

Cheers!
Joe
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:6>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Comment (by zky):

Replying to [comment:6 Johannes Maron]:

> Hi there,
>
> Good call. The dataclasses factory utilities are mainly supposed to
reduce boilerplate. They just create classes with the very same methods
for you. If it gets in the way, as it seems to do here, it is an excellent
choice to do things “manually.”
>
> Idioms and tools should never get in your way. That sentiment speaks
much to the core of Django and its success. In other words, I believe you
are on the right path :)
>
> Cheers!
> Joe

Thank you for the encouraging words! I completely agree—pragmatism over
strict adherence to tools is exactly what makes Django's design philosophy
so great to work with. I've already submitted the PR：
https://github.com/django/django/pull/21383
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:7>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by zky):

* has_patch: 0 => 1

Comment:

https://github.com/django/django/pull/21383
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:8>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by Johannes Maron):

* needs_better_patch: 0 => 1
* needs_docs: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:9>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by zky):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:10>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by Johannes Maron):

* needs_better_patch: 0 => 1

* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:11>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by zky):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:12>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by Johannes Maron):

* needs_better_patch: 0 => 1

* needs_tests: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:13>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------
Changes (by Johannes Maron):

* needs_docs: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:14>

[Django]

#37126: Make Task and TaskResult comparable

-------------------------------------+-------------------------------------

Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:15>

[Django]

#37126: Make Task and TaskResult comparable

--------------------------------+------------------------------------

Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0

--------------------------------+------------------------------------
Changes (by Johannes Maron):

* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:16>

[Django]

#37126: Make Task and TaskResult comparable
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------------

Changes (by zky):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:17>

[Django]

#37126: Make Task and TaskResult comparable

-------------------------------------+-------------------------------------

Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: assigned
Component: Tasks | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:18>

[Django]

#37126: Make Task and TaskResult comparable
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: zky

Type: New feature | Status: closed

Component: Tasks | Version: dev
Severity: Normal | Resolution:

| needsnewfeatureprocess
Keywords: | Triage Stage:
| Unreviewed

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* cc: Jake Howard (added)
* resolution: => needsnewfeatureprocess
* stage: Ready for checkin => Unreviewed
* status: assigned => closed

Comment:

Apologies if I have missed further discussion here but I believe this new
feature got accepted without clear consensus

There is a forum discussion but from what I can see:

> Make task results comparable: This one is interesting to me - what value
do you see in them being comparable? It’s more than just priority - in ORM
speak it should be `[F("priority").desc(), F("run_after").asc(),
F("enqueued_at").asc()]`. I’m not opposed to implementing that, but I’m
not sure I see the value (at least wide enough to be implemented in core).

I see `PriorityQueue` has been mentioned in this ticket, but not the forum
discussion, which might be the main motivation perhaps missed there.
I don't see a discussion on https://github.com/django/new-features/issues

I am going to mark this as `needsnewfeatureprocess`. We can reopen once it
is clearer this is something we want support for
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:19>

[Django]

#37126: Make Task and TaskResult comparable
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: closed
Component: Tasks | Version: dev
Severity: Normal | Resolution:
| needsnewfeatureprocess
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Johannes Maron):

I think Jake just used the ORM example to communicate how it should be
sorted; I think there was no misunderstanding about implementing `__lt__`.

Anyhow, is there a question about the why? Well, because a collection of
priority tasks should be sortable. The API `priotity` argument directly
implies this functionality. Therefore, I still don't think it's a new
feature; we already have consensus on having a task order; it simply
wasn't implemented properly. Hence the ticket here.
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:20>

[Django]

#37126: Make Task and TaskResult comparable
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: zky
Type: New feature | Status: closed
Component: Tasks | Version: dev
Severity: Normal | Resolution:
| needsnewfeatureprocess
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Johannes Maron):

To better illustrate my point, forget the priority queue: I'd currently
expect this to work:

{{{

>>> meh = Task(priority=3)
>>> super_important.= Task(priority=1)
>>> sorted([meh, super_important])
... [super_important, meh]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37126#comment:21>