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

Fix: report msg not generated at the end of run #573

Merged
merged 1 commit into from
Jan 28, 2020

Conversation

Rezney
Copy link
Member

@Rezney Rezney commented Nov 12, 2019

At the end of "leapp upgrade" run we do not print message
about generated reports, in contrast to "leapp preupgrade"
where we do.

@Rezney
Copy link
Member Author

Rezney commented Nov 12, 2019

@drehak @bocekm please take a look...

@centos-ci
Copy link

Can one of the admins verify this patch?

@drehak
Copy link
Contributor

drehak commented Nov 12, 2019

Tested, LGMT.

drehak
drehak previously approved these changes Nov 12, 2019
@drehak
Copy link
Contributor

drehak commented Nov 12, 2019

@examon @vojtechsokol Are these linter issues false positives or not?

bocekm
bocekm previously requested changes Nov 12, 2019
Copy link
Member

@bocekm bocekm left a comment

Choose a reason for hiding this comment

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

When running leapp upgrade --debug, I now get:

============================================================
                           REPORT                           
============================================================

A report has been generated at /var/log/leapp/leapp-report.json
A report has been generated at /var/log/leapp/leapp-report.txt
A report has been generated at /var/log/leapp/leapp-upgrade.log

============================================================
                       END OF REPORT                        
============================================================

The /var/log/leapp/leapp-upgrade.log shouldn't be there. That's not a report. The debug log should not be of any interest to users by default.
@Rezney would like to have a bigger quorum, so @shaded-enmity, @pirat89, do you agree with removing it?

@Rezney
Copy link
Member Author

Rezney commented Nov 13, 2019

When running leapp upgrade --debug, I now get:

============================================================
                           REPORT                           
============================================================

A report has been generated at /var/log/leapp/leapp-report.json
A report has been generated at /var/log/leapp/leapp-report.txt
A report has been generated at /var/log/leapp/leapp-upgrade.log

============================================================
                       END OF REPORT                        
============================================================

The /var/log/leapp/leapp-upgrade.log shouldn't be there. That's not a report.

If you run "leapp preupgrade --debug", what do you see?

@Rezney
Copy link
Member Author

Rezney commented Jan 8, 2020

@bocekm how do we move forward with this PR?

@pirat89
Copy link
Member

pirat89 commented Jan 8, 2020

The /var/log/leapp/leapp-upgrade.log shouldn't be there. That's not a report. The debug log should not be of any interest to users by default.
@Rezney would like to have a bigger quorum, so @shaded-enmity, @pirat89, do you agree with removing it?

Agree with Michal. log file should not be there.

@shaded-enmity
Copy link
Member

I don't mind the log file if we show it differently:

============================================================
                           REPORT                           
============================================================

A report has been generated at /var/log/leapp/leapp-report.json
A report has been generated at /var/log/leapp/leapp-report.txt

============================================================
                       END OF REPORT                        
============================================================

Debug output written to /var/log/leapp/leapp-upgrade.log

@Rezney Rezney changed the title Fix: report msg not generated at the end of run WIP: Fix: report msg not generated at the end of run Jan 23, 2020
@Rezney Rezney force-pushed the show_reports_fix branch 4 times, most recently from 5329f5b to 5e3b066 Compare January 23, 2020 20:39
@vinzenz vinzenz dismissed bocekm’s stale review January 24, 2020 09:04

Changes requested have been applied

@Rezney Rezney force-pushed the show_reports_fix branch 4 times, most recently from b28c95d to cbfc2c1 Compare January 27, 2020 11:38
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, one nit inline but up to you.
Will do a fullstack run before approval

files = []
for file_ in cfg.get(section, 'files').split(','):
file_path = os.path.join(cfg.get(section, 'dir'), file_)
if not must_exist:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'd put it as one condition

if not must_exist or must_exist and os.path.isfile(file_path):
    files.append(file_path)

Copy link
Member Author

@Rezney Rezney Jan 27, 2020

Choose a reason for hiding this comment

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

makes sense, will apply...

fernflower
fernflower previously approved these changes Jan 27, 2020
@Rezney
Copy link
Member Author

Rezney commented Jan 27, 2020

leapp-ci build

@Rezney Rezney changed the title WIP: Fix: report msg not generated at the end of run Fix: report msg not generated at the end of run Jan 27, 2020
@Rezney Rezney force-pushed the show_reports_fix branch 2 times, most recently from 9309e0a to fd7bf98 Compare January 27, 2020 15:04
@Rezney
Copy link
Member Author

Rezney commented Jan 27, 2020

rebased...

# debug logs that will get reported at the end of a preupgrade/upgrade run if they were created/modified during it
_LOGS = [
'leapp-upgrade.log',
'leapp-preupgrade.log'
Copy link
Contributor

Choose a reason for hiding this comment

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

These two files could be sorted, although that's just a tiny nitpick.

if log_paths:
for log_path in log_paths:
sys.stdout.write("Debug output written to {path}\n".format(path=log_path))
sys.stdout.write("\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move this newline above if log_paths:. Currently there's 0 newlines above Debug output... and 2 newlines below:

[...]
Transaction Summary
================================================================================
Install    325 Packages
Upgrade    441 Packages
Remove      89 Packages
Downgrade    5 Packages

Total size: 631 M
Total download size: 524 M
Downloading Packages:
Check completed.
Debug output written to /var/log/leapp/leapp-preupgrade.log


============================================================
                           REPORT                           
============================================================
[...]

At the end of "leapp upgrade" run we do not print message
about generated reports, in contrast to "leapp preupgrade"
where we do.
@Rezney
Copy link
Member Author

Rezney commented Jan 28, 2020

@drehak newline done

@drehak drehak merged commit 27dca40 into oamg:master Jan 28, 2020
pirat89 added a commit to pirat89/leapp that referenced this pull request Apr 7, 2020
## Packaging
- Add BuildRequires on python2-setuptools
- Add new dependency on python?-requests
- Add unversioned dependency on leapp-repository and provide
  the leapp-framework capability in python2-leapp (see docs about
  "new dependency mechanism") (oamg#591)
- Move all leapp and snactor files into related rpms istead of python?-leapp (oamg#591)
- Remove dependency on Jinja2

## Framework
### Fixes
- Fix json export capabilities using serialization (oamg#598)

### Enhancements
- Add the DESKTOP tag for the leapp report (oamg#612)
- Dialogs are non-interactive and redesigned significantly; but in beta support
  (see the known issue)
- Introduce DialogModel that could be processed by actors to add related
  information into the report (oamg#589)
- Introduce Workflow API (see the tutorial) (oamg#618)
- Report inhibitors seprately from errors on stdout (oamg#620)
- Show progress in non verbose executions (oamg#621)

### Known issue
- The answerfile is not generated on some machines after the run of leapp.
  Currently it's under investigation and it will be fixed definitely in the
  next release.

## Leapp
### Fixes
- Print message about generated report when `leapp upgrade` ends, as we do for
  `leapp preupgrade` (oamg#573)

### Enhancements
- Add `leapp answer` to answer Dialog questions on CLI (oamg#592)
- Add the --no-rhsm option for (pre)upgrade commands (oamg#622)
- Display warning when leapp is used in unsupported (devel/testing) mode (oamg#577)
- Print errors on stdout in pretty format (oamg#593)
- The error messages are part of the preupgrade report
- The verbosity options (--verbose | --debug) are available for leapp commands as well

## Snactor
### Fixes
### Enhancements

## stdlib
### Fixes

### Enhancements
- Add `stdin` and `encoding` parameters in the run function (oamg#583, oamg#595)

## Modifications
- Code is compatible with Python3 pylint
pirat89 added a commit to pirat89/leapp that referenced this pull request Apr 16, 2020
## Packaging
- Add BuildRequires on python2-setuptools
- Add new dependency on python2-requests
- Add unversioned dependency on leapp-repository and provide
  the leapp-framework capability in python2-leapp (see docs about
  "new dependency mechanism") (oamg#591)
- Move all leapp and snactor files into related rpms instead of python?-leapp (oamg#591)
- Remove dependency on Jinja2

## Framework
### Fixes
- Fix json export capabilities using serialization (oamg#598)

### Enhancements
- Add the DESKTOP tag for the leapp report (oamg#612)
- Dialogs are non-interactive and redesigned significantly; but in beta support
  (see the known issue)
- Introduce DialogModel that could be processed by actors to add related
  information into the report (oamg#589)
- Introduce Workflow API (see the Workflow APIs tutorial) (oamg#618)
- Report inhibitors separately from errors on stdout (oamg#620)
- Show progress in non-verbose executions (oamg#621)

### Known issue
- The answerfile is not generated on some machines after the run of leapp.
  Currently it's under investigation.

## Leapp
### Fixes
- Print message about generated report when `leapp upgrade` ends, as we do for
  `leapp preupgrade` (oamg#573)

### Enhancements
- Add `leapp answer` to answer Dialog questions in CLI (oamg#592)
- Add the --no-rhsm option for (pre)upgrade commands (oamg#622)
- Add the --enablerepo option for Leapp to use an existing custom yum/dnf
repository during the upgrade
- Display a warning when leapp is used in an unsupported (devel/testing) mode (oamg#577)
- Print errors on stdout in pretty format (oamg#593)
- Error messages are now part of the preupgrade report
- The verbosity options (--verbose | --debug) are available for leapp commands as well

## stdlib
### Enhancements
- Add `stdin` and `encoding` parameters in the run function (oamg#583, oamg#595)

## Modifications
- Code is compatible with Python3 pylint
@pirat89 pirat89 mentioned this pull request Apr 16, 2020
pirat89 added a commit that referenced this pull request Apr 16, 2020
## Packaging
- Add BuildRequires on python2-setuptools
- Add new dependency on python2-requests
- Add unversioned dependency on leapp-repository and provide
  the leapp-framework capability in python2-leapp (see docs about
  "new dependency mechanism") (#591)
- Move all leapp and snactor files into related rpms instead of python?-leapp (#591)
- Remove dependency on Jinja2

## Framework
### Fixes
- Fix json export capabilities using serialization (#598)

### Enhancements
- Add the DESKTOP tag for the leapp report (#612)
- Dialogs are non-interactive and redesigned significantly; but in beta support
  (see the known issue)
- Introduce DialogModel that could be processed by actors to add related
  information into the report (#589)
- Introduce Workflow API (see the Workflow APIs tutorial) (#618)
- Report inhibitors separately from errors on stdout (#620)
- Show progress in non-verbose executions (#621)

### Known issue
- The answerfile is not generated on some machines after the run of leapp.
  Currently it's under investigation.

## Leapp
### Fixes
- Print message about generated report when `leapp upgrade` ends, as we do for
  `leapp preupgrade` (#573)

### Enhancements
- Add `leapp answer` to answer Dialog questions in CLI (#592)
- Add the --no-rhsm option for (pre)upgrade commands (#622)
- Add the --enablerepo option for Leapp to use an existing custom yum/dnf
repository during the upgrade
- Display a warning when leapp is used in an unsupported (devel/testing) mode (#577)
- Print errors on stdout in pretty format (#593)
- Error messages are now part of the preupgrade report
- The verbosity options (--verbose | --debug) are available for leapp commands as well

## stdlib
### Enhancements
- Add `stdin` and `encoding` parameters in the run function (#583, #595)

## Modifications
- Code is compatible with Python3 pylint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants