Skip to content
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

Merged
merged 1 commit into from Jul 17, 2023
Merged

Conversation

matejmatuska
Copy link
Member

@matejmatuska matejmatuska commented Mar 17, 2023

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:

Screenshot from 2023-03-24 12-31-33

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

@github-actions
Copy link

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.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please to notify leapp developers of review request
  • /packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and leapp-repository*master* as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp-repository*PR42* as artifacts
  • /rerun-sst to schedule sst tests using this pr build and leapp-repository*master* as artifacts
  • /rerun-sst 42 to schedule sst tests using this pr build and leapp-repository*PR42* as artifacts

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.

@matejmatuska matejmatuska force-pushed the improve-output branch 3 times, most recently from 5f83d5e to d01c9cc Compare March 17, 2023 10:27
@fernflower
Copy link
Member

/rerun

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5693905

leapp/utils/output.py Outdated Show resolved Hide resolved
@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/5690410;5693905 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5690410;5693905 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5690410;5693905 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@fernflower
Copy link
Member

/rerun

Copy link
Member

@fernflower fernflower left a 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.

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5732462

@matejmatuska matejmatuska marked this pull request as draft March 31, 2023 10:25
@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/5715919;5732462 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5715919;5732462 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5715919;5732462 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

pirat89
pirat89 previously requested changes Apr 5, 2023
Copy link
Member

@pirat89 pirat89 left a 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.

@pirat89
Copy link
Member

pirat89 commented Apr 12, 2023

/packit build

@pirat89
Copy link
Member

pirat89 commented Apr 12, 2023

Seeing also white text in case of errors or inhibitors:

screenshot_report_error

I am now thinking whether we want to stay with the white color in such case or whether we want to use the yellow/red in such cases. WDYT guys? @oamg/developers

@pirat89
Copy link
Member

pirat89 commented Apr 12, 2023

@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.
@matejmatuska matejmatuska marked this pull request as ready for review May 15, 2023 11:28
@matejmatuska
Copy link
Member Author

@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.

@fernflower fernflower dismissed pirat89’s stale review May 16, 2023 09:53

stale, new changes

fernflower
fernflower previously approved these changes May 16, 2023
Copy link
Member

@fernflower fernflower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fernflower
Copy link
Member

/rerun

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5922519

@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/5927792;5927772 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-8.8.0-Nightly/5927792;5927772 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-8.6.0-Nightly/5927792;5927772 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5927792;5927772 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5927792;5927772 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@fernflower
Copy link
Member

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

@fernflower
Copy link
Member

/rerun 1061

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5972604

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5972609

@github-actions
Copy link

Testing Farm request for RHEL-8.6-rhui/5972609;5972604 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/5972609;5972604 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-8.6.0-Nightly/5972609;5972604 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-8.8.0-Nightly/5972609;5972604 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@dkubek
Copy link
Member

dkubek commented Jun 13, 2023

/packit copr-build

@fernflower
Copy link
Member

/rerun 1061

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6107176

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6107185

@github-actions
Copy link

Testing Farm request for RHEL-8.6-rhui/6107185;6107176 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/6107185;6107176 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@dkubek
Copy link
Member

dkubek commented Jul 11, 2023

/packit copr-build

@pirat89 pirat89 merged commit 154e1c5 into oamg:master Jul 17, 2023
14 checks passed
pirat89 pushed a commit to matejmatuska/leapp-repository that referenced this pull request Jul 17, 2023
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
pirat89 pushed a commit to oamg/leapp-repository that referenced this pull request Jul 17, 2023
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
pirat89 added a commit to pirat89/leapp that referenced this pull request Aug 23, 2023
## 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
@pirat89 pirat89 mentioned this pull request Aug 23, 2023
pirat89 added a commit to pirat89/leapp that referenced this pull request Aug 23, 2023
## 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
Rezney pushed a commit that referenced this pull request Aug 23, 2023
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requries Repo PR Use it when leapp-deps is changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants