Page MenuHomeSoftware Heritage

Add a simple alternative "client" in pure-python
ClosedPublic

Authored by vlorentz on May 3 2021, 6:17 PM.

Details

Summary

It allows other components that depend on swh-graph to run their tests
without depending on the WebGraph process itself.

I did not implement all the max_edges/limit stuff because they are not tested. Instead there are TODOs and a task to implement them: T3305

Test Plan

Made test fixtures parametrized, so existing tests also run on this "client"

Event Timeline

Build has FAILED

Patch application report for D5665 (id=20233)

Could not rebase; Attempt merge onto 6f21897a7e...

Updating 6f21897..d15ad6a
Fast-forward
 swh/graph/backend.py               |   3 +
 swh/graph/graph.py                 |   9 +-
 swh/graph/simple_backend.py        | 291 +++++++++++++++++++++++++++++++++++++
 swh/graph/tests/conftest.py        |  49 +++++--
 swh/graph/tests/test_api_client.py |  13 +-
 5 files changed, 349 insertions(+), 16 deletions(-)
 create mode 100644 swh/graph/simple_backend.py
Changes applied before test
commit d15ad6a473d9123367a0931c90f15950798c275e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative backend in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit ba03f35ae34a88eaa1eab18500f4a3f60dddc643
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:10:33 2021 +0200

    Make test_visit_edges_limited less strict
    
    It made too many assumptions on how swh-graph orders edges,
    and would break on implementation changes.
    
    Motivation: I am working on an alternative implementation.

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

Harbormaster returned this revision to the author for changes because remote builds failed.May 3 2021, 6:19 PM
Harbormaster failed remote builds in B21221: Diff 20233!

Rewrite SimpleBackend as a StandaloneClient

Build is green

Patch application report for D5665 (id=20244)

Could not rebase; Attempt merge onto 1a8b00f4cd...

Updating 1a8b00f..73da6e5
Fast-forward
 swh/graph/backend.py               |   3 +
 swh/graph/client.py                |  13 ++
 swh/graph/graph.py                 |   9 +-
 swh/graph/standalone_client.py     | 313 +++++++++++++++++++++++++++++++++++++
 swh/graph/tests/conftest.py        |  34 ++--
 swh/graph/tests/test_api_client.py |  21 ++-
 6 files changed, 372 insertions(+), 21 deletions(-)
 create mode 100644 swh/graph/standalone_client.py
Changes applied before test
commit 73da6e51c63c9296e6a9be9809ca8997d97ade08
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:17:14 2021 +0200

    Rewrite SimpleBackend as a StandaloneClient
    
    It is simpler this way (no need to hack around the internals of the Java proxy),
    and less likely to need refactoring in the future.

commit eab9fdaeb179f00fbe53b8c8926bc5148d6473c1
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative backend in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit 2364c7cd2db7f4610adff0fe7452af46a1df778f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:13:55 2021 +0200

    client: Raise GraphArgumentException on 4xx response, instead of generic RemoteException.
    
    For consistency with other RPC clients, and because RemoteException is too
    generic to be useful (it also covers internal errors of the server, which
    can be temporary errors, unlike GraphArgumentException which is for invalid
    arguments)

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

Build is green

Patch application report for D5665 (id=20246)

Could not rebase; Attempt merge onto 1a8b00f4cd...

Updating 1a8b00f..5217b7d
Fast-forward
 swh/graph/client.py                |  13 ++
 swh/graph/standalone_client.py     | 313 +++++++++++++++++++++++++++++++++++++
 swh/graph/tests/conftest.py        |  34 ++--
 swh/graph/tests/test_api_client.py |  21 ++-
 4 files changed, 363 insertions(+), 18 deletions(-)
 create mode 100644 swh/graph/standalone_client.py
Changes applied before test
commit 5217b7ddb5fddad49fb7ae4ea56b2906c81dd00c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:17:14 2021 +0200

    Rewrite SimpleBackend as a StandaloneClient
    
    It is simpler this way (no need to hack around the internals of the Java proxy),
    and less likely to need refactoring in the future.

commit eab9fdaeb179f00fbe53b8c8926bc5148d6473c1
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative backend in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit 2364c7cd2db7f4610adff0fe7452af46a1df778f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:13:55 2021 +0200

    client: Raise GraphArgumentException on 4xx response, instead of generic RemoteException.
    
    For consistency with other RPC clients, and because RemoteException is too
    generic to be useful (it also covers internal errors of the server, which
    can be temporary errors, unlike GraphArgumentException which is for invalid
    arguments)

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

vlorentz retitled this revision from Add a simple alternative backend in pure-python to Add a simple alternative "client" in pure-python.May 4 2021, 1:27 PM
vlorentz edited the test plan for this revision. (Show Details)

rename 'standalone' to 'naive'

Build is green

Patch application report for D5665 (id=20249)

Could not rebase; Attempt merge onto 1a8b00f4cd...

Updating 1a8b00f..65852f8
Fast-forward
 swh/graph/client.py                |  13 ++
 swh/graph/naive_client.py          | 313 +++++++++++++++++++++++++++++++++++++
 swh/graph/tests/conftest.py        |  34 ++--
 swh/graph/tests/test_api_client.py |  21 ++-
 4 files changed, 363 insertions(+), 18 deletions(-)
 create mode 100644 swh/graph/naive_client.py
Changes applied before test
commit 65852f82e731bad218a6ea07f498c1b19c6a40e8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:17:14 2021 +0200

    Rewrite SimpleBackend as a NaiveClient
    
    It is simpler this way (no need to hack around the internals of the Java proxy),
    and less likely to need refactoring in the future.

commit eab9fdaeb179f00fbe53b8c8926bc5148d6473c1
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative backend in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit 2364c7cd2db7f4610adff0fe7452af46a1df778f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:13:55 2021 +0200

    client: Raise GraphArgumentException on 4xx response, instead of generic RemoteException.
    
    For consistency with other RPC clients, and because RemoteException is too
    generic to be useful (it also covers internal errors of the server, which
    can be temporary errors, unlike GraphArgumentException which is for invalid
    arguments)

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

Build is green

Patch application report for D5665 (id=20251)

Could not rebase; Attempt merge onto 1a8b00f4cd...

Updating 1a8b00f..09aeab3
Fast-forward
 swh/graph/client.py                |  13 ++
 swh/graph/naive_client.py          | 313 +++++++++++++++++++++++++++++++++++++
 swh/graph/tests/conftest.py        |  34 ++--
 swh/graph/tests/test_api_client.py |  21 ++-
 4 files changed, 363 insertions(+), 18 deletions(-)
 create mode 100644 swh/graph/naive_client.py
Changes applied before test
commit 09aeab3920b2e347d016ad0f43b0553aa2d0fb65
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:17:14 2021 +0200

    Rewrite SimpleBackend as a NaiveClient
    
    It is simpler this way (no need to hack around the internals of the Java proxy),
    and less likely to need refactoring in the future.

commit 07a0829d1222439fea695a3db3046e543a66a31f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative backend in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit 642e8a5468675adcafc1c548abab3650e26289e2
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:13:55 2021 +0200

    client: Raise GraphArgumentException on 4xx response, instead of generic RemoteException.
    
    For consistency with other RPC clients, and because RemoteException is too
    generic to be useful (it also covers internal errors of the server, which
    can be temporary errors, unlike GraphArgumentException which is for invalid
    arguments)

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

seirl added inline comments.
swh/graph/tests/conftest.py
44

This is a bit subjective, but i would probably prefer a fixture for graph_remote_client, a fixture for graph_naive_client, and finally a fixture graph_client that does the dispatch.

This revision is now accepted and ready to land.May 4 2021, 3:10 PM
swh/graph/tests/conftest.py
44

I don't think I can do that without graph_client always starting a WebGraph process, even if we use a naive client

rebase, with added return_types argument support

Build is green

Patch application report for D5665 (id=20278)

Rebasing onto 62c2fd3726...

Current branch diff-target is up to date.
Changes applied before test
commit 48e277a8483a206f9c1933c1d771417d9335d741
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative "client" in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit d0ccf1e5a80625d67bbfce0eda86f6fbdb9d78fd
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:13:55 2021 +0200

    client: Raise GraphArgumentException on 4xx response, instead of generic RemoteException.
    
    For consistency with other RPC clients, and because RemoteException is too
    generic to be useful (it also covers internal errors of the server, which
    can be temporary errors, unlike GraphArgumentException which is for invalid
    arguments)

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

Build is green

Patch application report for D5665 (id=20279)

Could not rebase; Attempt merge onto 62c2fd3726...

Updating 62c2fd3..3e0da94
Fast-forward
 swh/graph/client.py                |  13 ++
 swh/graph/naive_client.py          | 369 +++++++++++++++++++++++++++++++++++++
 swh/graph/tests/conftest.py        |  34 +++-
 swh/graph/tests/test_api_client.py |  21 ++-
 4 files changed, 419 insertions(+), 18 deletions(-)
 create mode 100644 swh/graph/naive_client.py
Changes applied before test
commit 3e0da947777929fc4ca1ccfaa225554fb49558b9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 3 18:17:09 2021 +0200

    Add a simple alternative "client" in pure-python
    
    It allows other components that depend on swh-graph to run their tests
    without depending on the WebGraph process itself.

commit d0ccf1e5a80625d67bbfce0eda86f6fbdb9d78fd
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue May 4 12:13:55 2021 +0200

    client: Raise GraphArgumentException on 4xx response, instead of generic RemoteException.
    
    For consistency with other RPC clients, and because RemoteException is too
    generic to be useful (it also covers internal errors of the server, which
    can be temporary errors, unlike GraphArgumentException which is for invalid
    arguments)

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