Page MenuHomeSoftware Heritage

api/RPCClient: Do not raise 404 remote exception if it must be reraised
AbandonedPublic

Authored by anlambert on Aug 9 2022, 6:59 PM.

Details

Reviewers
None
Group Reviewers
Reviewers
Summary

The RPCClient class has a reraise_exceptions list containing exception types
that should be reraised client side instead of wrapping them in a RemoteException
instance.

If the remote exception comes with a 404 HTTP status code, it was automatically
raised as a RemoteException regardless if the exception type must be reraised
client side.

So ensure to not raise a RemoteException if the 404 remote exception should
be reraised.

That change is required to fix HTTP status code for missing objects when querying
the swh-objstorage RPC endpoints (aka 404, see D8228).

Diff Detail

Repository
rDCORE Foundations and core functionalities
Branch
api-fix-404-exception-handling
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 30734
Build 48053: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 48052: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D8227 (id=29675)

Rebasing onto 2d1cd6af71...

Current branch diff-target is up to date.
Changes applied before test
commit 163be8b966e63f37204071cdc7e711fa27bcb5de
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Aug 9 18:42:40 2022 +0200

    api/RPCClient: Do not raise 404 remote exception if it must be reraised
    
    The RPCClient class has a reraise_exceptions list containing exception types
    that should be reraised client side instead of wrapping them in a RemoteException
    instance.
    
    If the remote exception comes with a 404 HTTP status code, it was automatically
    raised as a RemoteException regardless if the exception type must be reraised
    client side.
    
    So ensure to not raise a RemoteException if the 404 remote exception should
    be reraised.

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

I'm not sure we actually need this; I started a discussion on D8228 as it is more appropriate there.

swh/core/api/__init__.py
356–357

I think that's equivalent here, given how exception is instantiated

Abandoning this as it is not needed after all.

swh/core/api/__init__.py
356–357

No we still need to check if the exception should be reraised or a RemoteException will be raised otherwise.