Page MenuHomeSoftware Heritage
Differential D7272

Use flask instead of aiohttp as RPC server.
ClosedPublic
Actions

Authored by vlorentz on Mar 1 2022, 4:32 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Maniphest Tasks
Restricted Maniphest Task
Commits
rDOBJS32cfda0406aa: Use flask instead of aiohttp as RPC server.
Summary

The azure backend cannot be run in the same thread as an asyncio
event loop.
Resolves T3981

Additionally, this simplifies the code, by reusing the same
method auto-generation as other SWH components.

Depends on D7266, D7267, D7268, D7271

Test Plan

will fail because it depends on two diffs in swh-core

Diff Detail

Repository
rDOBJS Object storage
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Mar 1 2022, 4:32 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 1 2022, 4:32 PM
vlorentz published this revision for review.Mar 1 2022, 4:33 PM
vlorentz added a parent revision: D7269: server: Group multiple object ids in the same HTTP chunk.
swh-public-ci added a comment.Mar 1 2022, 4:35 PM

Build has FAILED

Patch application report for D7272 (id=26332)

Could not rebase; Attempt merge onto 76a5f36b41...

Updating 76a5f36..d29a584
Fast-forward
 requirements.txt                            |   1 -
 swh/objstorage/api/client.py                |  70 ++-------
 swh/objstorage/api/server.py                | 215 +++++++++-----------------
 swh/objstorage/backends/pathslicing.py      |  16 ++
 swh/objstorage/backends/seaweedfs/http.py   |   4 +-
 swh/objstorage/cli.py                       |  20 ++-
 swh/objstorage/interface.py                 | 232 ++++++++++++++++++++++++++++
 swh/objstorage/objstorage.py                | 209 -------------------------
 swh/objstorage/tests/test_objstorage_api.py |  10 +-
 9 files changed, 353 insertions(+), 424 deletions(-)
 create mode 100644 swh/objstorage/interface.py
Changes applied before test
commit d29a584369b2802037da65a91c140801b3700e47
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:49:53 2022 +0100

    Use flask instead of aiohttp as RPC server.
    
    The azure backend cannot be run in the same thread as an asyncio
    event loop: https://forge.softwareheritage.org/T3981
    
    Additionally, this simplifies the code, by reusing the same
    method auto-generation as other SWH components.

commit 43e7d45961ef5241f4157d864cf4218dba86b55e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:17:42 2022 +0100

    server: Group multiple object ids in the same HTTP chunk
    
    This reduces the size of the response from 26 bytes per id
    to an average of 20.06 bytes (rounded up to the closest
    multiple of 26).

commit ff319b60b2221333074d16659f8222a0d19bfe66
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:13:43 2022 +0100

    client: Do not depend on how the server chunks the response
    
    Currently, the server uses HTTP chunked encoding to send object ids
    one by one, each in its own HTTP frame.
    
    I think this is a mistake to rely on such a detail of the HTTP protocol
    in a high-level API like this.
    
    Additionally, a future commit will rewrite the server to use Flask
    instead of aiohttp, which does not allow this kind of fine-grained
    control about HTTP chunks.

commit b159f04b8d591947003cf3a4cd15fbcf0f3cb31e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 13:36:02 2022 +0100

    Remove method add_stream from the RPC API.
    
    Rationale:
    
    1. Only the pathslicing backend implements it
    2. No other package uses it (besides a dead code path in the vault)
    3. A future commit will rewrite the RPC server to use Flask instead of
       aiohttp, and rewriting this view correctly is going to be hard
       (though possible)

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

Harbormaster failed remote builds in B27204: Diff 26332!Mar 1 2022, 4:35 PM
swh-public-ci added a comment.Mar 1 2022, 5:07 PM

Build has FAILED

Patch application report for D7272 (id=26332)

Could not rebase; Attempt merge onto 76a5f36b41...

Updating 76a5f36..d29a584
Fast-forward
 requirements.txt                            |   1 -
 swh/objstorage/api/client.py                |  70 ++-------
 swh/objstorage/api/server.py                | 215 +++++++++-----------------
 swh/objstorage/backends/pathslicing.py      |  16 ++
 swh/objstorage/backends/seaweedfs/http.py   |   4 +-
 swh/objstorage/cli.py                       |  20 ++-
 swh/objstorage/interface.py                 | 232 ++++++++++++++++++++++++++++
 swh/objstorage/objstorage.py                | 209 -------------------------
 swh/objstorage/tests/test_objstorage_api.py |  10 +-
 9 files changed, 353 insertions(+), 424 deletions(-)
 create mode 100644 swh/objstorage/interface.py
Changes applied before test
commit d29a584369b2802037da65a91c140801b3700e47
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:49:53 2022 +0100

    Use flask instead of aiohttp as RPC server.
    
    The azure backend cannot be run in the same thread as an asyncio
    event loop: https://forge.softwareheritage.org/T3981
    
    Additionally, this simplifies the code, by reusing the same
    method auto-generation as other SWH components.

commit 43e7d45961ef5241f4157d864cf4218dba86b55e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:17:42 2022 +0100

    server: Group multiple object ids in the same HTTP chunk
    
    This reduces the size of the response from 26 bytes per id
    to an average of 20.06 bytes (rounded up to the closest
    multiple of 26).

commit ff319b60b2221333074d16659f8222a0d19bfe66
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:13:43 2022 +0100

    client: Do not depend on how the server chunks the response
    
    Currently, the server uses HTTP chunked encoding to send object ids
    one by one, each in its own HTTP frame.
    
    I think this is a mistake to rely on such a detail of the HTTP protocol
    in a high-level API like this.
    
    Additionally, a future commit will rewrite the server to use Flask
    instead of aiohttp, which does not allow this kind of fine-grained
    control about HTTP chunks.

commit b159f04b8d591947003cf3a4cd15fbcf0f3cb31e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 13:36:02 2022 +0100

    Remove method add_stream from the RPC API.
    
    Rationale:
    
    1. Only the pathslicing backend implements it
    2. No other package uses it (besides a dead code path in the vault)
    3. A future commit will rewrite the RPC server to use Flask instead of
       aiohttp, and rewriting this view correctly is going to be hard
       (though possible)

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

vlorentz updated this revision to Diff 26336.Mar 1 2022, 5:10 PM

fix tests

swh-public-ci added a comment.Mar 1 2022, 5:12 PM

Build is green

Patch application report for D7272 (id=26336)

Could not rebase; Attempt merge onto 76a5f36b41...

Updating 76a5f36..f578d1e
Fast-forward
 requirements.txt                            |   1 -
 swh/objstorage/api/client.py                |  70 ++-------
 swh/objstorage/api/server.py                | 214 +++++++++----------------
 swh/objstorage/backends/pathslicing.py      |  16 ++
 swh/objstorage/backends/seaweedfs/http.py   |   4 +-
 swh/objstorage/cli.py                       |  20 ++-
 swh/objstorage/interface.py                 | 232 ++++++++++++++++++++++++++++
 swh/objstorage/objstorage.py                | 206 ------------------------
 swh/objstorage/tests/test_objstorage_api.py |  10 +-
 9 files changed, 351 insertions(+), 422 deletions(-)
 create mode 100644 swh/objstorage/interface.py
Changes applied before test
commit f578d1ef703064e20fdafc70d4be7469eb554790
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:49:53 2022 +0100

    Use flask instead of aiohttp as RPC server.
    
    The azure backend cannot be run in the same thread as an asyncio
    event loop: https://forge.softwareheritage.org/T3981
    
    Additionally, this simplifies the code, by reusing the same
    method auto-generation as other SWH components.

commit 43e7d45961ef5241f4157d864cf4218dba86b55e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:17:42 2022 +0100

    server: Group multiple object ids in the same HTTP chunk
    
    This reduces the size of the response from 26 bytes per id
    to an average of 20.06 bytes (rounded up to the closest
    multiple of 26).

commit ff319b60b2221333074d16659f8222a0d19bfe66
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:13:43 2022 +0100

    client: Do not depend on how the server chunks the response
    
    Currently, the server uses HTTP chunked encoding to send object ids
    one by one, each in its own HTTP frame.
    
    I think this is a mistake to rely on such a detail of the HTTP protocol
    in a high-level API like this.
    
    Additionally, a future commit will rewrite the server to use Flask
    instead of aiohttp, which does not allow this kind of fine-grained
    control about HTTP chunks.

commit b159f04b8d591947003cf3a4cd15fbcf0f3cb31e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 13:36:02 2022 +0100

    Remove method add_stream from the RPC API.
    
    Rationale:
    
    1. Only the pathslicing backend implements it
    2. No other package uses it (besides a dead code path in the vault)
    3. A future commit will rewrite the RPC server to use Flask instead of
       aiohttp, and rewriting this view correctly is going to be hard
       (though possible)

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

Harbormaster completed remote builds in B27210: Diff 26336.Mar 1 2022, 5:12 PM
vlorentz added a child revision: D7273: Add test to enforce all objstorage backends follow the interface.Mar 1 2022, 5:58 PM
vlorentz updated this revision to Diff 26349.Mar 2 2022, 11:50 AM

rebase

swh-public-ci added a comment.Mar 2 2022, 11:53 AM

Build is green

Patch application report for D7272 (id=26349)

Could not rebase; Attempt merge onto 76a5f36b41...

Updating 76a5f36..32cfda0
Fast-forward
 requirements.txt                            |   1 -
 swh/objstorage/api/client.py                |  70 ++-------
 swh/objstorage/api/server.py                | 214 +++++++++----------------
 swh/objstorage/backends/pathslicing.py      |  16 ++
 swh/objstorage/backends/seaweedfs/http.py   |   4 +-
 swh/objstorage/cli.py                       |  20 ++-
 swh/objstorage/interface.py                 | 232 ++++++++++++++++++++++++++++
 swh/objstorage/objstorage.py                | 206 ------------------------
 swh/objstorage/tests/test_objstorage_api.py |  10 +-
 9 files changed, 351 insertions(+), 422 deletions(-)
 create mode 100644 swh/objstorage/interface.py
Changes applied before test
commit 32cfda0406aa2a8b9f4727abf2ea56b7563460ea
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:49:53 2022 +0100

    Use flask instead of aiohttp as RPC server.
    
    The azure backend cannot be run in the same thread as an asyncio
    event loop: https://forge.softwareheritage.org/T3981
    
    Additionally, this simplifies the code, by reusing the same
    method auto-generation as other SWH components.

commit 9be723dcbc9813a4a66e0068de22e36dbaee09c2
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:17:42 2022 +0100

    server: Group multiple object ids in the same HTTP chunk
    
    This reduces the size of the response from 26 bytes per id
    to an average of 20.06 bytes (rounded up to the closest
    multiple of 26).

commit a2424b7a13e9a696329a9e9efa6c86f9864dd60b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 15:13:43 2022 +0100

    client: Do not depend on how the server chunks the response
    
    Currently, the server uses HTTP chunked encoding to send object ids
    one by one, each in its own HTTP frame.
    
    I think this is a mistake to rely on such a detail of the HTTP protocol
    in a high-level API like this.
    
    Additionally, a future commit will rewrite the server to use Flask
    instead of aiohttp, which does not allow this kind of fine-grained
    control about HTTP chunks.

commit b159f04b8d591947003cf3a4cd15fbcf0f3cb31e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 1 13:36:02 2022 +0100

    Remove method add_stream from the RPC API.
    
    Rationale:
    
    1. Only the pathslicing backend implements it
    2. No other package uses it (besides a dead code path in the vault)
    3. A future commit will rewrite the RPC server to use Flask instead of
       aiohttp, and rewriting this view correctly is going to be hard
       (though possible)

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

Harbormaster completed remote builds in B27224: Diff 26349.Mar 2 2022, 11:53 AM
olasd added a subscriber: olasd.Mar 2 2022, 5:05 PM

I think this all looks good, thank you! Obviously, the consistency with other RPCs is much appreciated.

olasd accepted this revision.Mar 2 2022, 5:06 PM
This revision is now accepted and ready to land.Mar 2 2022, 5:06 PM
olasd added inline comments.Mar 2 2022, 5:08 PM
swh/objstorage/tests/test_objstorage_api.py
41–48

I'm a bit doubtful of these overrides considering that delete is in fact implemented in the remote APIs?

vlorentz added inline comments.Mar 2 2022, 5:24 PM
swh/objstorage/tests/test_objstorage_api.py
41–48

they both error because they expect PermissionError to be raised, but it's not.

🤷

Closed by commit rDOBJS32cfda0406aa: Use flask instead of aiohttp as RPC server. (authored by vlorentz). · Explain WhyMar 2 2022, 5:25 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDOBJS32cfda0406aa: Use flask instead of aiohttp as RPC server..
olasd added inline comments.Mar 2 2022, 5:52 PM
swh/objstorage/tests/test_objstorage_api.py
41–48

Maybe it could be added to reraise_exceptions?

vlorentz added inline comments.Mar 2 2022, 6:07 PM
swh/objstorage/tests/test_objstorage_api.py
41–48

Nope, reraise_exceptions is only used when it would raise RemoteException, but here, no exception is raised at all.

Revision Contents
Changeset List

PathSize
1 line
swh/
objstorage/
api/
56 lines
185 lines
20 lines
interface.py
objstorage.py
196 lines
206 lines
tests/
10 lines

Diff 26361

View Options

requirements.txt

Loading...
View Options

swh/objstorage/api/client.py

Loading...
View Options

swh/objstorage/api/server.py

Loading...
View Options

swh/objstorage/cli.py

Loading...
View Options

swh/objstorage/interface.py

Loading...
View Options

swh/objstorage/objstorage.py

Loading...
View Options

swh/objstorage/tests/test_objstorage_api.py

Loading...
About · Developer information · Legal