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
Collect sensor readings from Lenovo/IBM PDUs (Power Distribution Units) #1475
Conversation
python/nav/mibs/ibm_pdu_mib.py
Outdated
|
||
|
||
class IbmPduMib(MibRetriever): | ||
""" Custom class for retrieveing sensors from APC UPSes.""" |
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.
Maybe update docstrings?
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.
Definitely :-)
python/nav/mibs/ibm_pdu_mib.py
Outdated
self._logger.debug("Got phase power readings: %r", phases) | ||
|
||
result = [] | ||
column = self.nodes.get(PHASE_LAST_POWER_READING, None) |
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.
Either don't put a default here, or do not assume this is not None later in the code
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.
A good point. It would be really bad if the OID definition isn't present in the MIB we already compiled ourselves - this is what you get for using older code as inspiration ;)
python/nav/mibs/ibm_pdu_mib.py
Outdated
result = [] | ||
for index, row in outlets.items(): | ||
for sensor in self._outlet_row_to_sensors(index, row): | ||
result.append(sensor) |
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.
Maybe use result.extend()
rather than an extra for loop?
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.
Yes.
python/nav/mibs/ibm_pdu_mib.py
Outdated
description = row.get(OUTLET_DESCRIPTION) | ||
|
||
voltage = dict( | ||
oid=str(self.nodes.get(OUTLET_VOLTAGE, None).oid + str(index)), |
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.
None.oid will not work. If the key is missing it makes more sense to get a KeyError than an AttributeError
python/nav/mibs/ibm_pdu_mib.py
Outdated
yield voltage | ||
|
||
current = dict( | ||
oid=str(self.nodes.get(OUTLET_CURRENT, None).oid + str(index)), |
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.
Again
python/nav/mibs/ibm_pdu_mib.py
Outdated
|
||
power = dict( | ||
oid=str(self.nodes.get(OUTLET_LAST_POWER_READING, | ||
None).oid + str(index)), |
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.
And again :)
As requested by SIGMA2, here is basic support for retrieving power readings from phases and outlets on the Lenovo PDUs currently being deployed.