-
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
Add reports summary to the output #818
Conversation
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. |
5f83d5e
to
d01c9cc
Compare
/rerun |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5693905 |
Testing Farm request for RHEL-7.9-rhui/5690410;5693905 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5690410;5693905 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5690410;5693905 regression testing has been created. |
/rerun |
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.
Codewise looks ok, let's see how regression tests do with output format change.
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5732462 |
Testing Farm request for RHEL-7.9-rhui/5715919;5732462 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5715919;5732462 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5715919;5732462 regression testing has been created. |
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.
the changed API requires bump of the leapp-framework capability, however, I am not fully sure the change in the API is needed - so do not change it yet. I will do the proper review in upcoming days.
Here is the doc reference: https://leapp.readthedocs.io/en/latest/compatibility-with-leapp-repository.html
Also commit msgs does not reflect the change in API.
/packit build |
@matejmatuska btw, regarding the API, I see it's needed here. So the leapp-framework capability should be bumped from 3.1 to 4.0 as it's incompatible with the previous one right now. requirements in leapp-repository needs to be updated then also. |
Previously only the paths to reports were printed and as a result the reports were easily missed. Leapp now prints a list of HIGH and MEDIUM priority reports along with a summary of number of reports with individual severities to make reports more visible. Also if there are any HIGH or MEDIUM severity reports, the block titles are yellow. The framework-version is bumped to 4.0, because the `report_info` funtion now takes a context_id parameter (to be able to fetch the reports), so the new API is incompatible.
a9832ce
to
0d530a3
Compare
@pirat89 I squashed the commits and added information about the changes in API and also bumped the leapp-framework to 4.0. If we are going to keep the white color I believe this might be ready now. |
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.
lgtm!
/rerun |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5922519 |
Testing Farm request for RHEL-7.9-rhui/5927792;5927772 regression testing has been created. |
Testing Farm request for RHEL-8.8.0-Nightly/5927792;5927772 regression testing has been created. |
Testing Farm request for RHEL-8.6.0-Nightly/5927792;5927772 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5927792;5927772 regression testing has been created. |
Testing Farm request for RHEL-7.9-ZStream/5927792;5927772 regression testing has been created. |
Codewise lgtm, regression tests that rely on checking leapp report output require minor tuning due to obvious reasons (like 'Inhibitor: some message' being changed to 'some message' under inhibitors section) so I'd wait for QE's good to go. @mm0ran @mmacura311 |
/rerun 1061 |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5972604 |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5972609 |
Testing Farm request for RHEL-8.6-rhui/5972609;5972604 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/5972609;5972604 regression testing has been created. |
Testing Farm request for RHEL-8.6.0-Nightly/5972609;5972604 regression testing has been created. |
Testing Farm request for RHEL-8.8.0-Nightly/5972609;5972604 regression testing has been created. |
/packit copr-build |
/rerun 1061 |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6107176 |
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6107185 |
Testing Farm request for RHEL-8.6-rhui/6107185;6107176 regression testing has been created. |
Testing Farm request for RHEL-7.9-rhui/6107185;6107176 regression testing has been created. |
/packit copr-build |
The new Leapp output APIs now display better summary about the report. See oamg/leapp#818 for more info. * Require leapp-framework versio 4.0 * Suppress redundant-keyword-arg for pylint pstodulk: we have one error or another and this one is not actually so important from my POV - I would even argue that it's not a bad habit
The new Leapp output APIs now display better summary about the report. See oamg/leapp#818 for more info. * Require leapp-framework versio 4.0 * Suppress redundant-keyword-arg for pylint pstodulk: we have one error or another and this one is not actually so important from my POV - I would even argue that it's not a bad habit
## 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
Previously only the paths to reports were printed and as a result the reports were easily missed.
Leapp now prints a list of HIGH and MEDIUM priority reports along with a summary of number of reports with individual severities to make reports more visible.
Example of the new output:
breaking changes
This change is incompatible with previous versions of leapp so the leapp-framework is going to be bumped to v 4.0. The leapp-repository needs to be updated also to be able to use the new leapp as it requires an update of CLI commands.
Jira ref.: OAMG-8586
Requires: oamg/leapp-repository#1061