-
Notifications
You must be signed in to change notification settings - Fork 70
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
Save actor tracebacks in leapp.db #832
Conversation
Now when an exception is thrown during actor execution it won't be shown only on stdout, but will appear in leapp.db and leapp report as high risk report message. OAMG-4186
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 mergeable.
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. |
/rerun |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6020934 |
Testing Farm request for RHEL-8.6-rhui/5990856;6020934 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/5990856;6020934 regression testing has been created. |
/rerun |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6091405 |
Testing Farm request for RHEL-8.6-rhui/6080984;6091405 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/6080984;6091405 regression testing has been created. |
LGTM, although I agree with the suggestion from @pirat89 in previous comment - changing the immediate CLI output to something similar to the Tried on rhel7 vm by raising an exception in Leapp report:
Database:
I'd suggest to add an ERROR log as well, if that makes sense and would be doable. |
@PeterMocary @pirat89 Thanks for the review! |
@fernflower In case of the report I am more wondering WDYT about putting it into the report like this:
I mean to keep the title as shorter msg and use details/summary/... for the rest. But as I told I am ok with this solution also. Btw, for the console output it's same as before:
I actually realized the "banner" statement just now :-D It's cool. |
@fernflower @PeterMocary btw, I've realized that for error models in the report.txt file, we do not print that it's error, inhibitor, ... it looks just a high severity msg. I am thinking we should improve it in future somehow to make it clear that it's error, but I do not want to change the structure/schema because of that. |
Just realized you summarized that right! Sorry, my brain is slow today :/ |
/rerun |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6107459 |
Testing Farm request for RHEL-8.6-rhui/6096363;6107459 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/6096363;6107459 regression testing has been created. |
@PeterMocary @pirat89 All your comments should be resolved - moved trace to the message details (so that a nice Summary with shorter message name will be shown in the report), error is logged at the time of the crash as well. Could you please re-review?
(except from leapp-upgrade.log)
|
@fernflower thanks! lgtm and tests passed. Merging. |
## Packaging - Provide leapp-framework 5.0 (oamg#818, oamg#840) ## Framework ### Fixes - Improve processing of reports with UTF-8 characters (oamg#821, oamg#828) - Fix info reporting with only one path to log (oamg#834) ### Enhancements - Include tracebacks of actors into the leapp.db (oamg#832) ## Leapp (tool) ### Fixes - Dialog creation fails not more for a component without choices (oamg#826) - Empty report is generated correctly (oamg#828) - Fix processing data in remediation instructions with non-ascii characters () ### Enhancements - Improve report summary output to make it more visible (oamg#818, oamg#840) ## stdlib ### Fixes - Fixed the call when the execute cannot be performed (oamg#836) ### Enhancements - changes related just to stdlib - under leapp/libraries/stdlib
## Packaging - Provide leapp-framework 5.0 (oamg#818, oamg#840) ## Framework ### Fixes - Improve processing of reports with UTF-8 characters (oamg#821, oamg#828) - Fix info reporting with only one path to log (oamg#834) ### Enhancements - Include tracebacks of actors into the leapp.db (oamg#832) ## Leapp (tool) ### Fixes - Dialog creation fails not more for a component without choices (oamg#826) - Empty report is generated correctly (oamg#828) - Fix processing data in remediation instructions with non-ascii characters () ### Enhancements - Improve report summary output to make it more visible (oamg#818, oamg#840) ## stdlib ### Fixes - Fixed the call when the execute cannot be performed (oamg#836) ### Enhancements - changes related just to stdlib - under leapp/libraries/stdlib
## Packaging - Provide leapp-framework 5.0 (#818, #840) ## Framework ### Fixes - Improve processing of reports with UTF-8 characters (#821, #828) - Fix info reporting with only one path to log (#834) ### Enhancements - Include tracebacks of actors into the leapp.db (#832) ## Leapp (tool) ### Fixes - Dialog creation fails not more for a component without choices (#826) - Empty report is generated correctly (#828) - Fix processing data in remediation instructions with non-ascii characters () ### Enhancements - Improve report summary output to make it more visible (#818, #840) ## stdlib ### Fixes - Fixed the call when the execute cannot be performed (#836) ### Enhancements - changes related just to stdlib - under leapp/libraries/stdlib
Now when an exception is thrown during actor
execution it won't be shown only on stdout, but
will appear in leapp.db and leapp report as high
risk report message.
OAMG-4186