Change PSU key to use get_name API instead of index #446
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 11, 2024
Open
gechiang
approved these changes
Jun 3, 2024
|
/azp run |
|
Commenter does not have sufficient privileges for PR 446 in repo sonic-net/sonic-platform-daemons |
|
/azpw run |
|
@gregoryboudreau useful addition. Couple questions:
Additionally, please fix unit test failures that appear to be indexing related. Thanks! |
|
@gregoryboudreau can you please run sonic-mgmt test cases for PSU with all the changes. Attach the result to this PR? |
Let me know if there are other test cases that I missed in running, thanks! |
|
@assrinivasan with the get_name API implemented: This is with the get_name API overwritten to raise NotImplemented for PSUs: The only functional difference between the two is the key/naming |
|
@assrinivasan : can we please help with merge this. |
|
@prgeor please help review/merge as appropriate |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
judyjoseph
reviewed
Mar 12, 2025
| @@ -122,6 +122,14 @@ def _wrapper_get_psu_status(psu_index): | |||
| # | |||
|
|
|||
| def get_psu_key(psu_index): | |||
| if platform_chassis is not None: | |||
| try: | |||
| return platform_chassis.get_psu(psu_index - 1).get_name() | |||
There was a problem hiding this comment.
I have seen many platform drivers implementing the name as below
device = "PSU{}".format(self.psu_index)
no space between PSU and [index]
while PSU_INFO_KEY_TEMPLATE is of the format "PSU [index]". There is a space after PSU substring
Is this ok ?
There was a problem hiding this comment.
this change would allow for both. If a vendor has defined the get_name() api it will fall back to the original implementation of the enumerated index based name (full key: "PSU_INFO|PSU 1" for example). With the get name api implemented though the vendor gets full determination of this name: (full key: "PSU_INFO|PSU0" or "PSU_INFO|psutray0.psu0" for example).
mlok-nokia
approved these changes
Mar 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change PSU key to use get_name API instead of index
Description
Changes PSU keys to be implemented via their get_name API like we do for fans instead of simple indexing. If the get_name API is not implemented by vendors then will fall back to the original indexing schema.
Motivation and Context
Makes PSU objects more clear in context by attaching their associated get_name to their PSU key similar to how it is done for fan objects.
How Has This Been Tested?
Along with other modifications in other repos, this is passing mgmt related test cases such as snmp tests and psu api tests. Additionally, can verify the output via
psushowscript.Additional Information (Optional)
Related PRs:
sonic-net/sonic-mgmt#11944
sonic-net/sonic-utilities#3208
sonic-net/sonic-snmpagent#312