-
Notifications
You must be signed in to change notification settings - Fork 115
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
Extend rfc3433.py to support more Physical Entity Sensor MIB entries #211
Extend rfc3433.py to support more Physical Entity Sensor MIB entries #211
Conversation
Fan tachometers Thermal sensors PSU Voltage, Current, Power Signed-off-by: Kebo Liu <[email protected]>
Signed-off-by: Kebo Liu <[email protected]>
Signed-off-by: Kebo Liu <[email protected]>
Signed-off-by: Kebo Liu <[email protected]>
…tests/test_sensor.py Signed-off-by: Kebo Liu <[email protected]>
@SuvarnaMeenakshi would you please help to review? |
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.
As comments.
@SuvarnaMeenakshi all your comments have been addressed, please check again. |
@SuvarnaMeenakshi please check my latest commit. and for the LGTM warnings, all of them are legacy code not touched in this PR. |
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.
LGTM
@SuvarnaMeenakshi would you please help to restart the azure pipeline? seems one of the check is blocking. |
@qiluo-msft do we need additional approval? |
/azp run |
Commenter does not have sufficient privileges for PR 211 in repo Azure/sonic-snmpagent |
@qiluo-msft can you please help to rerun azure pipeline? it is pending few days and i have no privilege to do so |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@qiluo-msft seems that the LGTM warning is not related to the change in the PR, and not even in the files that touched this time, what's your view? and also the "snmpagent" seems never going to be finished, it just waiting there after you started 2 days ago. |
I tested and fixed lgtm.yml. Please check and cherry-pick #216 |
Signed-off-by: Qi Luo <[email protected]>
@SuvarnaMeenakshi @qiluo-msft Thanks for the review and fix for the LGTM checker. now everything passed after cherry-picking the LGTM checker fix, please help to proceed. |
…#3357) Extend test_snmp_phy_entity to cover new add phyEntitySensor MIB entries in PR sonic-net/sonic-snmpagent#211 What is the motivation for this PR? cover new added MIB entries in physical entity sensor MIB(RFC3433) How did you do it? Extend current test cases to check the correctness of the newly added MIB entries. How did you verify/test it? Run changed test_snmp_phy_entity Signed-off-by: Kebo Liu <[email protected]>
…sonic-net#3357) Extend test_snmp_phy_entity to cover new add phyEntitySensor MIB entries in PR sonic-net/sonic-snmpagent#211 What is the motivation for this PR? cover new added MIB entries in physical entity sensor MIB(RFC3433) How did you do it? Extend current test cases to check the correctness of the newly added MIB entries. How did you verify/test it? Run changed test_snmp_phy_entity Signed-off-by: Kebo Liu <[email protected]>
- What I did
Extend RFC3433 implementation with:
MIB HLD update PR to reflect this change please refer to: sonic-net/SONiC#766
A fix for the LGTM checker
- How I did it
- How to verify it
Manual test and run updated community SNMP test case(sonic-net/sonic-mgmt#3357).
- Description for the changelog