Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions sonic-psud/scripts/psud
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,11 @@ class DaemonPsud(daemon_base.DaemonBase):

# Connect to STATE_DB and create psu/chassis info tables
state_db = daemon_base.db_connect("STATE_DB")
self.chassis_tbl = swsscommon.Table(state_db, CHASSIS_INFO_TABLE)
self.psu_tbl = swsscommon.Table(state_db, PSU_INFO_TABLE)
self.fan_tbl = swsscommon.Table(state_db, FAN_INFO_TABLE)
self.phy_entity_tbl = swsscommon.Table(state_db, PHYSICAL_ENTITY_INFO_TABLE)
self.statedb_redisPipeline = swsscommon.RedisPipeline(state_db, 10)
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE, True)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE, True)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE, True)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE, True)
Comment on lines +407 to +410
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.

Suggested change
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE, True)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE, True)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE, True)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE, True)
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This parameter is needed for the Redis Pipeline implementation and the Table constructor actually does allow these arguments as shown here Table Constructor

Comment on lines +407 to +410
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.

Suggested change
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE, True)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE, True)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE, True)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE, True)
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This parameter is needed for the Redis Pipeline implementation and the Table constructor actually does allow these arguments as shown here Table Constructor

Comment on lines +407 to +410
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.

Suggested change
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE, True)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE, True)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE, True)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE, True)
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This parameter is needed for the Redis Pipeline implementation and the Table constructor actually does allow these arguments as shown here Table Constructor

Comment on lines +407 to +410
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.
Call to Table.init with too many arguments; should be no more than 2.

Suggested change
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE, True)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE, True)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE, True)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE, True)
self.chassis_tbl = swsscommon.Table(self.statedb_redisPipeline, CHASSIS_INFO_TABLE)
self.psu_tbl = swsscommon.Table(self.statedb_redisPipeline, PSU_INFO_TABLE)
self.fan_tbl = swsscommon.Table(self.statedb_redisPipeline, FAN_INFO_TABLE)
self.phy_entity_tbl = swsscommon.Table(self.statedb_redisPipeline, PHYSICAL_ENTITY_INFO_TABLE)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This parameter is needed for the Redis Pipeline implementation and the Table constructor actually does allow these arguments as shown here Table Constructor


# Post psu number info to STATE_DB
self.num_psus = _wrapper_get_num_psus()
Expand Down Expand Up @@ -459,6 +460,11 @@ class DaemonPsud(daemon_base.DaemonBase):
if self.first_run:
self.first_run = False

# flush any remaining requests on the pipeline to STATE_DB at end of every cycle
try:
self.statedb_redisPipeline.flush()
except Exception as e:
self.log_error(f"Exception occurred while flushing Redis pipeline : {type(e).__name__} : {e}")
return True

def update_psu_data(self):
Expand Down
12 changes: 11 additions & 1 deletion sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,19 @@

STATE_DB = ''

class RedisPipeline:
def __init__(self, db, batch_size=128):
self.db = db
self.batch_size = batch_size
self.queue = []

def flush(self):
# Mock flush operation - just clear the queue
self.queue.clear()

class Table:
def __init__(self, db, table_name):
def __init__(self, db_or_pipeline, table_name, buffered=False):
# Mock to support both constructors (db, table_name) and (pipeline, table_name, buffered)
self.table_name = table_name
self.mock_dict = {}

Expand Down
12 changes: 12 additions & 0 deletions sonic-psud/tests/test_DaemonPsud.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,21 @@ def test_run(self):
daemon_psud.update_psu_data = mock.MagicMock()
daemon_psud._update_led_color = mock.MagicMock()
daemon_psud.update_psu_chassis_info = mock.MagicMock()
daemon_psud.statedb_redisPipeline = mock.MagicMock()

daemon_psud.run()
assert daemon_psud.first_run is False
Comment on lines 108 to 109
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The test should verify that statedb_redisPipeline.flush() is called during the successful execution path. Currently, only the exception scenario is explicitly tested. Consider adding an assertion like daemon_psud.statedb_redisPipeline.flush.assert_called() after line 108 to verify the flush operation is invoked in the normal case.

Copilot uses AI. Check for mistakes.
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

daemon_psud.statedb_redisPipeline.flush.assert_called_once()

# Test Redis pipeline flush exception scenario
daemon_psud.statedb_redisPipeline.flush.side_effect = Exception("Flush error")
daemon_psud.log_error = mock.MagicMock()

result = daemon_psud.run()
assert result is True
assert daemon_psud.first_run is False
daemon_psud.log_error.assert_called_once()
assert "Exception occurred while flushing Redis pipeline" in daemon_psud.log_error.call_args[0][0]

def test_update_psu_data(self):
mock_psu1 = MockPsu("PSU 1", 0, True, True)
Expand Down
Loading