Page MenuHomeSoftware Heritage

Explicitly register class-based tasks inheriting from our own class
ClosedPublic

Authored by olasd on Dec 18 2018, 5:32 PM.

Details

Summary

This lets go of the metaclass madness imported from Celery 3, and goes for an
explicit task registration mechanism as advised in Celery 4.

We do explicit registration of all our tasks in the 'worker ready' signal, just
before they automatically subscribe to the task queues.

Unfortunately that signal is not emitted by the "test fixture" worker, so we
need to explicitly register class-based tasks that are being used in tests. This
doesn't show up for functions as the @task decorator handles registration.

Test Plan

same tests as D852: tox, worker with custom config, plain worker with -I <modules>

Diff Detail

Repository
rDSCH Scheduling utilities
Branch
feature/worker-overhaul
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3158
Build 4048: tox-on-jenkinsJenkins
Build 4047: arc lint + arc unit

Event Timeline

We could keep the metaclass to set cls.name, instead of defining it in every class.

Or maybe migrate to using functions decorated with @app.task.

swh/scheduler/celery_backend/config.py
5

Could you just import itertools? chain is also used by celery (which recommends using from celery import chain) so it got me confused for a second when reading the code below

84–114

Adding a docstring to this function would be nice, it's getting non-trivial.

We could keep the metaclass to set cls.name, instead of defining it in every class.

eww :(

I think the few places we need to explicitly set the name (basically, in tests, and when we register the class) means the metaclass would not really save that much.

Or maybe migrate to using functions decorated with @app.task.

We currently use inheritance in a bunch of places to create "task patterns" (the most obvious use of this is in the lister). I agree that's something we c/should reconsider, but for now I focused on making the inter-module diff minimal (only the lister needs a change because the metaclass has been dropped)

swh/scheduler/celery_backend/config.py
84–114

Yes, agreed.

This revision is now accepted and ready to land.Dec 19 2018, 12:40 PM

Pushed as b6bc2f2ae26b6d0bb7768db8f2ffac2002867ca3