Page MenuHomeSoftware Heritage

celery: auto add tasks declared in the swh.workers entry point in task_modules
ClosedPublic

Authored by douardda on May 22 2019, 2:46 PM.

Details

Summary

allows to declare worker tasks in a 'swh.workers' entry point. This later is expected to be a callable which returns a dict which 'tasks' key is
the module name of the celery tasks exported.

Some other 'hooks' could be provided as callable in this dictionnary,
typically db initialization stuff in listers could be handled that way.

Not sure what a decent 'API' for this shoudl look like for now, still
experimenting.

See D1504 for a use case implementation

Diff Detail

Repository
rDSCH Scheduling utilities
Branch
plugins
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7496
Build 10716: tox-on-jenkinsJenkins
Build 10715: arc lint + arc unit

Event Timeline

vlorentz added inline comments.
swh/scheduler/tests/conftest.py
40

Why not toplevel?

ardumont added inline comments.
swh/scheduler/tests/conftest.py
40

I think the rationale is to implement a poor man's lazy policy ;)

You should mention that with this, we can drop some part of the scheduler's configuration file.
The task_modules in the yaml can be dropped.

Which is something less to worry about in the configuration part... i think ;)

swh/scheduler/celery_backend/config.py
203

I have a little trouble reading that.

Can you explicit what the entrypoint.load() returns...

... plugging brain cables...

Nevermind, i think i grokked it:

  • entrypoint.load() returns the function registered in the setup.py. For example, you dubbed it swh.lister.*.__init__.register in the other diff.
  • and then you call it, to actually access the dict the register function returns

Ok.

Maybe this would be more readable?

for ...:
    worker_register_fn = entrypoint.load()
    CONFIG['task_modules'].append(worker_register_fn()['tasks])

I'm fine with this btw.

Not sure what a decent 'API' for this shoudl look like for now, still experimenting.

neither do i.

swh/scheduler/tests/conftest.py
40

It's a test fixture, it will always run if that file is imported

swh/scheduler/tests/conftest.py
40

heh, sounds about right indeed.

rebased + a very small bit of doc

Now depends on D1714

swh/scheduler/celery_backend/config.py
203

a more explicit version is fine for me, will do

swh/scheduler/tests/conftest.py
40

Why not toplevel?

Why toplevel?

More explicit code for the loading of swh.workers entrypoint

in fact not just a more explcit, allows registry['tasks'] to be either a single string or a list of strings.

swh/scheduler/tests/conftest.py
40

Because imports should always be at the toplevel unless there's a reason not to.

vlorentz requested changes to this revision.Aug 6 2019, 3:26 PM
This revision now requires changes to proceed.Aug 6 2019, 3:26 PM

conftest: Move import statement at the top, as requested by vlorentz

This revision is now accepted and ready to land.Aug 30 2019, 11:10 AM

Use task_modules (a list) as registry entry

This revision was automatically updated to reflect the committed changes.