Added logic to get PSU info directly from the state DB #101
Conversation
Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com>
Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com>
|
@jleveque please review. |
| return None | ||
|
|
||
| psu_index = int(sub_id[0]) | ||
|
|
||
| try: | ||
| num_psus = self.psuutil.get_num_psus() | ||
| psu_num = self._getPsuNum() |
There was a problem hiding this comment.
Why change variable name (and new related getter function name) to "psu_num"? To me, "psu_num" infers the index of a given PSU. If you prefer to start the variable name with "psu_" (as opposed to "num_psus"), I suggest renaming to "psu_count".
Also, please follow PEP8 style rules for consistency (i.e., function names should not be camelCase, but rather lowercase with words separated by underscores as necessary.
There was a problem hiding this comment.
@jleveque The reason for that is official Spec name convention: https://github.com/Azure/SONiC/blob/master/doc/pmon/pmon-enhancement-design.md
1.5.2 Chassis Table
psu_num = INT ; psu numbers on the chassis
There was a problem hiding this comment.
I see. Would you consider changing the variable names?
There was a problem hiding this comment.
@jleveque sure. Will do it.
There was a problem hiding this comment.
@jleveque done. The variable names were aligned as well.
There was a problem hiding this comment.
Thanks. You decided on "num_psus" instead of something else like "psu_count"? If you want the variable name to start with "psu" I would recommend psu_count. We didn't need to go back to "num_psus", but I just think "psu_num" is confusing.
There was a problem hiding this comment.
@jleveque NP. I decided to stay with original name. Do you want me to force psu_count? I don't care to much regarding variables naming, but rather the stuff to be working. The idea of these changes to fix SNMP test.
There was a problem hiding this comment.
I'm fine either way. I just wanted to make it clear that I'm not set on the old name.
There was a problem hiding this comment.
@jleveque ok. Got it. Thanks for the clarifications.
Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com>
nazariig
force-pushed
the
master-get-psu-info-from-db
branch
from
a1c18a6
to
222dc25
Compare
|
|
||
| try: | ||
| module = imp.load_source(PSU_PLUGIN_MODULE_NAME, PSU_PLUGIN_MODULE_PATH) |
There was a problem hiding this comment.
load_source [](start = 25, length = 11)
Since we don't need the module 'psutil', can we remove it from setup.py?
There was a problem hiding this comment.
@qiluo-msft Unfortunately we can't remove it from setup.py
Summary:
From psutil:
psutil (process and system utilities) is a cross-platform library for retrieving information on running processes and system utilization (CPU, memory, disks, network, sensors) in Python.
Usage:
From sonic_ax_impl:
# From the psutil documentation https://pythonhosted.org/psutil/#psutil.cpu_percent:
#
# Warning the first time this function is called
# with interval = 0.0 or None it will return a
# meaningless 0.0 value which you are supposed
# to ignore.
psutil.cpu_percent()
# '...is recommended for accuracy that this function be called with at least 0.1 seconds between calls.'
time.sleep(0.1)
# a sliding window of 60 contiguous 5 sec utilization (up to five minutes)
self.cpuutils = collections.deque([psutil.cpu_percent()], maxlen=60)
self.system_virtual_memory = psutil.virtual_memory()
|
@lguohan please review |
* Added logic to get PSU info directly from the state DB. Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com> * Fixed PSU UTs. Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com> * Aligned method names according to PEP8. Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com>
| if not sub_id: | ||
| return (1,) | ||
|
|
||
| psu_index = self._getPsuIndex(sub_id) | ||
| psu_index = self._get_psu_index(sub_id) |
There was a problem hiding this comment.
_get_psu_index [](start = 25, length = 14)
get_next is nothrow function. However, if the StateDB is not as good as you expect, there will be exception.
This is a sample:
File "/usr/local/lib/python3.6/dist-packages/sonic_ax_impl/mibs/vendor/cisco/ciscoEntityFruControlMIB.py", line 102, in _get_psu_index
num_psus = self._get_num_psus()
File "/usr/local/lib/python3.6/dist-packages/sonic_ax_impl/mibs/vendor/cisco/ciscoEntityFruControlMIB.py", line 65, in _get_num_psus
num_psus = get_chassis_data(chassis_info)
File "/usr/local/lib/python3.6/dist-packages/sonic_ax_impl/mibs/vendor/cisco/ciscoEntityFruControlMIB.py", line 36, in get_chassis_data
return tuple(chassis_info.get(chassis_field.value, b"").decode() for chassis_field in CHASSISInfoDB)
File "/usr/local/lib/python3.6/dist-packages/sonic_ax_impl/mibs/vendor/cisco/ciscoEntityFruControlMIB.py", line 36, in <genexpr>
return tuple(chassis_info.get(chassis_field.value, b"").decode() for chassis_field in CHASSISInfoDB)
AttributeError: 'NoneType' object has no attribute 'get'
Signed-off-by: Nazarii Hnydyn nazariig@mellanox.com
Propagating SNMP test fix.
- What I did
- How I did it
- How to verify it
- Description for the changelog