[202012] Refactor Pcied and add unittest #199
Conversation
|
This pull request introduces 1 alert when merging cde1d42 into 507cdf7 - view on LGTM.com new alerts:
|
qiluo-msft
changed the title
|
Could you fix PR checkers? |
|
@sujinmkang please note that without this PR we have a degradation in 202012. |
|
I rebased your commits and force pushed. Still there is PR checker failure. |
| daemon_pcied.check_n_update_pcie_aer_stats(0,1,0) | ||
| assert daemon_pcied.update_aer_to_statedb.call_count == 0 | ||
| assert daemon_pcied.device_table.set.call_count == 0 | ||
| assert platform_pcieutil.get_pcie_aer_stats.call_count == 0 |
There was a problem hiding this comment.
Test is failing due to "NameError: global name 'platform_pcieutil' is not defined"
I belive you meant to write:
| assert platform_pcieutil.get_pcie_aer_stats.call_count == 0 | |
| assert pcied.platform_pcieutil.get_pcie_aer_stats.call_count == 0 |
| daemon_pcied.check_n_update_pcie_aer_stats(0,1,0) | ||
| assert daemon_pcied.update_aer_to_statedb.call_count == 1 | ||
| assert daemon_pcied.device_table.set.call_count == 1 | ||
| assert platform_pcieutil.get_pcie_aer_stats.call_count == 1 |
There was a problem hiding this comment.
Same here:
| assert platform_pcieutil.get_pcie_aer_stats.call_count == 1 | |
| assert pcied.platform_pcieutil.get_pcie_aer_stats.call_count == 1 |
sonic-pcied/scripts/pcied
Outdated
| from sonic_platform.pcie import Pcie | ||
| _platform_pcieutil = Pcie(platform_path) | ||
| except ImportError as e: | ||
| self.log_error("Failed to load platform Pcie module. Error : {}".format(str(e)), True) |
There was a problem hiding this comment.
This caused in master for pcied to enter FATAL state (sonic-net/sonic-buildimage#7993)
This probably need the same fix as in master.
I also think this line shouldn't be an error level, since this is not an error flow, this is part of the normal loading flow we expect when a vendor didn't supply a Pcie class.
| self.log_error("Failed to load platform Pcie module. Error : {}".format(str(e)), True) | |
| log.log_notice("Failed to load platform Pcie module. Error : {}, Fallback to default module".format(str(e)), True) |
sonic-pcied/scripts/pcied
Outdated
| from sonic_platform_base.sonic_pcie.pcie_common import PcieUtil | ||
| _platform_pcieutil = PcieUtil(platform_path) | ||
| except ImportError as e: | ||
| self.log_error("Failed to load default PcieUtil module. Error : {}".format(str(e)), True) |
There was a problem hiding this comment.
| self.log_error("Failed to load default PcieUtil module. Error : {}".format(str(e)), True) | |
| log.log_error("Failed to load default PcieUtil module. Error : {}".format(str(e)), True) |
sonic-pcied/scripts/pcied
Outdated
| import sys | ||
| import threading | ||
|
|
||
| from sonic_py_common import daemon_base, device_info |
There was a problem hiding this comment.
This change is needed for the fixes on line 48 & 53 to run
| from sonic_py_common import daemon_base, device_info | |
| from sonic_py_common import daemon_base, device_info, logger |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@sujinmkang please note than the you will need to have the submodule of 202012 back to buildimage before this can get in as there was recently a revert. |
|
@DavidZagury if you have no further comments, can you please approve this PR? |
…ns (#8262) sonic-platform-daemons: * c90bb29 [202012] Refactor Pcied and add unittest (sonic-net/sonic-platform-daemons#199) sonic-platform-common: * 9a59e19 Revert "Unifying the platform api for get_pcie_aer_stats with PcieBase (sonic-net/sonic-platform-common#197)" (sonic-net/sonic-platform-common#207) * d35960b Revert "Revert "Unifying the platform api for get_pcie_aer_stats with PcieBase (sonic-net/sonic-platform-common#197)" (sonic-net/sonic-platform-common#207)" (sonic-net/sonic-platform-common#210)
#### Description Following error is seen while parsing EEPROM fields for few transceivers. 'utf-8' codec can't decode byte 0xff in position 6: invalid start byte #### Motivation and Context EEPROM fields will not be displayed if unicode characters are not properly. E.g. Vendor Date: 20'u-tf--8'
Description
Refactor the pcied and add the unit test
Motivation and Context
Added unit test to increase the pmon unit test coverage.
How Has This Been Tested?
Build with unit test enabled and run manually on a dut to verify the pcied.
Additional Information (Optional)