Differential D2627

In case of errors, return a simple dictionary instead of pickled exception.
ClosedPublic
Actions

Authored by vlorentz on Feb 5 2020, 5:12 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T2239: storage: kafka issue: Can't pickle <class 'cimpl.KafkaException'>: import of module 'cimpl' failed

Commits

rDCORE08289ce5c33e: In case of errors, return a simple dictionary instead of pickled exception.

Summary

Pros:

Language-independent
Works with unpickleable exceptions (eg. cimpl.KafkaException, and RLock objects in exceptions raised by tenacity)

Con:

Shows a less nice error on the client side (if the client is in Python)

Resolves T2239.

Test Plan

tox passes this repo, but I'm going to try tests with RPC servers that use it.

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

vlorentz created this revision.Feb 5 2020, 5:12 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 5 2020, 5:12 PM

but I'm going to try tests with RPC servers that use it.

Meh. Of course it doesn't.

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/399/ for more details.

Harbormaster completed remote builds in B10453: Diff 9393.Feb 5 2020, 5:14 PM

starts to look, better ;)

swh/core/api/__init__.py
97	is self.args systematically a list?
284	unsure about removing that statement. i guess that's the price for not having tests on that part ;)

ardumont accepted this revision.Feb 6 2020, 12:07 PM

This revision is now accepted and ready to land.Feb 6 2020, 12:07 PM

vlorentz added inline comments.Feb 6 2020, 12:09 PM

swh/core/api/__init__.py
97	It's a tuple, but yes
284	It's a bad idea to return None in case an endpoint doesn't exist, anyway.

Note: this diff will stay open for a little while, @ardumont and I have other changes to make before it can be merged

vlorentz added a child revision: D2630: Re-raise StorageArgumentException through API calls..Feb 6 2020, 2:22 PM

Update RPCClient to re-raise whitelisted exceptions types.

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/405/ for more details.

Harbormaster completed remote builds in B10540: Diff 9477.Feb 11 2020, 3:48 PM

rebase

This revision was landed with ongoing or failed builds.Feb 14 2020, 12:22 PM

Closed by commit rDCORE08289ce5c33e: In case of errors, return a simple dictionary instead of pickled exception. (authored by vlorentz). · Explain Why

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDCORE08289ce5c33e: In case of errors, return a simple dictionary instead of pickled exception..

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/407/ for more details.

Harbormaster completed remote builds in B10588: Diff 9527.Feb 14 2020, 12:22 PM

Revision Contents
Changeset List

Path

Size

swh/

core/

api/

__init__.py

69 lines

tests/

test_rpc_client_server.py

11 lines

Diff 9528

View Options

swh/core/api/init.py

View Options

swh/core/api/tests/test_rpc_client_server.py

In case of errors, return a simple dictionary instead of pickled exception.ClosedPublicActions