Add needrestart for tracing of services #124
Conversation
test/test_katello/test_deb_tracer.py
Outdated
| LC_NUMERIC = "fr_FR.UTF-8", | ||
| LANG = "en_US.UTF-8" | ||
| are supported and installed on your system. | ||
| perl: warning: Falling back to a fallback locale ("en_US.UTF-8"). |
There was a problem hiding this comment.
The mock data should only contain the real output - not the locale warnings.
There was a problem hiding this comment.
I also think that goes to stderr, which is a good point to think about. https://docs.python.org/3/library/subprocess.html#subprocess.check_output says stderr is discarded by default and can be captured optionally. So to capture it manually, I'd run needrestart -b 2> /dev/null.
|
[test katello-host-tools] |
louisrokitta
force-pushed
the
expand_tracing_of_services_need_restart
branch
from
7d4540a
to
63b210b
Compare
louisrokitta
force-pushed
the
expand_tracing_of_services_need_restart
branch
2 times, most recently
from
eb380bc
to
6b760b4
Compare
|
I'm wondering if the functionality implemented here would be a better fit in frostyx/tracer . It's what provides the traces functionality to katello-host-tools. By enhancing frostyx/tracer we would benefit - and so would 'tracer' users themselves. |
This was our first thought. Unfortunately, there is no tracer package available for SLES and debian because they have different approaches. Additionally, the implementation is pretty easy because we can use already existing apps. |
|
Oh that's an excellent point. It looks like tracer already supports Debian so how hard would it be to package tracer for Debian? |
Unfortunately, the debian support in tracer isn't working / is not working completely and the "debian preferred way" is to use the data from needrestart. Therefore we used this solution - which is much easier and more reliable. |
|
Is there a discussion with tracer to refer to, just to close the loop? |
No. We tried the tracer on debian and found out, that it doesn't work currently and that there are some missing parts. Then we saw, that needrestart a) is already available as a package b) is "standard" in debian/ubuntu world c) works as expected and d) can be integrated easily. |
|
I think it's a missed opportunity if you didn't try to collaborate with upstream tracer to make it work. Their readme states it's supposed to work with Debian so they care about it. That keeps this project smaller because it can rely on a common API. |
|
Understand your point regarding one API but if the truth is, that needrestart is packaged already and therefore widely used in debian environments, I see no reason to spend time to work on a project to create a second solution. |
|
Is there a reason tracer couldn't use |
|
|
@jlsherrill can you please continue to discuss this topic? Together with #126 |
|
I'm gonna work with @parthaa to get it reviewed |
|
Just to be clear, I don't explicitly object to merging this but I think it'd be good to keep things consistent. Ideally every platform fits in the same interface. That is still a goal that I think is good to keep in mind. That can also mean opening an issue to start a discussion. |
louisrokitta
force-pushed
the
expand_tracing_of_services_need_restart
branch
from
39c1500
to
a8c6827
Compare
louisrokitta
force-pushed
the
expand_tracing_of_services_need_restart
branch
from
a8c6827
to
9a41e48
Compare
|
Was able to generate traces and check the functionality. ACK |
Add "needrestart" to trace services which need to be restarted.