patch faulthandler defaults to underlying stderr #92
Conversation
|
Wouldn't that fail on Python 2 that have no fault handler module ? |
|
Yup, forgot about Python 2. |
|
+1. |
|
The patch as it stands right now looks good, I just wonder if it wouldn't be a good idea to add a small test that makes the actual Though given Python's lack of formal interface description and testing, we can't really catch forward changes, so perhaps there isn't that much value to it. They are unlikely to introduce backwards-incompatible changes, and short of adding a signature-inspection warning using Thoughts? |
|
I thought about making it |
|
Yes, that was exactly my concern, which is why I'm wondering if adding a test would be kind of a pointless exercise in putting in tests for their own sake. Maybe we should just leave it as-is, and instead just add an explicit note as a comment in the code, just a simple: # Warning: this is a monkeypatch of `faulthandler.enable`, watch for possible
# updates to the upstream API and update accordingly:
# https://docs.python.org/3/library/faulthandler.html#faulthandler.enableBetter than putting in code that isn't really adding value. |
fault handler requires access to C-level FD, which doesn't make sense for forwarded stdout/stderr. Patch default to be underlying `__stderr__` instead of overridden `stderr`.
|
Added your comment, a smoke test, and passthrough of any added kwargs, which should cover it for a long time, at least. |
|
OK, this is as much as we can reasonably do on this. Thanks! Merging. |
patch faulthandler defaults to underlying stderr. This redirects faulthandler to a real file descriptor instead of a zmq socket, so that in the event of a hard C-level crash, traceback data can still get logged. Closes #91.
3 participants
faulthandler requires access to C-level FD, which doesn't make sense for forwarded stdout/stderr. Patch default to be underlying
__stderr__instead of overriddenstderr.closes #91