tomerfiliba / plumbum

Plumbum: Shell Combinators
https://plumbum.readthedocs.io
MIT License
2.79k stars 182 forks source link

exceptions: ensure ProcessExecutionError can be pickled #586

Closed koreno closed 2 years ago

koreno commented 2 years ago

Something in recent refactors broke this capability, which we are using in our infrastructure, so I've fixed and cemented it with a UT.

henryiii commented 2 years ago

EnvironmentError isn't a real error anymore in Python 3, it's just an OSError. OSErrors return subclasses based on what the OS return code in the first argument is. I think we need to refactor this a bit. Specifically, the first value is the return code, and for us it's the second, and OSError's new is where the subclass selection is happening - this sounds very incorrect to me! For now, this is fine to fix the immediate problem.

henryiii commented 2 years ago

Maybe it is implemented in __init__, since this seems to be stable (after the patch), or maybe it checks the cls before doing the subclass (though EnvironementError(3, "msg") returns ProcessLookupError(3, 'msg'), etc). The exceptions are mostly implemented in C, so hard to quickly check.

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 83.608% when pulling a7c656e62782d22af926c552d815e6b582146246 on fix-exception-pickling into 7bba25587967a5aa0c9f32933bc2a9b59f83bb24 on master.

henryiii commented 2 years ago

Thanks!!!