Python3 compatibility

[rgarner-vivcourt]

Some minor fixes to bring scripts up to python3.

[abower-amd]

Looks good to me, modulo python path.

It would be nice to have a 'Testing done' section, at a minimum explaining that you have run the changed version successfully, preferably adding a note on test environment and abridged transcript.

Thanks for the contribution!

[rgarner-vivcourt]

Updates addressing review comments:

• #!/usr/bin/python3 throughout instead of /usr/bin/env
• Syntax fixes in solar_replay, too

Testing done on Red Hat Enterprise Linux 9.2 with a solarflare X2541

• solar_capture tested extensively, running in production on one node for 2 months
• solar_replay lightly tested in our test lab
• Other components tested for syntax only
• solar_capture_monitor passes syntax checks, but fails with:

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'

[abrunnin-xilinx]

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.

[rgarner-vivcourt]

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'

[abrunnin-xilinx]

Right, gotcha. Duplicated, we'd best add those subcommands to the tests too.

[abrunnin-xilinx]

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)):

[rgarner-vivcourt]

Thanks @abrunnin-xilinx, that patch fixes my remaining issues. Would you like me to incorporate it in my PR or commit it yourself ?

[abrunnin-xilinx]

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.

[abrunnin-xilinx]

Looks good, I resolved the conflict with the scm patch.