-
Notifications
You must be signed in to change notification settings - Fork 71
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose tracebacks from actor exceptions #802
Conversation
Previously when leapp would be running actors, any exceptions raised by those actors in child processes would only be output to stderr, while the main process would only have the exit code to examine. Consequently, there was no real way to log occuring exceptions - only providing the aforementioned exit code was possible. This patch adds a pipe between the main and the actor processes, through which a formatted exception + traceback string is provided to the main process. This string is then packaged into the LeappRuntimeError thrown from the process, to be utilized downstream in workflow/command code.
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
To launch regression testing public members of oamg organization can leave the following comment:
Please open ticket in case you experience technical problem with the CI. (RH internal only) Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra. |
This PR has been linked in issue tracker (OAMG-7853). |
@prilr also please take a look at the failed test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the LeappRuntimeError
constructor
Other than the constructor the PR LGTM, once it passes the tests I will give it a try myself. |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5067065 |
Testing Farm request for RHEL-8.6-rhui/5057264;5067065 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/5057264;5067065 regression testing has been created. |
Testing Farm request for RHEL-8.7.0-Nightly/5057264;5067065 regression testing has been created. |
Testing Farm request for RHEL-8.6.0-Nightly/5057264;5067065 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5057264;5067065 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5057264;5067065 regression testing has been created. |
Should've checked with Python 2 before submitting, sorry about that. |
/rerun |
Wow, that's a great addition that has somehow slipped under the radar. I will try to take a look at it this week. |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5243870 |
Testing Farm request for RHEL-8.6-rhui/5201672;5243870 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/5201672;5243870 regression testing has been created. |
Testing Farm request for RHEL-8.6.0-Nightly/5201672;5243870 regression testing has been created. |
Testing Farm request for RHEL-8.7.0-Nightly/5201672;5243870 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5201672;5243870 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5201672;5243870 regression testing has been created. |
Integration tests (apart from the flaky initram one I will hopefully solve soon) are fine. |
## Packaging - Change permissions for /var/lib/leapp to 0700 (oamg#807) ## Framework ### Fixes - Prevent unicode errors when printing error messages (oamg#806) - Make checks for missing answers more strict (oamg#797) ### Enhancements - Expose tracebacks from actors (oamg#802) ### Known issue - introduced new known issue in framework...
Previously when leapp would be running actors, any exceptions raised by
those actors in child processes would only be output to stderr,
while the main process would only have the exit code to examine.
Consequently, there was no real way to log occurring exceptions -
only providing the aforementioned exit code was possible.
This patch adds a pipe between the main and the actor processes,
through which a formatted exception + traceback string is provided to
the main process.
This string is then packaged into the
LeappRuntimeError
thrown from theprocess, to be utilized downstream in
workflow/command code.
This should serve as a step to closing #692 - although actually logging
the exception traceback likely should be done on the
leapp-repository
side.