-
Notifications
You must be signed in to change notification settings - Fork 141
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
needs-restarting: Add --exclude-services #556
needs-restarting: Add --exclude-services #556
Conversation
cb57c07
to
1cd6d32
Compare
@martinpitt would you mind reviewing this to see whether it would work for you? |
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.
Thanks @evan-goode ! This looks correct and provides what we need for dnf4.
Hmm, I guess our ci-dnf-stack test suite is not really prepared to test this functionality (it doesn't test |
For consumers of `dnf4 needs-restarting`'s output, it's useful to separate the systemd services needing restart vs. the processes not managed by systemd that need restarting. This new `dnf4 needs-restarting --exclude-services` ONLY prints information about processes NOT managed by systemd, and for listing systemd services that need to be restarted, the existing `--services` flag can be used. When both `--services` and `--exclude-services` are passed `--exclude-services` is ignored. For https://issues.redhat.com/browse/RHEL-56137.
e05dff9
to
6977682
Compare
if not self.opts.exclude_services: | ||
stale_pids.add(pid) |
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.
I didn't realize this before but now that the --services
option prints only stale_service_names
there is no point in adding to the stale_pids
when --services
is set right?
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.
I suppose, but I think it's fine to leave it.
The PR has been tested downstream in RHEL, everything passed. |
The PR has also been tested in Fedora40, everything passed. I'd say it's possible to merge the PR. |
For consumers of
dnf4 needs-restarting
's output, it's useful to separate the systemd services needing restart vs. the processes not managed by systemd that need restarting. This newdnf4 needs-restarting --exclude-services
ONLY prints information about processes NOT managed by systemd, and for listing systemd services that need to be restarted, the existing--services
flag can be used.When both
--services
and--exclude-services
are passed--exclude-services
is ignored.For https://issues.redhat.com/browse/RHEL-56137.
CI PR on the way.