Page MenuHomeSoftware Heritage
Differential D4284

scheduler: Type and unify get_scheduler factory with other factories
ClosedPublic
Actions

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

Details

Reviewers
tenma
Group Reviewers
Reviewers
Maniphest Tasks
T1410: Kill implicit configuration: new configuration scheme
Commits
rDSCH13dcaddbed81: scheduler: Type and unify get_scheduler factory with other factories
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 16373
Build 25215: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 25214: arc lint + arc unit

Event Timeline

ardumont created this revision.Oct 16 2020, 1:13 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 16 2020, 1:13 PM
swh-public-ci added a comment.Oct 16 2020, 1:15 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.

Harbormaster completed remote builds in B16373: Diff 15145.Oct 16 2020, 1:15 PM
vlorentz added a subscriber: vlorentz.Oct 16 2020, 1:18 PM
vlorentz added inline comments.
swh/scheduler/__init__.py
46

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.Oct 16 2020, 1:29 PM
swh/scheduler/__init__.py
46

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.Oct 16 2020, 1:37 PM
  • Rename to BACKEND_TYPES
  • Adapt docstring according to review
  • Fix one config left in the old format
ardumont edited the summary of this revision. (Show Details)Oct 16 2020, 1:39 PM
ardumont added a parent revision: D4286: test_server: Simplify exception manipulations.
swh-public-ci added a comment.Oct 16 2020, 1:39 PM

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.

Harbormaster completed remote builds in B16377: Diff 15149.Oct 16 2020, 1:39 PM
ardumont updated this revision to Diff 15151.Oct 16 2020, 1:49 PM

Rebase

swh-public-ci added a comment.Oct 16 2020, 1:53 PM

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.

Harbormaster completed remote builds in B16379: Diff 15151.Oct 16 2020, 1:53 PM
ardumont updated this revision to Diff 15153.Oct 16 2020, 1:58 PM

Align rpc server configuration as well

swh-public-ci added a comment.Oct 16 2020, 2:00 PM

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.

Harbormaster completed remote builds in B16381: Diff 15153.Oct 16 2020, 2:00 PM
ardumont mentioned this in D4290: docker/conf: Migrate configuration to the latest schema change.Oct 16 2020, 3:52 PM
tenma added a subscriber: tenma.Oct 16 2020, 6:11 PM
tenma added inline comments.
swh/scheduler/__init__.py
46

@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!

ardumont added inline comments.Oct 16 2020, 6:19 PM
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.

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

Drop unneeded instruction

ardumont marked an inline comment as done.Oct 16 2020, 6:23 PM
swh-public-ci added a comment.Oct 16 2020, 6:25 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.

Harbormaster completed remote builds in B16403: Diff 15174.Oct 16 2020, 6:25 PM
tenma accepted this revision as: tenma.Oct 16 2020, 6:35 PM
This revision is now accepted and ready to land.Oct 16 2020, 6:35 PM
Closed by commit rDSCH13dcaddbed81: scheduler: Type and unify get_scheduler factory with other factories (authored by ardumont). · Explain WhyOct 19 2020, 9:21 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDSCH13dcaddbed81: scheduler: Type and unify get_scheduler factory with other factories.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
1 line--
swh/
scheduler/
55 lines96%
Loading...
5 lines66%
Loading...
2 lines100%
Loading...
tests/
es/
2 lines96%
Loading...
2 lines93%
Loading...
78 lines92%
Loading...

Diff 15145

View Options

requirements.txt

Loading...
View Options

swh/scheduler/__init__.py

Loading...
View Options

swh/scheduler/interface.py

Loading...
View Options

swh/scheduler/pytest_plugin.py

Loading...
View Options

swh/scheduler/tests/es/conftest.py

Loading...
View Options

swh/scheduler/tests/test_cli_task_type.py

Loading...
View Options

swh/scheduler/tests/test_init.py

Loading...
About · Developer information · Legal