Opened 3 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#37206 closed Cleanup/optimization (needsnewfeatureprocess)

Drop tasks' is_module_level_function in favor of the established deconstructable paradigm

Reported by: Johannes Maron Owned by:
Component: Tasks Version: 6.0
Severity: Normal Keywords:
Cc: Johannes Maron, Jake Howard Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The task framework introduces a new is_module_level_function inspection function instead of relying on the established deconstructible paradigm.

Django's task functions (Task.func) need to be serializable. However, Django has an established paradigm for how to do this since the introduction of migrations: deconstructible classes that need to either implement deconstruct or use the @deconstructalbe decorator.

Yes, it will also add support for callables, but more importantly, we don't introduce too many serialization paradigms internally and provide integrators a familiar interface.

Change History (4)

comment:1 by Sarah Boyce, 3 weeks ago

Cc: Jake Howard added

Sorry Johannes, can you clarify what changes you are expecting here in terms of what would be supported?
I think this is saying you would like support to be added for something like

from django.utils.deconstruct import deconstructible


@deconstructible
class NoopCallable:
    def __init__(self, value):
        self.value = value

    def __call__(self, *args, **kwargs):
        pass


my_task = task(foo=5, bar=600)(NoopCallable(value=123))

And that the check in BaseTaskBackend.validate_task might become:

if not is_module_level_function(task.func) and not hasattr(task.func, "deconstruct"):
    raise InvalidTask(
        "Task function must be a module-level function or deconstructible callable."
    )

Is that right? Can you add an example to illustrate in case I have misunderstood?

comment:2 by Johannes Maron, 3 weeks ago

Yes, that looks about right. Just like you'd write a validator in the ORM, right 🤷‍♂️

I stumbled upon this when implementing retry callbacks in Threadmill. There using callable objects is really handy. BUT I don't think there is a real use case for tasks. But I feel like serialization should be handled as consistently as possible across Django. Which is why I didn't sell it as a feature; I don't believe there is one. This is just my OCD.

comment:3 by Jake Howard, 3 weeks ago

What is being serialized here is different. For a task, it's a function, where as for deconstructible it's a class. You can't use deconstructible on a function, and I don't think there's really value in defining tasks as callable classes (given no other aspect of a class is usable for a task).

I could perhaps see some value in making Task and TaskResult deconstructible, since those are classes with some kind of serialization, however that was recently handled in #36919 separately.

comment:4 by Johannes Maron, 3 weeks ago

Resolution: needsnewfeatureprocess
Status: newclosed

Hm… yes, on second thought. It would probably add complexity to the implementation.
I will check if we can reuse the is_module_level_function somewhere else in the codebase and close this.

That being said, I have thought about it some during a walk; someone could use callable tasks to dynamically build parametrized tasks. If you can think of it, someone will probably try to do it. However, that would definitely shift this to a new feature.

Last edited 3 weeks ago by Johannes Maron (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top