Page MenuHomeSoftware Heritage

scheduler: Type and unify get_scheduler factory with other factories
ClosedPublic

Authored by ardumont on Oct 16 2020, 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
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16379
Build 25227: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 25226: arc lint + arc unit

Event Timeline

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"

57

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

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).

57

thx

  • 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.

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 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
24

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

swh/scheduler/tests/test_init.py
24

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.

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.

This revision is now accepted and ready to land.Oct 16 2020, 6:35 PM