Page MenuHomeSoftware Heritage
Differential D3056

Deprecate the `config-path` argument of the `swh storage rpc-serve` command
ClosedPublic
Actions

Authored by douardda on Apr 23 2020, 3:31 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2355: Make swh-journal independent from swh-storage or swh-objstorage
Commits
rDSTO5fd9b563298f: Deprecate the `config-path` argument of the `swh storage rpc-serve` command
Summary

in favor of the standard --config-file option of swh storage.

Attempt to write a couple of tests for the rpc-serve command.

Depends on D3055

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

douardda created this revision.Apr 23 2020, 3:31 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 23 2020, 3:31 PM
swh-public-ci added a comment.Apr 23 2020, 3:48 PM

Build is green

Patch application report for D3056 (id=10860)

Could not rebase; Attempt merge onto fe56005555...

Updating fe56005..1904fa7
Fast-forward
 mypy.ini                           |   3 +
 swh/storage/backfill.py            | 490 +++++++++++++++++++++++++++++++++++++
 swh/storage/cli.py                 |  92 ++++++-
 swh/storage/tests/test_backfill.py | 160 ++++++++++++
 swh/storage/tests/test_cli.py      |  99 ++++++++
 5 files changed, 839 insertions(+), 5 deletions(-)
 create mode 100644 swh/storage/backfill.py
 create mode 100644 swh/storage/tests/test_backfill.py
 create mode 100644 swh/storage/tests/test_cli.py
Changes applied before test
commit 1904fa77545acf9c7d9bb37e44c6b6ef9a6605be
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 23 15:27:28 2020 +0200

    Deprecate the `config-path` argument of the `swh storage rpc-serve` command
    
    in favor of the standard `--config-file` option of `swh storage`.
    
    Attempt to write a couple of tests for the rpc-serve command.

commit cd32cf42538ff256bed718fd747e289c9a311ebf
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Apr 22 16:57:07 2020 +0200

    Normalize the configuration file handling in the `swh storage` CLI command
    
    Almost every other swh package handles the loading of the config file
    from the main click command group of the package.
    So we make storage behaves the same.

commit 698af8ca0028c5847326cf57f867608e533af9ed
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Apr 10 11:03:21 2020 +0200

    cli: rename the command 'backfiller' as 'backfill'
    
    for the sake of consistency.

commit b2bba450aaa808406b024800dcd50b4778313903
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 9 16:56:44 2020 +0200

    Copy the backfiller component from swh-journal
    
    This componant makes more sense in the swh-storage package.

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

Harbormaster completed remote builds in B12014: Diff 10860.Apr 23 2020, 3:48 PM
olasd added a subscriber: olasd.Apr 23 2020, 3:54 PM

Jeez. The check_rpc_serve context manager is so backwards it broke my brain a little. Took me quite a while to figure out that the thread actually runs the test rather than the server. I also think the synchronization between the test and the server initialization should be done more explicitly.

(but I don't really have a better thing to suggest right now)

douardda added a comment.Apr 23 2020, 4:12 PM
In D3056#74217, @olasd wrote:

Jeez. The check_rpc_serve context manager is so backwards it broke my brain a little. Took me quite a while to figure out that the thread actually runs the test rather than the server. I also think the synchronization between the test and the server initialization should be done more explicitly.

(but I don't really have a better thing to suggest right now)

Well there is a comment :-)

douardda updated this revision to Diff 10868.Apr 23 2020, 4:26 PM

Add more comments in the convoluted test code

swh-public-ci added a comment.Apr 23 2020, 4:39 PM

Build is green

Patch application report for D3056 (id=10868)

Could not rebase; Attempt merge onto fe56005555...

Updating fe56005..851540b
Fast-forward
 mypy.ini                           |   3 +
 swh/storage/backfill.py            | 490 +++++++++++++++++++++++++++++++++++++
 swh/storage/cli.py                 |  92 ++++++-
 swh/storage/tests/test_backfill.py | 160 ++++++++++++
 swh/storage/tests/test_cli.py      | 113 +++++++++
 5 files changed, 853 insertions(+), 5 deletions(-)
 create mode 100644 swh/storage/backfill.py
 create mode 100644 swh/storage/tests/test_backfill.py
 create mode 100644 swh/storage/tests/test_cli.py
Changes applied before test
commit 851540b5335464bf32dfffc2d733af4ce8424edf
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 23 15:27:28 2020 +0200

    Deprecate the `config-path` argument of the `swh storage rpc-serve` command
    
    in favor of the standard `--config-file` option of `swh storage`.
    
    Attempt to write a couple of tests for the rpc-serve command.

commit cd32cf42538ff256bed718fd747e289c9a311ebf
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Apr 22 16:57:07 2020 +0200

    Normalize the configuration file handling in the `swh storage` CLI command
    
    Almost every other swh package handles the loading of the config file
    from the main click command group of the package.
    So we make storage behaves the same.

commit 698af8ca0028c5847326cf57f867608e533af9ed
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Apr 10 11:03:21 2020 +0200

    cli: rename the command 'backfiller' as 'backfill'
    
    for the sake of consistency.

commit b2bba450aaa808406b024800dcd50b4778313903
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 9 16:56:44 2020 +0200

    Copy the backfiller component from swh-journal
    
    This componant makes more sense in the swh-storage package.

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

Harbormaster completed remote builds in B12019: Diff 10868.Apr 23 2020, 4:39 PM
douardda updated this revision to Diff 10873.Apr 23 2020, 4:50 PM

yet more comments

is this a bad small? well...

swh-public-ci added a comment.Apr 23 2020, 5:02 PM

Build is green

Patch application report for D3056 (id=10873)

Could not rebase; Attempt merge onto fe56005555...

Updating fe56005..e245a46
Fast-forward
 mypy.ini                           |   3 +
 swh/storage/backfill.py            | 490 +++++++++++++++++++++++++++++++++++++
 swh/storage/cli.py                 |  92 ++++++-
 swh/storage/tests/test_backfill.py | 160 ++++++++++++
 swh/storage/tests/test_cli.py      | 118 +++++++++
 5 files changed, 858 insertions(+), 5 deletions(-)
 create mode 100644 swh/storage/backfill.py
 create mode 100644 swh/storage/tests/test_backfill.py
 create mode 100644 swh/storage/tests/test_cli.py
Changes applied before test
commit e245a465589fa24288bb62bd57aaf925d665c353
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 23 15:27:28 2020 +0200

    Deprecate the `config-path` argument of the `swh storage rpc-serve` command
    
    in favor of the standard `--config-file` option of `swh storage`.
    
    Attempt to write a couple of tests for the rpc-serve command.

commit cd32cf42538ff256bed718fd747e289c9a311ebf
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Apr 22 16:57:07 2020 +0200

    Normalize the configuration file handling in the `swh storage` CLI command
    
    Almost every other swh package handles the loading of the config file
    from the main click command group of the package.
    So we make storage behaves the same.

commit 698af8ca0028c5847326cf57f867608e533af9ed
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Apr 10 11:03:21 2020 +0200

    cli: rename the command 'backfiller' as 'backfill'
    
    for the sake of consistency.

commit b2bba450aaa808406b024800dcd50b4778313903
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 9 16:56:44 2020 +0200

    Copy the backfiller component from swh-journal
    
    This componant makes more sense in the swh-storage package.

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

Harbormaster completed remote builds in B12024: Diff 10873.Apr 23 2020, 5:02 PM
ardumont added a subscriber: ardumont.Apr 24 2020, 10:28 AM

A tad complicated to follow but that does the job, so fine by me.
One question about further check.

swh/storage/tests/test_cli.py
1

2020

wondering if there ain't some pre-commit plugin which either checks (or even updates) that...

104

can't we check for the / endpoint to return "Software Heritage Storage server" or some such?

This one:

$ curl http://storage0.internal.staging.swh.network:5002/
<html>
<head><title>Software Heritage storage server</title></head>
<body>
<p>You have reached the
<a href="https://www.softwareheritage.org/">Software Heritage</a>
storage server.<br />
See its
<a href="https://docs.softwareheritage.org/devel/swh-storage/">documentation
and API</a> for more information</p>
</body>
</html>%
douardda added inline comments.Apr 24 2020, 11:20 AM
swh/storage/tests/test_cli.py
1

thx

104

sure

douardda updated this revision to Diff 10876.Apr 24 2020, 11:29 AM
douardda added a task: T2355: Make swh-journal independent from swh-storage or swh-objstorage.

Adapt according to ardumont's comments

ardumont accepted this revision.Apr 24 2020, 11:30 AM
This revision is now accepted and ready to land.Apr 24 2020, 11:30 AM
swh-public-ci added a comment.Apr 24 2020, 11:41 AM

Build is green

Patch application report for D3056 (id=10876)

Could not rebase; Attempt merge onto fe56005555...

Updating fe56005..5fd9b56
Fast-forward
 mypy.ini                           |   3 +
 swh/storage/backfill.py            | 490 +++++++++++++++++++++++++++++++++++++
 swh/storage/cli.py                 |  92 ++++++-
 swh/storage/tests/test_backfill.py | 160 ++++++++++++
 swh/storage/tests/test_cli.py      | 119 +++++++++
 5 files changed, 859 insertions(+), 5 deletions(-)
 create mode 100644 swh/storage/backfill.py
 create mode 100644 swh/storage/tests/test_backfill.py
 create mode 100644 swh/storage/tests/test_cli.py
Changes applied before test
commit 5fd9b563298f1068636ba57be10d2f1975f09781
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 23 15:27:28 2020 +0200

    Deprecate the `config-path` argument of the `swh storage rpc-serve` command
    
    in favor of the standard `--config-file` option of `swh storage`.
    
    Attempt to write a couple of tests for the rpc-serve command.

commit cd32cf42538ff256bed718fd747e289c9a311ebf
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Apr 22 16:57:07 2020 +0200

    Normalize the configuration file handling in the `swh storage` CLI command
    
    Almost every other swh package handles the loading of the config file
    from the main click command group of the package.
    So we make storage behaves the same.

commit 698af8ca0028c5847326cf57f867608e533af9ed
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Apr 10 11:03:21 2020 +0200

    cli: rename the command 'backfiller' as 'backfill'
    
    for the sake of consistency.

commit b2bba450aaa808406b024800dcd50b4778313903
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 9 16:56:44 2020 +0200

    Copy the backfiller component from swh-journal
    
    This componant makes more sense in the swh-storage package.

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

Harbormaster completed remote builds in B12028: Diff 10876.Apr 24 2020, 11:41 AM
Closed by commit rDSTO5fd9b563298f: Deprecate the `config-path` argument of the `swh storage rpc-serve` command (authored by douardda). · Explain WhyApr 24 2020, 11:54 AM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDSTO5fd9b563298f: Deprecate the `config-path` argument of the `swh storage rpc-serve` command.

Revision Contents
Changeset List

PathSize
swh/
storage/
17 lines
tests/
119 lines

Diff 10882

View Options

swh/storage/cli.py

Loading...
View Options

swh/storage/tests/test_cli.py

Loading...
About · Developer information · Legal