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
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.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–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/

ardumont added inline comments.Oct 16 2020, 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).

57

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

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

PathSize
1 line
swh/
scheduler/
57 lines
api/
22 lines
5 lines
2 lines
tests/
es/
2 lines
2 lines
77 lines
33 lines

Diff 15182

View Options

requirements.txt

Loading...
View Options

swh/scheduler/__init__.py

Loading...
View Options

swh/scheduler/api/server.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...
View Options

swh/scheduler/tests/test_server.py

Loading...
About · Developer information · Legal