Page MenuHomeSoftware Heritage

swh.scheduler.cli: Add `swh scheduler task-type register` cli
ClosedPublic

Authored by ardumont on Nov 18 2019, 10:24 AM.

Details

Summary

This allows registering of workers' task types within the scheduler backend.

Those are declared in their respective setup.py's entrypoint under the section [swh.workers] .

(This is moved from swh.lister to swh.scheduler).

Test Plan

tox

Diff Detail

Repository
rDSCH Scheduling utilities
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9112
Build 13324: tox-on-jenkinsJenkins
Build 13323: arc lint + arc unit

Event Timeline

swh/scheduler/cli/task_type.py
80

I did not find a way to make that actually a "choice".
So the current code loads all that is found through the pkg_resources mechanism.

  • cli.task_type: Make the cli user able to choose the worker to load
ardumont edited the summary of this revision. (Show Details)

Add test on this use case (and refactor a bit the tests)

douardda added inline comments.
swh/scheduler/__init__.py
74 ↗(On Diff #7883)

May I suggest to find a better name here? I mean load_xxx_module makes the reader think this function will load these plugin python modules (aka import them) which it does not.

It loads known swh.workers plugin declarations.

In fact, I'm not even sure this function is useful. The diffs below like

- for entrypoint in pkg_resources.iter_entry_points('swh.workers'):
+ for entrypoint in load_worker_modules().values():

looks not so useful in terms of code clarity/readability.

swh/scheduler/cli/task_type.py
35

naming again: these are not worker modules, and not even plugin (python) modules, but worker plugin descriptions.
The name should be more explicit.

92

/nitpicky/ this worker_modules local variable is useless IMHO.

This revision now requires changes to proceed.Nov 18 2019, 1:58 PM
  • swh.scheduler.cli.task_type: Improve naming

Remove indirection and squash commits.

swh/scheduler/cli/task_type.py
80

It's now a real "choice".

This revision is now accepted and ready to land.Nov 19 2019, 1:48 PM