Skip to content

Commit 6baf219

Browse files
committed
[tempershow]: Address review comments
- _init_sfp_util_helper: also catch SystemExit raised by platform_sfputil_helper load functions so the CLI falls back to logical port names instead of terminating. - _get_sfp_db_connections: keep successful per-namespace STATE_DB connections and skip only the failing namespace, instead of dropping all connections on any failure. - _collect_sfp_sensors: prefetch TRANSCEIVER_DOM_THRESHOLD and TRANSCEIVER_DOM_FLAG once per namespace via new _prefetch_table() to avoid an N+1 hgetall pattern per port. - _derive_sfp_warning: return 'True'/'False'/'N/A' strings to keep the JSON output's Warning field type consistent with platform sensor rows from TEMPERATURE_INFO. Signed-off-by: Vasundhara Volam <vvolam@microsoft.com>
1 parent a1e3d9c commit 6baf219

1 file changed

Lines changed: 48 additions & 17 deletions

File tree

scripts/tempershow

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class TemperShow(object):
8686
platform_sfputil_helper.load_platform_sfputil()
8787
platform_sfputil_helper.platform_sfputil_read_porttab_mappings()
8888
return True
89-
except Exception:
89+
except (Exception, SystemExit):
9090
return False
9191

9292
def _sfp_display_name(self, port_name):
@@ -118,11 +118,16 @@ class TemperShow(object):
118118
connections = []
119119
if multi_asic is not None and multi_asic.is_multi_asic():
120120
try:
121-
for ns in multi_asic.get_front_end_namespaces():
122-
db = multi_asic.connect_to_all_dbs_for_ns(ns)
123-
connections.append(db)
121+
namespaces = multi_asic.get_front_end_namespaces()
124122
except Exception:
125-
connections = []
123+
namespaces = []
124+
# Connect per-namespace so a single failing namespace does not
125+
# silently drop SFP rows from the other namespaces.
126+
for ns in namespaces:
127+
try:
128+
connections.append(multi_asic.connect_to_all_dbs_for_ns(ns))
129+
except Exception:
130+
continue
126131
if not connections:
127132
db = SonicV2Connector(host="127.0.0.1")
128133
db.connect(db.STATE_DB)
@@ -180,6 +185,8 @@ class TemperShow(object):
180185
continue
181186
if not keys:
182187
continue
188+
thr_map = self._prefetch_table(db, SFP_THRESHOLD_TABLE_NAME)
189+
flag_map = self._prefetch_table(db, SFP_FLAG_TABLE_NAME)
183190
for key in natsorted(keys):
184191
key_list = key.split('|')
185192
if len(key_list) != 2:
@@ -190,12 +197,8 @@ class TemperShow(object):
190197
continue
191198
seen_ports.add(port)
192199
temp_dict = db.get_all(db.STATE_DB, key) or {}
193-
thr_dict = db.get_all(
194-
db.STATE_DB,
195-
'{}|{}'.format(SFP_THRESHOLD_TABLE_NAME, port)) or {}
196-
flag_dict = db.get_all(
197-
db.STATE_DB,
198-
'{}|{}'.format(SFP_FLAG_TABLE_NAME, port)) or {}
200+
thr_dict = thr_map.get(port, {})
201+
flag_dict = flag_map.get(port, {})
199202
rows.append({
200203
'name': self._sfp_display_name(port),
201204
'temperature': temp_dict.get(SFP_TEMPER_FIELD_NAME, NA),
@@ -208,13 +211,41 @@ class TemperShow(object):
208211
})
209212
return rows
210213

214+
@staticmethod
215+
def _prefetch_table(db, table_name):
216+
"""Build a {port: {field: value}} map for an entire STATE_DB table.
217+
218+
Used to avoid per-port hgetall round trips when populating SFP rows.
219+
Returns an empty dict on any DB error.
220+
"""
221+
result = {}
222+
try:
223+
keys = db.keys(db.STATE_DB, table_name + '|*')
224+
except Exception:
225+
return result
226+
if not keys:
227+
return result
228+
for key in keys:
229+
parts = key.split('|')
230+
if len(parts) != 2:
231+
continue
232+
try:
233+
result[parts[1]] = db.get_all(db.STATE_DB, key) or {}
234+
except Exception:
235+
continue
236+
return result
237+
211238
@staticmethod
212239
def _derive_sfp_warning(flag_dict):
213-
"""Return True/False/N/A based on temp flags in TRANSCEIVER_DOM_FLAG.
240+
"""Return 'True'/'False'/'N/A' based on temp flags in TRANSCEIVER_DOM_FLAG.
241+
242+
Returns string values to stay consistent with the platform sensor
243+
warning column (which is read as a string from TEMPERATURE_INFO),
244+
keeping the JSON output's 'Warning' field type stable across rows.
214245
215-
True - any of temphigh/lowwarning/alarm is asserted
216-
False - all four flags are present and de-asserted
217-
N/A - none of the four flags are present
246+
'True' - any of temphigh/lowwarning/alarm is asserted
247+
'False' - all four flags are present and de-asserted
248+
'N/A' - none of the four flags are present
218249
"""
219250
if not flag_dict:
220251
return NA
@@ -225,8 +256,8 @@ class TemperShow(object):
225256
continue
226257
seen = True
227258
if str(value).strip().lower() == 'true':
228-
return True
229-
return False if seen else NA
259+
return 'True'
260+
return 'False' if seen else NA
230261

231262
def show(self, output_json):
232263
rows = self._collect_platform_sensors() + self._collect_sfp_sensors()

0 commit comments

Comments
 (0)