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

Reduce the noise around running scriptlets #1606

Merged
merged 2 commits into from
Aug 5, 2024
Merged

Conversation

m-blaha
Copy link
Member

@m-blaha m-blaha commented Aug 2, 2024

Changes:

d1ffe89 (Marek Blaha, 9 minutes ago)
dnf5: Reduce the noise around running scriptlets

If the scriptlet finishes successfully, do not print the
"Running scriptlet..." / "Stop scriptlet..." pair of messages in the
transaction progress bar.

The "Running scriptlet" message is displayed during the scriptlet execution
to inform the user that a time-consuming operation is in progress. If the
scriptlet completes successfully, the message is removed. If it fails, the
message remains, and an error/warning message and the "Stop scriptlet"
message is printed to enable debugging.

b3163df (Marek Blaha, 12 minutes ago)
cli: Method to drop the last progress bar message

Useful to create "temporary" messages, will be used to reduce the
"Running scriptlet" / "Stop scriptlet" rpm transaction noise.

Resolves: #1533

m-blaha added 2 commits August 2, 2024 13:24
Useful to create "temporary" messages, will be used to reduce the
"Running scriptlet" / "Stop scriptlet" rpm transaction noise.
If the scriptlet finishes successfully, do not print the
"Running scriptlet..." / "Stop scriptlet..." pair of messages in the
transaction progress bar.

The "Running scriptlet" message is displayed during the scriptlet
execution to inform the user that a time-consuming operation is in
progress. If the scriptlet completes successfully, the message is
removed. If it fails, the message remains, and an error/warning message
and the "Stop scriptlet" message is printed to enable debugging.
@m-blaha m-blaha force-pushed the mblaha/scriptlet-output branch from d1ffe89 to e10dc24 Compare August 2, 2024 11:49
@m-blaha m-blaha requested a review from jan-kolarik August 2, 2024 12:39
@ppisar
Copy link
Contributor

ppisar commented Aug 2, 2024

I haven't run code yet, but it looks reasonable.

The only concern I have is flickering the line by periodical printing and erasing the message. Maybe we should think about rate limiting of the progress bar. But that's an idea for another pull request.

Copy link
Contributor

@ppisar ppisar left a comment

Choose a reason for hiding this comment

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

I tested the code and it works for me.

@m-blaha
Copy link
Member Author

m-blaha commented Aug 2, 2024

It should not flicker - the starting message for the scriptlet is printed and then erased only once. So it should be visible during execution of the scriptlet, and eventually erased after the scriptlet successfully finishes.

@jan-kolarik jan-kolarik self-assigned this Aug 5, 2024
switch (return_code) {
case RPMRC_OK:
// remove the script_start message in case the script finished successfully
active_progress_bar->pop_message();
Copy link
Member

@jan-kolarik jan-kolarik Aug 5, 2024

Choose a reason for hiding this comment

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

As @ppisar mentioned, I've also noticed the flickering. There is always a flash of an empty line after the message is popped and before the next one is printed. I've tried to modify the implementation locally to always just switch on a flag that we have to rewrite the last message next time when adding a new one. It seems the flickering is gone then, but still some edge cases seem need to be covered, like when the message is the last one in the given bar. Let's defer it to a separate issue, see #1611.

@Conan-Kudo Conan-Kudo added this pull request to the merge queue Aug 5, 2024
Merged via the queue into main with commit 1be56c5 Aug 5, 2024
16 checks passed
@Conan-Kudo Conan-Kudo deleted the mblaha/scriptlet-output branch August 5, 2024 09:08
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.

Excessive scriptlet output during the transaction
4 participants