Skip to content

Conversation

@yuazhe
Copy link
Contributor

@yuazhe yuazhe commented Sep 26, 2025

What I did

Following this HLD for liquid cooling sonic-net/SONiC#2032, I created this pr to add relevant CLI for checking liquid cooling status

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

show platform leakage status
Name       Leaking  
-----------------
leak_sensors1    NO   
leak_sensors2    NO   
...
leak_sensorsX    Yes   

show system-health detail
System services and devices monitor list
Name                     Status    Type
-----------------------  --------  ----------
...
leak_sensors1            OK        LiquidCooling
leak_sensors2            OK        LiquidCooling
...
leak_sensors3            Not OK    LiquidCooling

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@keboliu
Copy link
Collaborator

keboliu commented Oct 22, 2025

@judyjoseph would you please help to review this PR?

@judyjoseph
Copy link
Contributor

@yuazhe please add sample o/p fof the new CLI added in PR description

@yuazhe
Copy link
Contributor Author

yuazhe commented Oct 27, 2025

@yuazhe please add sample o/p fof the new CLI added in PR description

thanks, I have updated it accordingly.

Copy link

@nate-nexthop nate-nexthop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM.


from swsscommon.swsscommon import SonicV2Connector

header = ['Name', 'Leak']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change the header for the leak sensor status to Leaking from Leak?

show platform leakage status
Name       Leaking  
-----------------
leak_sensors1    NO   
leak_sensors2    NO   
...
leak_sensorsX    Yes   

I think this reads better. You would ask the question, "Is this sensor leaking?". Not, "Is this sensor leak?".

Copy link
Collaborator

@liat-grozovik liat-grozovik Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. @yuazhe please handle and sync team internally . then i can move on and merge it

Copy link
Contributor Author

@yuazhe yuazhe Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nate-nexthop thanks for the correction, I have updated the PR

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Please update the PR description with the new text

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done @liat-grozovik we should be good to move forward here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

judyjoseph
judyjoseph previously approved these changes Nov 10, 2025
Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please resolve comments as well

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants