Closed rgarner-vivcourt closed 5 months ago
Updates addressing review comments:
#!/usr/bin/python3
throughout instead of /usr/bin/env
Testing done on Red Hat Enterprise Linux 9.2 with a solarflare X2541
Traceback (most recent call last):
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 962, in <module>
main()
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 954, in main
action_fn(sessions, *args)
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 652, in action_nodes_rate
run_curses_app(content, refresh=options.interval)
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 464, in run_curses_app
curses.wrapper(curses_app, curses, *args, **kwargs)
File "/usr/lib64/python3.9/curses/__init__.py", line 94, in wrapper
return func(stdscr, *args, **kwds)
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 451, in curses_app
win.addstr(0, 0, content_generator.next())
AttributeError: 'generator' object has no attribute 'next'
I'm not able to duplicate this issue with the current version, running: "solar_capture_monitor" and "solar_capture_monitor dump" with a single solar_capture running in the background. Can you give more details on the situation that provokes the problem? I can't see how your changes are provoking it, so it's presumably something specific to your use-case or system.
Ah, yep, should have included the actual command I was running :/
Command | Result |
---|---|
good | |
dump | good |
list | good |
nodes | working - just pushed a fix for it. |
nodes_rate | fail AttributeError: 'generator' object has no attribute 'next' |
line_rate | fail AttributeError: 'generator' object has no attribute 'next' |
line_total | fail AttributeError: 'generator' object has no attribute 'next' |
dot | good |
Stack traces for the 3 failed commands all look like this:
# solar_capture_monitor line_total
Traceback (most recent call last):
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 959, in <module>
main()
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 951, in main
action_fn(sessions, *args)
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 630, in action_line_rate
periodic_table(sessions, columns, context)
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 591, in periodic_table
periodic_writer(content_generator, sys.stdout)
File "/opt/solarcapture/usr/bin/solar_capture_monitor", line 430, in periodic_writer
stream.write(content_generator.next())
AttributeError: 'generator' object has no attribute 'next'
Right, gotcha. Duplicated, we'd best add those subcommands to the tests too.
Ok, patch that appears to fix those:
diff --git a/src/python/solar_capture/tabulate.py b/src/python/solar_capture/tabulate.py
index f069566..d6d70c5 100644
--- a/src/python/solar_capture/tabulate.py
+++ b/src/python/solar_capture/tabulate.py
@@ -21,10 +21,9 @@ def is_int(str):
def auto_justify_col(col, width):
if min(map(is_int, col[1:])):
# All fields (possibly excepting first) are integers.
- just = string.rjust
+ return [f.rjust(width) for f in col]
else:
- just = string.ljust
- return [just(f, width) for f in col]
+ return [f.ljust(width) for f in col]
def justify_col_by_field(justify_field):
@@ -32,7 +31,6 @@ def justify_col_by_field(justify_field):
return [justify_field(f, width) for f in col]
return justify_col
-
def pad_table(rows, justify_field=None, justify_col=None, col_widths=None):
if justify_field is None and justify_col is None:
justify_col = auto_justify_col
@@ -44,7 +42,7 @@ def pad_table(rows, justify_field=None, justify_col=None, col_widths=None):
widths = [max(len(f) for f in col) for col in cols]
if col_widths:
- widths = map(max, widths, col_widths)
+ widths = list(map(max, widths, col_widths))
for i, w in enumerate(widths):
col_widths[i] = w
col_widths[i] = w
@@ -80,8 +78,8 @@ def playpen():
widths = [0] * 3
print( fmt_table(test_data, col_widths=widths) )
- l = string.ljust
- r = string.rjust
+ l = "".ljust
+ r = "".rjust
print( fmt_table(test_data, col_widths=widths,
justify_field=[r, l, l], colsep=' | ') )
diff --git a/src/solar_capture_monitor b/src/solar_capture_monitor
index 346fac8..a1fe359 100644
--- a/src/solar_capture_monitor
+++ b/src/solar_capture_monitor
@@ -13,7 +13,7 @@ if os.path.exists(os.path.join(top, 'src', 'python', 'solar_capture')):
import solar_capture as sc
import solar_capture.stats as stats
import solar_capture.tabulate as tab
-
+from functools import reduce
usage_text = '''
solar_capture_monitor [options] [sessions] [commands]
@@ -427,7 +427,7 @@ def periodic_writer(content_generator, stream):
try:
t_next_wake = time.time()
while True:
- stream.write(content_generator.next())
+ stream.write(next(content_generator))
stream.flush()
time_now = time.time()
while t_next_wake - time_now <= 0.0:
@@ -445,7 +445,7 @@ def curses_app(stdscr, curses, content_generator, refresh=1.0):
done = False
while not done:
win.erase()
- win.addstr(0, 0, content_generator.next())
+ win.addstr(0, 0, next(content_generator))
win.refresh()
# Wait [refresh] secs, but respond to key-press within 1/10th of sec.
for i in range(int(refresh / 0.1)):
Thanks @abrunnin-xilinx, that patch fixes my remaining issues. Would you like me to incorporate it in my PR or commit it yourself ?
I added it, along with a unit test for it. Because clearly we didn't exercise those part of monitor, and ought to, to stop it coming back.
Some minor fixes to bring scripts up to python3.