Page MenuHomeSoftware Heritage
Differential D4329

vault.server: Introduce typed VaultInterface
ClosedPublic
Actions

Authored by ardumont on Oct 22 2020, 1:47 PM.

Details

Reviewers
vlorentz
tenma
Group Reviewers
Reviewers
Maniphest Tasks
T2223: Type checking
Commits
rDVAU08c35ed0f1f4: vault.server: Introduce typed VaultInterface
Summary

Technically, this allows to drop redundant code between rpc server and
rpc client using the base class from swh.core.api.asynchronous.RPCServerApp (which
now hides the detail of aiohttp there). This now matches how other rpc servers
are declared, reducing friction on reading.

Ultimately, the goal is to make this maintenance a bit less heavy than it currently is.
This also adds types so it aligns with other modules.

Related to T2223

Test Plan
tox

then with some docker love:

$ docker-compose up -d
$ cat docker-compose.override.yaml
...
services:
...
  swh-storage-db:
    ports:
      - "5434:5432"

  swh-vault-db:
    ports:
      - "5435:5432"

  swh-vault:
    volumes:
      - "$SWH_ENVIRONMENT_HOME/swh-vault:/src/swh-vault"  # <- contains this diff

  swh-vault-worker:
    volumes:
      - "$SWH_ENVIRONMENT_HOME/swh-vault:/src/swh-vault"
...
$ time doco exec swh-loader swh loader run git https://github.com/ardumont/home-manager
INFO:swh.loader.git.BulkLoader:Load origin 'https://github.com/ardumont/home-manager' with type 'git'
Enumerating objects: 8451, done.
Total 8451 (delta 0), reused 0 (delta 0), pack-reused 8451
INFO:swh.loader.git.BulkLoader:Listed 14 refs for repo https://github.com/ardumont/home-manager
{'status': 'eventful'}
docker-compose exec swh-loader swh loader run git   0.59s user 0.45s system 2% cpu 37.315 total
$ unset PAGER; psql service=swh-dev -c 'select id from directory limit 5'
                     id
--------------------------------------------
 \xa4a3a2a858eb84f2e77b44081c0a00cae7200c37
 \xade428bf6618bc66ac725bad3d14efd92d3f6e82
 \x8077bb0a78ddf494e7adb88ea87b8f9d9eeba15c
 \x336b371f1c7c8b9cdaa1a71b0922e18e9ef7e684
 \x838bc7ca7541fd96ed6e665c1485a0f8b6d4c3f3
$ doco exec swh-scheduler  \
    swh scheduler task add cook-vault-bundle \
    obj_type=directory obj_id=838bc7ca7541fd96ed6e665c1485a0f8b6d4c3f3

Created 1 tasks

Task 4
  Next run: just now (2020-10-22 12:50:04+00:00)
  Interval: 1 day, 0:00:00
  Type: cook-vault-bundle
  Policy: recurring
  Args:
  Keyword args:
    obj_id: '838bc7ca7541fd96ed6e665c1485a0f8b6d4c3f3'
    obj_type: 'directory'
$ doco logs -f swh-vault
...
swh-vault_1                     | ======== Running on http://0.0.0.0:5005 ========
swh-vault_1                     | (Press CTRL+C to quit)
swh-vault_1                     | INFO:aiohttp.access:172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_status HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_status HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_progress HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_progress HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /put_bundle HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /put_bundle HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_status HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_status HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_progress HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /set_progress HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /send_notif HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 172.29.0.30 [22/Oct/2020:12:50:08 +0000] "POST /send_notif HTTP/1.1" 200 147 "-" "python-requests/2.24.0"
...

Diff Detail

Event Timeline

ardumont created this revision.Oct 22 2020, 1:47 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 22 2020, 1:47 PM
ardumont edited the test plan for this revision. (Show Details)Oct 22 2020, 1:48 PM
swh-public-ci added a comment.Oct 22 2020, 1:49 PM

Build is green

Patch application report for D4329 (id=15320)

Could not rebase; Attempt merge onto 97a860997b...

Updating 97a8609..c14a0f2
Fast-forward
 requirements.txt                     |   2 +-
 swh/vault/api/client.py              |  48 +-------
 swh/vault/api/server.py              | 228 ++++++++++-------------------------
 swh/vault/backend.py                 | 176 +++++++++++++++++----------
 swh/vault/cookers/__init__.py        |  21 +++-
 swh/vault/cookers/base.py            |   4 +-
 swh/vault/interface.py               |  68 +++++++++++
 swh/vault/tests/conftest.py          |  17 ++-
 swh/vault/tests/test_backend.py      |  22 +++-
 swh/vault/tests/test_cookers_base.py |   6 +-
 swh/vault/tests/test_init_cookers.py | 109 +++++++++++++++++
 swh/vault/tests/test_server.py       | 128 +++++++++++++++++---
 12 files changed, 520 insertions(+), 309 deletions(-)
 create mode 100644 swh/vault/interface.py
 create mode 100644 swh/vault/tests/test_init_cookers.py
Changes applied before test
commit c14a0f2d6139a55208da6dee53e29d56ce3c7543
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

commit eb868d0e690e9890172b699184bcffb80696fe89
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Oct 20 15:15:52 2020 +0200

    Add tests on current configuration check for cooker instantiation

commit e5e428481c76966cf253d914ca1276c7c9749e69
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 20:06:00 2020 +0200

    api.server: Add types and tests on configuration checks

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

Harbormaster completed remote builds in B16548: Diff 15320.Oct 22 2020, 1:49 PM
ardumont edited the test plan for this revision. (Show Details)Oct 22 2020, 1:51 PM
ardumont edited the summary of this revision. (Show Details)Oct 22 2020, 1:56 PM
ardumont edited the summary of this revision. (Show Details)Oct 22 2020, 2:01 PM
ardumont edited the summary of this revision. (Show Details)Oct 22 2020, 2:03 PM
vlorentz accepted this revision.Oct 22 2020, 2:20 PM
This revision is now accepted and ready to land.Oct 22 2020, 2:20 PM
ardumont edited the summary of this revision. (Show Details)Oct 22 2020, 2:22 PM
ardumont edited the summary of this revision. (Show Details)Oct 22 2020, 2:41 PM
ardumont added a task: T2223: Type checking.
ardumont edited the test plan for this revision. (Show Details)Oct 22 2020, 2:52 PM
ardumont updated this revision to Diff 15334.Oct 22 2020, 4:12 PM
ardumont edited the test plan for this revision. (Show Details)

Fix drop of the configuration client_max_size key by mistake

swh-public-ci added a comment.Oct 22 2020, 4:13 PM

Build is green

Patch application report for D4329 (id=15334)

Rebasing onto eb868d0e69...

Current branch diff-target is up to date.
Changes applied before test
commit 144c1f10c4b7d1a3bb1d2d87109b3f27a38174b3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

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

Harbormaster completed remote builds in B16561: Diff 15334.Oct 22 2020, 4:14 PM
ardumont updated this revision to Diff 15340.Oct 22 2020, 4:43 PM

Add missing coverage

swh-public-ci added a comment.Oct 22 2020, 4:44 PM

Build is green

Patch application report for D4329 (id=15340)

Rebasing onto eb868d0e69...

Current branch diff-target is up to date.
Changes applied before test
commit 4d49f567fde289b3316236a1a6ed65f3bc95fff4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

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

Harbormaster completed remote builds in B16567: Diff 15340.Oct 22 2020, 4:44 PM
tenma edited the summary of this revision. (Show Details)Oct 22 2020, 5:05 PM
ardumont added inline comments.Oct 22 2020, 5:50 PM
swh/vault/api/server.py
178

i've diverged here, all methods are now post.
That's supposedly a detail...
I'm gonna fix it, i need to work in the core for that though.

But i noticed i have failure in docker when triggering cooking from the webapp so, let's be iso for now.

ardumont added inline comments.Oct 22 2020, 5:57 PM
swh/vault/api/server.py
178

D4337

tenma accepted this revision.Oct 22 2020, 6:15 PM
tenma added a subscriber: tenma.

LGTM, but could be splitted

ardumont added inline comments.Oct 22 2020, 6:15 PM
swh/vault/api/server.py
54

@tenma, this got moved within the backend implementation because that needs to be done within the backend now.

The rpc server RPCServerApp is just a decorator in charge of serializing/deserializing data at the right moment.
So nothing more happens here any longer.

To keep the functionality iso, this kind of enriched behavior centralized here (in the prior implementation) needs to go back within the backend.

That's why you see now the ifs, raise NotFoundExc and the returned values in the backend.py code now.
So the existing tests continue passing (almost unchanged).

ardumont updated this revision to Diff 15355.Oct 22 2020, 6:49 PM

Be iso functionality in terms of rpc method (get, post)

swh-public-ci added a comment.Oct 22 2020, 6:50 PM

Build is green

Patch application report for D4329 (id=15355)

Rebasing onto eb868d0e69...

Current branch diff-target is up to date.
Changes applied before test
commit 42822a2e38e0c141d33493b2ba633525920fb8cc
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

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

Harbormaster completed remote builds in B16582: Diff 15355.Oct 22 2020, 6:50 PM
ardumont mentioned this in D4341: rpc_client: Allow http method declaration consistently with the rpc server.Oct 23 2020, 12:28 PM
ardumont updated this revision to Diff 15385.Oct 23 2020, 5:40 PM
  • So tests were fine but the vault use through webapp was not (understand docker tryouts here)
  • so realign code so the existing behavior is sanely kept as before

-> internally, all client code now uses POST method, but that's a detail of the
rpc communication underneath, this does not impact the vault clients

swh-public-ci added a comment.Oct 23 2020, 5:41 PM

Build is green

Patch application report for D4329 (id=15385)

Rebasing onto eb868d0e69...

Current branch diff-target is up to date.
Changes applied before test
commit 595524c9007cc87abf4cad69da3cf8efef0b2561
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

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

Harbormaster completed remote builds in B16612: Diff 15385.Oct 23 2020, 5:41 PM
ardumont updated this revision to Diff 15390.Oct 23 2020, 6:27 PM

Rename type to ObjectId

swh-public-ci added a comment.Oct 23 2020, 6:28 PM

Build is green

Patch application report for D4329 (id=15390)

Rebasing onto 448dfa07cd...

First, rewinding head to replay your work on top of it...
Applying: vault.server: Introduce typed VaultInterface
Changes applied before test
commit 0014eda21bde102544d3ea91292dcb64a17a3083
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

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

Harbormaster completed remote builds in B16617: Diff 15390.Oct 23 2020, 6:28 PM
ardumont updated this revision to Diff 15391.Oct 23 2020, 6:52 PM

Rebase

swh-public-ci added a comment.Oct 23 2020, 6:53 PM

Build is green

Patch application report for D4329 (id=15391)

Rebasing onto 448dfa07cd...

Current branch diff-target is up to date.
Changes applied before test
commit 08c35ed0f1f433b24682d2337e46f6c49476d129
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce typed VaultInterface
    
    This allows to drop redundant code in between rpc server and clients using the
    base class from swh.core.api.async module.

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

Harbormaster completed remote builds in B16618: Diff 15391.Oct 23 2020, 6:53 PM
Closed by commit rDVAU08c35ed0f1f4: vault.server: Introduce typed VaultInterface (authored by ardumont). · Explain WhyOct 23 2020, 6:55 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDVAU08c35ed0f1f4: vault.server: Introduce typed VaultInterface.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
2 lines--
swh/
vault/
api/
48 lines100%
Loading...
253 lines98%
Loading...
176 lines83%
Loading...
cookers/
2 lines94%
Loading...
68 lines71%
Loading...
tests/
20 lines84%
Loading...
22 lines100%
Loading...
6 lines97%
Loading...
108 lines100%
Loading...

Diff 15355

View Options

requirements.txt

Loading...
View Options

swh/vault/api/client.py

Loading...
View Options

swh/vault/api/server.py

Loading...
View Options

swh/vault/backend.py

Loading...
View Options

swh/vault/cookers/base.py

Loading...
View Options

swh/vault/interface.py

Loading...
View Options

swh/vault/tests/conftest.py

Loading...
View Options

swh/vault/tests/test_backend.py

Loading...
View Options

swh/vault/tests/test_cookers_base.py

Loading...
View Options

swh/vault/tests/test_server.py

Loading...
About · Developer information · Legal