Opened 3 weeks ago
Last modified 2 weeks ago
#36919 assigned Cleanup/optimization
Allow `TaskResult` (and `Task`) to be pickled
| Reported by: | Jake Howard | Owned by: | Varun Kasyap Pentamaraju |
|---|---|---|---|
| Component: | Tasks | Version: | 6.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
As part of implementing some task backends, it can be useful to pickle a TaskResult to pass it around implementation.
Since it's a dataclass most of the implementation is already there - the main issue is that Task itself can't be pickled as it references a function. Replacing that with a string reference during pickling will likely resolve the current issues.
The implementation isn't especially complex, so I'm not opposed to this being closed and left to backend implementers to deal with instead.
Change History (7)
comment:1 by , 3 weeks ago
comment:2 by , 2 weeks ago
| Triage Stage: | Unreviewed → Accepted |
|---|
Thank you Jake, I think the request makes sense. I think we should borrow/reuse from https://github.com/django/django/blob/main/django/db/migrations/serializer.py (see Serializer class).
follow-up: 5 comment:3 by , 2 weeks ago
I'm not sure I understand where DB serialization comes into this? I don't think Task or TaskResult will ever need to be serialized in a migration (and it's probably an antipattern if they do). It did remind me of django.core.serializers, but that seems focused on the ORM, which I also don't think is relevant.
I have a working implementation which could do with some polishing. If someone doesn't beat me to getting a PR up, I'll push something up soon (intentionally leaving unassigned).
follow-up: 6 comment:5 by , 2 weeks ago
Replying to Jake Howard:
Hi Jake, I guess Natalia is just suggesting reusing (or taking inspiration from) some logic inside Serializer, which already converts a function to its dotted import path and includes validation for non-importable cases.
I'm not sure I understand where DB serialization comes into this? I don't think
TaskorTaskResultwill ever need to be serialized in a migration (and it's probably an antipattern if they do). It did remind me ofdjango.core.serializers, but that seems focused on the ORM, which I also don't think is relevant.
comment:6 by , 2 weeks ago
Replying to Varun Kasyap Pentamaraju:
Replying to Jake Howard:
Hi Jake, I guess Natalia is just suggesting reusing (or taking inspiration from) some logic inside Serializer, which already converts a function to its dotted import path and includes validation for non-importable cases.
Yes, thank you! That was my point, we already have robust logic for serializing methods and functions, so we shouldn't (ideally, when possible) re-invent the wheel.
I’d like to investigate this further.
I’ll start by reproducing the pickling failure for Task and TaskResult on the current main branch to better understand the exact limitations.
Before exploring an implementation, I’d appreciate clarification on whether this is something Django core should support directly, or if the expectation is that task backends handle serialization themselves.
I’ll report back with findings shortly.