Page MenuHomeSoftware Heritage

scheduler: Type and unify get_scheduler factory with other factories
ClosedPublic

Authored by ardumont on Fri, Oct 16, 1:13 PM.

Details

Summary

Impacts:

$SWH_ENVIRONMENT_HOME/swh-deposit/swh/deposit/config.py:        self.scheduler: SchedulerInterface = get_scheduler(**self.config["scheduler"])
$SWH_ENVIRONMENT_HOME/swh-deposit/swh/deposit/tests/conftest.py:        scheduler = get_scheduler(**cfg["scheduler"])
$SWH_ENVIRONMENT_HOME/swh-lister/swh/lister/core/lister_base.py:        self.scheduler = get_scheduler(**self.config["scheduler"])
$SWH_ENVIRONMENT_HOME/swh-scheduler/swh/scheduler/api/server.py:        scheduler = get_scheduler(**app.config["scheduler"])
$SWH_ENVIRONMENT_HOME/swh-scheduler/swh/scheduler/__init__.py:def get_scheduler(cls: str, **kwargs) -> SchedulerInterface:
... # snip scheduler impacts dealt with here
$SWH_ENVIRONMENT_HOME/swh-vault/swh/vault/__init__.py:        args["scheduler"] = get_scheduler(**args["scheduler"])
$SWH_ENVIRONMENT_HOME/swh-web/swh/web/config.py:        swhweb_config["scheduler"] = get_scheduler(**swhweb_config["scheduler"])

Note: Those should still work ok, only a deprecation warning should show up

Related to T1410

Depends on D4286

Diff Detail

Repository
rDSCH Scheduling utilities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Fri, Oct 16, 1:13 PM

Build is green

Patch application report for D4284 (id=15145)

Rebasing onto 315a2c9daf...

Current branch diff-target is up to date.
Changes applied before test
commit c64796332ca7bc9e99745589a49a3adb610eb379
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:12:27 2020 +0200

    scheduler: Type and unify get_scheduler factory with other factories
    
    Related to T1410

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/64/ for more details.

vlorentz added inline comments.
swh/scheduler/__init__.py
46–52

they are paths/names, not types.

also probably rename "component" to "backend" or "class"

58

s/dictionary with keys, default to empty./arguments to pass to the class' constructor/

ardumont added inline comments.Fri, Oct 16, 1:29 PM
swh/scheduler/__init__.py
46–52

well local and remote are specific backend types for me.
It's a map from type to actual fqdn names.

What do you think of, BACKEND_TYPES ?
(also i'd like to unify that name at some point).

58

thx

ardumont updated this revision to Diff 15149.Fri, Oct 16, 1:37 PM
  • Rename to BACKEND_TYPES
  • Adapt docstring according to review
  • Fix one config left in the old format

Build is green

Patch application report for D4284 (id=15149)

Could not rebase; Attempt merge onto 315a2c9daf...

Updating 315a2c9..4ac463b
Fast-forward
 requirements.txt                          |  1 +
 swh/scheduler/__init__.py                 | 57 +++++++++++++++-------
 swh/scheduler/interface.py                |  5 +-
 swh/scheduler/pytest_plugin.py            |  2 +-
 swh/scheduler/tests/es/conftest.py        |  2 +-
 swh/scheduler/tests/test_cli_task_type.py |  2 +-
 swh/scheduler/tests/test_init.py          | 78 +++++++++++++++++++++++++++++++
 swh/scheduler/tests/test_server.py        | 34 +++++---------
 8 files changed, 137 insertions(+), 44 deletions(-)
 create mode 100644 swh/scheduler/tests/test_init.py
Changes applied before test
commit 4ac463b11f06a2f0ae924d66b82f5f7c36edf8fc
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:12:27 2020 +0200

    scheduler: Type and unify get_scheduler factory with other factories
    
    Related to T1410

commit 4fcd407139502035759bee9268cc5499375de356
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:35:54 2020 +0200

    test_server: Simplify exception manipulations

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/66/ for more details.

Build is green

Patch application report for D4284 (id=15151)

Could not rebase; Attempt merge onto 315a2c9daf...

Updating 315a2c9..5e2f8dc
Fast-forward
 requirements.txt                          |  1 +
 swh/scheduler/__init__.py                 | 57 +++++++++++++++-------
 swh/scheduler/interface.py                |  5 +-
 swh/scheduler/pytest_plugin.py            |  2 +-
 swh/scheduler/tests/es/conftest.py        |  2 +-
 swh/scheduler/tests/test_cli_task_type.py |  2 +-
 swh/scheduler/tests/test_init.py          | 78 +++++++++++++++++++++++++++++++
 swh/scheduler/tests/test_server.py        | 42 +++++++----------
 8 files changed, 143 insertions(+), 46 deletions(-)
 create mode 100644 swh/scheduler/tests/test_init.py
Changes applied before test
commit 5e2f8dc07ccc025e282a8629eb1107d09e32b226
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:12:27 2020 +0200

    scheduler: Type and unify get_scheduler factory with other factories
    
    Related to T1410

commit dd33cdcfd266f80cb94eb9b3a32fc2c3f2f63592
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:35:54 2020 +0200

    test_server: Simplify exception manipulations

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/68/ for more details.

ardumont updated this revision to Diff 15153.Fri, Oct 16, 1:58 PM

Align rpc server configuration as well

Build is green

Patch application report for D4284 (id=15153)

Rebasing onto dd33cdcfd2...

Current branch diff-target is up to date.
Changes applied before test
commit 4657cf5b6cfc282615a149decfbed940f71eb96e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:12:27 2020 +0200

    scheduler: Type and unify get_scheduler factory with other factories
    
    Related to T1410

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/69/ for more details.

tenma added a subscriber: tenma.Fri, Oct 16, 6:11 PM
tenma added inline comments.
swh/scheduler/__init__.py
46–52

@vlorentz They are qualified types specified as strings (quoted, a la LISP). Would be equivalent to import beforehand and put the type objects.

swh/scheduler/tests/test_init.py
25

Event if useful, this is very surprising.
One hardly expects a mock fixture to return a mocker instead of the mock!

ardumont added inline comments.Fri, Oct 16, 6:19 PM
swh/scheduler/tests/test_init.py
25

well, i "injected" the mock_psycopg2 by passing it as parameter so i guess my reasoning was "that's probably needed something not None". I did not check if having None returned is a viable solution.

It's used indirectly without specifically needing to manipulate that variable.
I could have also used:

@pytest.usefixture("mock_psycopg2")
def test_init_get_scheduler(class_name, expected_class, kwargs):

I did not check if i could just return None.

ardumont updated this revision to Diff 15174.Fri, Oct 16, 6:23 PM

Drop unneeded instruction

ardumont marked an inline comment as done.Fri, Oct 16, 6:23 PM

Build is green

Patch application report for D4284 (id=15174)

Rebasing onto dd33cdcfd2...

Current branch diff-target is up to date.
Changes applied before test
commit 13dcaddbed8125a2b722a8ca1186853fd77e9cba
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 13:12:27 2020 +0200

    scheduler: Type and unify get_scheduler factory with other factories
    
    Related to T1410

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/70/ for more details.

tenma accepted this revision as: tenma.Fri, Oct 16, 6:35 PM
This revision is now accepted and ready to land.Fri, Oct 16, 6:35 PM