Page MenuHomeSoftware Heritage

asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
ClosedPublic

Authored by ardumont on Oct 19 2020, 9:59 AM.

Details

Summary

This will allow to align the few modules declaring client/server code (redundantly) with the
other rpc servers (e.g. vault, objstorage).

Test Plan

tox

Diff Detail

Repository
rDCORE Foundations and core functionalities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D4299 (id=15187)

Could not rebase; Attempt merge onto 1657fb827f...

Updating 1657fb8..604a08c
Fast-forward
 swh/core/api/asynchronous.py                       | 128 +++++++++++++++++---
 swh/core/api/tests/conftest.py                     |   3 +
 swh/core/api/tests/test_async.py                   |  38 +++---
 swh/core/api/tests/test_rpc_server_asynchronous.py | 132 +++++++++++++++++++++
 tox.ini                                            |   1 +
 5 files changed, 261 insertions(+), 41 deletions(-)
 create mode 100644 swh/core/api/tests/conftest.py
 create mode 100644 swh/core/api/tests/test_rpc_server_asynchronous.py
Changes applied before test
commit 604a08cbe80439b35ce0197805b6a4c536d96e62
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

commit 7f928a07d91460efbbfbc6c5264a4fdd60e58be2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 18 10:25:01 2020 +0200

    api.tests.test_async: Simplify fixture setup

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

Are there tests for the backend_factory/backend_class stuff?

swh/core/api/asynchronous.py
44–48

not tested?

103–161

make this a constant at the class or module level so it doesn't bloat the constructor

169–171

that'sa weird logic. It means that if backend_factory is not None but backend_class is, then backend_factory is ignored?

swh/core/api/asynchronous.py
44–48

I initially added a comment saying i'll add tests on that...
But i think it's actually not used.

I'll check ;)

169–171

yes, strange, coming from the existing rpc server...

Maybe we should make those backend_factory and backend_class mandatory.
That's how they are used anyway iirc.

backend_factory sample: get_storage, etc...
backend_class: StorageInterface, ... (that's actually our interfaces now)

swh/core/api/asynchronous.py
169–171

maybe mandatory for backend_class but not the backend_factory...

swh/core/api/asynchronous.py
169–171

for the record, i was referring to this piece of code earlier [1]

[1] https://forge.softwareheritage.org/source/swh-core/browse/master/swh/core/api/__init__.py$434-436

swh/core/api/asynchronous.py
103–161

Hmm, unsure on how to properly do that? I'm using the self.extra_type_encoders here.

Drop unneeded function (most likely an intermediary method i forgot to clean up)

swh/core/api/asynchronous.py
44–48

encode_data was not needed, so dropped.

Build is green

Patch application report for D4299 (id=15217)

Could not rebase; Attempt merge onto 7f928a07d9...

Updating 7f928a0..b920fae
Fast-forward
 mypy.ini                                           |   3 +
 requirements-db.txt                                |   1 +
 requirements-test-db.txt                           |   2 +-
 swh/core/api/asynchronous.py                       | 115 +++++++++++++++---
 swh/core/api/tests/conftest.py                     |   3 +
 swh/core/api/tests/test_rpc_server_asynchronous.py | 132 +++++++++++++++++++++
 swh/core/db/db_utils.py                            | 104 +++++++++++++++-
 swh/core/db/pytest_plugin.py                       |  59 +++++++++
 swh/core/db/tests/conftest.py                      |   2 +
 swh/core/db/tests/data/0-schema.sql                |  13 ++
 swh/core/db/tests/data/1-data.sql                  |  10 ++
 swh/core/db/tests/test_db_utils.py                 |  56 +++++++++
 tox.ini                                            |   1 +
 13 files changed, 482 insertions(+), 19 deletions(-)
 create mode 100644 swh/core/api/tests/conftest.py
 create mode 100644 swh/core/api/tests/test_rpc_server_asynchronous.py
 create mode 100644 swh/core/db/pytest_plugin.py
 create mode 100644 swh/core/db/tests/data/0-schema.sql
 create mode 100644 swh/core/db/tests/data/1-data.sql
 create mode 100644 swh/core/db/tests/test_db_utils.py
Changes applied before test
commit b920faec4c4d9ee0b871e9fd6c3f26ec2dd0d1d8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

commit 28f8e7ab54cdeb7ed6d6f474bb6c89e891638236
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 18 14:46:39 2020 +0200

    Install postgresql_fact fixture for faster postgres tests
    
    This moves the SWHDatabaseJanitor from swh.storage to swh.core. This also adds
    tests to it.
    
    This will allow to share the behavior on other swh services with postgres
    backends (vault, scheduler, etc...).

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

Add missing coverage in impacted code

Build is green

Patch application report for D4299 (id=15218)

Could not rebase; Attempt merge onto 7f928a07d9...

Updating 7f928a0..afdd9e6
Fast-forward
 mypy.ini                                           |   3 +
 requirements-db.txt                                |   1 +
 requirements-test-db.txt                           |   2 +-
 swh/core/api/asynchronous.py                       | 115 +++++++++++++++---
 swh/core/api/tests/conftest.py                     |   3 +
 swh/core/api/tests/test_async.py                   |  25 +++-
 swh/core/api/tests/test_rpc_server_asynchronous.py | 132 +++++++++++++++++++++
 swh/core/db/db_utils.py                            | 104 +++++++++++++++-
 swh/core/db/pytest_plugin.py                       |  59 +++++++++
 swh/core/db/tests/conftest.py                      |   2 +
 swh/core/db/tests/data/0-schema.sql                |  13 ++
 swh/core/db/tests/data/1-data.sql                  |  10 ++
 swh/core/db/tests/test_db_utils.py                 |  56 +++++++++
 tox.ini                                            |   1 +
 14 files changed, 506 insertions(+), 20 deletions(-)
 create mode 100644 swh/core/api/tests/conftest.py
 create mode 100644 swh/core/api/tests/test_rpc_server_asynchronous.py
 create mode 100644 swh/core/db/pytest_plugin.py
 create mode 100644 swh/core/db/tests/data/0-schema.sql
 create mode 100644 swh/core/db/tests/data/1-data.sql
 create mode 100644 swh/core/db/tests/test_db_utils.py
Changes applied before test
commit afdd9e689ab45cdeb837b65aa6e034edadc101f4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

commit 28f8e7ab54cdeb7ed6d6f474bb6c89e891638236
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 18 14:46:39 2020 +0200

    Install postgresql_fact fixture for faster postgres tests
    
    This moves the SWHDatabaseJanitor from swh.storage to swh.core. This also adds
    tests to it.
    
    This will allow to share the behavior on other swh services with postgres
    backends (vault, scheduler, etc...).

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

swh/core/api/asynchronous.py
103–161

oh right. Then make it a method.

The point is, I like to have the super() call as close as possible to the beginning (or the end) of a function

  • Rebase
  • extract renderers into a method to simplify the constructor reading

Build is green

Patch application report for D4299 (id=15235)

Could not rebase; Attempt merge onto 7f928a07d9...

Updating 7f928a0..c8714b9
Fast-forward
 mypy.ini                                           |   3 +
 requirements-db.txt                                |   1 +
 requirements-test-db.txt                           |   2 +-
 swh/core/api/asynchronous.py                       | 131 ++++++++++++++++----
 swh/core/api/tests/conftest.py                     |   3 +
 swh/core/api/tests/test_async.py                   |  25 +++-
 swh/core/api/tests/test_rpc_server_asynchronous.py | 132 +++++++++++++++++++++
 swh/core/db/db_utils.py                            | 111 ++++++++++++++++-
 swh/core/db/pytest_plugin.py                       |  59 +++++++++
 swh/core/db/tests/data/0-schema.sql                |  16 +++
 swh/core/db/tests/data/1-data.sql                  |  12 ++
 swh/core/db/tests/test_db_utils.py                 |  67 +++++++++++
 tox.ini                                            |   1 +
 13 files changed, 536 insertions(+), 27 deletions(-)
 create mode 100644 swh/core/api/tests/conftest.py
 create mode 100644 swh/core/api/tests/test_rpc_server_asynchronous.py
 create mode 100644 swh/core/db/pytest_plugin.py
 create mode 100644 swh/core/db/tests/data/0-schema.sql
 create mode 100644 swh/core/db/tests/data/1-data.sql
 create mode 100644 swh/core/db/tests/test_db_utils.py
Changes applied before test
commit c8714b9eb1f5d41cd470a988da416f71215d6fe2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

commit 6b311c8f7794a548c68c786e5adb79dff609843a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 18 14:46:39 2020 +0200

    Install postgresql_fact fixture for faster postgres tests
    
    This moves the SWHDatabaseJanitor from swh.storage to swh.core. This also adds
    tests to it.
    
    This will allow to share the behavior on other swh services with postgres
    backends (vault, scheduler, etc...).

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

  • Use the endpoint-path and not the method name as rpc server endpoint
  • Drop the unneeded asyncio coroutine import

Build is green

Patch application report for D4299 (id=15255)

Could not rebase; Attempt merge onto 7f928a07d9...

Updating 7f928a0..4a45166
Fast-forward
 mypy.ini                                           |   3 +
 requirements-db.txt                                |   1 +
 requirements-test-db.txt                           |   2 +-
 swh/core/api/asynchronous.py                       | 130 ++++++++++++++++----
 swh/core/api/tests/conftest.py                     |   3 +
 swh/core/api/tests/test_async.py                   |  25 +++-
 swh/core/api/tests/test_rpc_server_asynchronous.py | 132 +++++++++++++++++++++
 swh/core/db/db_utils.py                            | 111 ++++++++++++++++-
 swh/core/db/pytest_plugin.py                       |  59 +++++++++
 swh/core/db/tests/data/0-schema.sql                |  16 +++
 swh/core/db/tests/data/1-data.sql                  |  12 ++
 swh/core/db/tests/test_db_utils.py                 |  67 +++++++++++
 tox.ini                                            |   1 +
 13 files changed, 535 insertions(+), 27 deletions(-)
 create mode 100644 swh/core/api/tests/conftest.py
 create mode 100644 swh/core/api/tests/test_rpc_server_asynchronous.py
 create mode 100644 swh/core/db/pytest_plugin.py
 create mode 100644 swh/core/db/tests/data/0-schema.sql
 create mode 100644 swh/core/db/tests/data/1-data.sql
 create mode 100644 swh/core/db/tests/test_db_utils.py
Changes applied before test
commit 4a4516643331221d3499b9d9e777b0eb1ca66257
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

commit 6b311c8f7794a548c68c786e5adb79dff609843a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 18 14:46:39 2020 +0200

    Install postgresql_fact fixture for faster postgres tests
    
    This moves the SWHDatabaseJanitor from swh.storage to swh.core. This also adds
    tests to it.
    
    This will allow to share the behavior on other swh services with postgres
    backends (vault, scheduler, etc...).

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

ardumont edited the summary of this revision. (Show Details)
ardumont edited the summary of this revision. (Show Details)
  • Rebase
  • use the endpoint path name as rpc endpoint path (and not the method name)

Build is green

Patch application report for D4299 (id=15288)

Could not rebase; Attempt merge onto 7f928a07d9...

Updating 7f928a0..e55490d
Fast-forward
 mypy.ini                                           |   3 +
 requirements-db.txt                                |   1 +
 requirements-test-db.txt                           |   2 +-
 swh/core/api/asynchronous.py                       | 130 +++++++++++++++----
 swh/core/api/tests/conftest.py                     |   3 +
 swh/core/api/tests/test_async.py                   |  25 +++-
 swh/core/api/tests/test_rpc_server_asynchronous.py | 132 +++++++++++++++++++
 swh/core/db/db_utils.py                            | 118 ++++++++++++++++-
 swh/core/db/pytest_plugin.py                       |  62 +++++++++
 swh/core/db/tests/data/0-schema.sql                |  19 +++
 swh/core/db/tests/data/1-data.sql                  |  15 +++
 swh/core/db/tests/test_db_utils.py                 | 142 +++++++++++++++++++++
 tox.ini                                            |   1 +
 13 files changed, 625 insertions(+), 28 deletions(-)
 create mode 100644 swh/core/api/tests/conftest.py
 create mode 100644 swh/core/api/tests/test_rpc_server_asynchronous.py
 create mode 100644 swh/core/db/pytest_plugin.py
 create mode 100644 swh/core/db/tests/data/0-schema.sql
 create mode 100644 swh/core/db/tests/data/1-data.sql
 create mode 100644 swh/core/db/tests/test_db_utils.py
Changes applied before test
commit e55490d737d1fab7af75d91c80f3e9df0ee86590
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

commit 0c01f4a09284b8843ac238f61f0a9f3ba3a0cd7a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 18 14:46:39 2020 +0200

    Install postgresql_fact fixture for faster postgres tests
    
    This moves the SWHDatabaseJanitor from swh.storage to swh.core. This also adds
    tests to it.
    
    This will allow to share the behavior on other swh services with postgres
    backends (vault, scheduler, etc...).

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

Use the correct repository to sync the diff ¯\_(ツ)_/¯ (again!)

Build has FAILED

Patch application report for D4299 (id=15301)

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

Updating 97a8609..8274a55
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 8274a551a888a36de3afa166732b00c983c670ba
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 16 18:12:18 2020 +0200

    vault.server: Introduce a 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

Link to build: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/33/
See console output for more information: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/33/console

Build is green

Patch application report for D4299 (id=15302)

Rebasing onto 0c01f4a092...

Current branch diff-target is up to date.
Changes applied before test
commit e55490d737d1fab7af75d91c80f3e9df0ee86590
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

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

Adapt according to discussion

Build is green

Patch application report for D4299 (id=15308)

Rebasing onto 0c01f4a092...

Current branch diff-target is up to date.
Changes applied before test
commit 8cf0f1a1cd95060a3e1a78fe3ce58438a4d76f78
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

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

keep the self.backend_class reference

Build is green

Patch application report for D4299 (id=15309)

Rebasing onto 0c01f4a092...

Current branch diff-target is up to date.
Changes applied before test
commit e1a047b5a597a39df1ab2314401e113c95f56af2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

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

vlorentz added inline comments.
swh/core/api/asynchronous.py
131

equivalent to: backend_factory = backend_factory or backend_class

This revision is now accepted and ready to land.Oct 22 2020, 1:12 PM

Simplify tests

swh/core/api/asynchronous.py
131

but but... yeah...
i keep forgetting that form which i prefer btw...

Thanks for yet again telling it to me

Build is green

Patch application report for D4299 (id=15313)

Rebasing onto 0c01f4a092...

Current branch diff-target is up to date.
Changes applied before test
commit 301061aa34c0da261a3cb6228f43cddcec88cfd4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

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

prefer or form to ternary form

Build is green

Patch application report for D4299 (id=15314)

Rebasing onto 0c01f4a092...

Current branch diff-target is up to date.
Changes applied before test
commit 97c091e04da9ec51cbc0b7f1e23dbe61eb553e81
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 17 19:06:57 2020 +0200

    asynchronous.RPCServerApp: Align implementation with api.RPCServerApp
    
    This will allow to align the Vault's client/server code with the other rpc
    servers. And start adding one typed interface with more consistent checks in
    between implementations.

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