Cockpit pcp python follow up #21056
Cockpit pcp python follow up #21056
Conversation
jelly
force-pushed
the
cockpit-pcp-python-follow-up
branch
3 times, most recently
from
8fbc4a0
to
e6d0ca9
Compare
The PCP functionality is now part of the Python bridge which replaces the C cockpit-pcp binary.
We no longer run the PCP bridge.
jelly
force-pushed
the
cockpit-pcp-python-follow-up
branch
2 times, most recently
from
e1f5a57
to
4447e3f
Compare
jelly
force-pushed
the
cockpit-pcp-python-follow-up
branch
from
4447e3f
to
338882d
Compare
There was a problem hiding this comment.
Thanks! At least to me, the python3-pcp vs. pcp handling is still rather muddy -- I think if you do this deliberately, I'd need a high-level write-up which parts look at pcp and the service, and which parts look at the py module, and why they have to disagree.
| const os_release = await read_os_release(); | ||
| const pcp_packages = ["pcp"]; | ||
|
|
||
| // PCP contains the Python module on Arch Linux, for all other distro's it is split up. |
There was a problem hiding this comment.
For our future selves: Please document why we have to explicitly install both: That on Fedora, python3-pcp depends on pcp, and on Debian pcp depends on python3-pcp.
| @@ -1789,8 +1805,10 @@ class MetricsHistory extends React.Component { | |||
| this.setState({ | |||
| loading: false, | |||
| metricsAvailable: false, | |||
| isPythonPCPInstalled: message?.message !== "python3-pcp not installed", | |||
There was a problem hiding this comment.
Please do message.problem === "not-supported" ? That's much better than this human string comparison. Also, your mock hack just does raise ChannelError('not-supported') without a message.
(The ? here is unnecessary -- you already asserted that message.problem exists)
There was a problem hiding this comment.
No, the mock is for a different scenario.
The python PCP bridge implementation is now the default in Cockpit and also supports the beiboot scenario as we can install pcp (and python3-pcp if required) as it does not pull in any cockpit* package) We depend on python3-pcp explicitly as Fedora's pcp package does not pull it in while Debian does.
This could likely already work fine on Arch as cockpit-pcp was split but our tests weren't adjusted.
jelly
force-pushed
the
cockpit-pcp-python-follow-up
branch
from
338882d
to
c225f9d
Compare
Throwing this at CI, this still needs: