Page MenuHomeSoftware Heritage

Deprecate the `args` argument to get_objstorage in favor of kwargs
ClosedPublic

Authored by olasd on Oct 8 2020, 10:01 PM.

Details

Summary

This makes get_objstorage consistent with most of the other get_foo factories
available in other swh modules.

We will need to do a pass to drop the few hundred args that are used
currently.

Test Plan

tox tests updated

Diff Detail

Repository
rDOBJS Object storage
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16085
Build 24741: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 24740: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D4212 (id=14847)

Rebasing onto 6a9896d628...

First, rewinding head to replay your work on top of it...
Applying: Deprecate the `args` argument to get_objstorage in favor of kwargs
Changes applied before test
commit 4c4364c6b0593808d60d3d145710d20f280ad210
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Oct 8 21:59:20 2020 +0200

    Deprecate the `args` argument to get_objstorage in favor of kwargs
    
    This makes get_objstorage consistent with most of the other `get_foo` factories
    available in other swh modules.
    
    We will need to do a pass to drop the few hundred `args` that are used
    currently.

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

Build is green

Patch application report for D4212 (id=14848)

Rebasing onto 6a9896d628...

Current branch diff-target is up to date.
Changes applied before test
commit b78406dcecc4c442fa08ad21e615354e813d5935
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Oct 8 21:59:20 2020 +0200

    Deprecate the `args` argument to get_objstorage in favor of kwargs
    
    This makes get_objstorage consistent with most of the other `get_foo` factories
    available in other swh modules.
    
    We will need to do a pass to drop the few hundred `args` that are used
    currently.

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

ardumont added a subscriber: ardumont.

lgtm, a couple of suggestions inline ;)

swh/objstorage/tests/test_server.py
64

jsyk, we can (and use) and mostly do now the simplified version of:

with pytest.raises(KeyError, match="missing cls config entry"):
    load_and_check_config(config_path)
126

And for this ^ 3 scenarios, I would be tempted to use pytest parametrize functionality [1]:

@pytest.mark.parametrize("config", [
    {"objstorage": {"cls": "pathslicing", "root": "root", "slicing": "slicing"}},
    {"client_max_size": "10", "objstorage": {"cls": "memory", "args": {}}},
    {"client_max_size": "10", "objstorage": {"cls": "memory"}}
])
def test_load_and_check_config(config, tmpdir):
    config_path = prepare_config_file(tmpdir, config)
    cfg = load_and_check_config(config_path)
    assert cfg == config

[1] unsure whether the tmpdir fixture injection is working with this though ¯\_(ツ)_/¯

This revision is now accepted and ready to land.Oct 9 2020, 8:54 AM

Apply ardumont's comments

Build is green

Patch application report for D4212 (id=14855)

Rebasing onto 6a9896d628...

Current branch diff-target is up to date.
Changes applied before test
commit aa19b280840c0bd1e47d9d4741fa76bf9a4df7db
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Oct 9 12:04:38 2020 +0200

    Coalesce successful server config loading tests into a single one

commit 289f9709f5951e28f480f1ae67f2bd60ef14eb26
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Oct 9 12:03:28 2020 +0200

    Replace manual pytest.raises mangling with the match argument

commit b78406dcecc4c442fa08ad21e615354e813d5935
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Oct 8 21:59:20 2020 +0200

    Deprecate the `args` argument to get_objstorage in favor of kwargs
    
    This makes get_objstorage consistent with most of the other `get_foo` factories
    available in other swh modules.
    
    We will need to do a pass to drop the few hundred `args` that are used
    currently.

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