- Notifications
You must be signed in to change notification settings - Fork750
Fix exception pickling#286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The "args" slot of BaseException was not filled, instead we provided theargs through our __getattr__ implementation. This fails in BaseException_reducewhich depends on "args" being not NULL.We fix this by explicitly setting the "args" slot on all CLR exception objectson creation. This also makes tp_repr obsolete.
2537408
to54868ec
Compare@vmuriart @denfromufa@tonyroberts Please review. |
@filmor can you add tests compatible with both python 2 and 3 that use cPickle:
|
@denfromufa Done. Please check :) |
So is this a "feature" or "bug fix"? If latter, then let's merge this for Have you ran the embedding tests to make sure that exceptions do not break On Wed, Nov 9, 2016 at 4:22 AM, Benedikt Reinartznotifications@github.com
|
I haven't run the embedded tests, I'll check how to do that. This is definitely a bug fix, and a pretty critical one from my point of view (as said, this was killing my worker processes in production). |
Ok, please let me know if you have any luck running embedding tests and BTW, why do you pickle exceptions - really seems a wild idea to me? On Wed, Nov 9, 2016 at 9:20 AM, Benedikt Reinartznotifications@github.com
|
The tests work against Python 3.5 (though they have various issues, I'll prepare a PR). I pass the exceptions back from a multiprocessing worker to show them to the user, and during that they are automatically pickled. |
Uh oh!
There was an error while loading.Please reload this page.
Fixes#284.
This PR contains 3 componenents:
BaseException
object instead of faking the functionality by overloading__getattr__
__getattr__
overload altogether since the only functionality it provides (lookup of.message
) is deprecated in Python >= 2.6