[PATCH nfs-utils v3 0/7] rpcctl: Flake8 cleanups

[PATCH nfs-utils v3 0/7] rpcctl: Flake8 cleanups

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Apologies for the noise. I just realized I gave the wrong range to `git
format-patch`, and accidentally resent somebody else's patch that has
already been merged.

This is a series of cleanups for rpcctl.py to fix up various style
issues after running `flake8` on the code.

Thoughts?
Anna


Anna Schumaker (7):
  rpcctl: Fix flake8 whitespace errors
  rpcctl: Fix flake8 line-too-long errors
  rpcctl: Fix flake8 bare exception error
  rpcctl: Fix flake8 ambiguous-variable-name error
  rpcctl: Add missing docstrings to the Xprt class
  rpcctl: Add missing docstrings to the XprtSwitch class
  rpcctl: Add remaining missing docstrings

 tools/rpcctl/rpcctl.py | 107 ++++++++++++++++++++++++++++++-----------
 1 file changed, 80 insertions(+), 27 deletions(-)

-- 
2.48.1

[PATCH nfs-utils v3 1/7] rpcctl: Fix flake8 whitespace errors

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index d2110ad6de93..92a851c2278b 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -8,7 +8,7 @@ import socket
 import sys
 
 with open("/proc/mounts", 'r') as f:
-    mount = [ line.split()[1] for line in f if "sysfs" in line ]
+    mount = [line.split()[1] for line in f if "sysfs" in line]
     if len(mount) == 0:
         print("ERROR: sysfs is not mounted")
         sys.exit(1)
@@ -18,6 +18,7 @@ if not sunrpc.is_dir():
     print("ERROR: sysfs does not have sunrpc directory")
     sys.exit(1)
 
+
 def read_addr_file(path):
     try:
         with open(path, 'r') as f:
@@ -25,17 +26,19 @@ def read_addr_file(path):
     except:
         return "(enoent)"
 
+
 def write_addr_file(path, newaddr):
     with open(path, 'w') as f:
         f.write(newaddr)
     return read_addr_file(path)
 
+
 def read_info_file(path):
     res = collections.defaultdict(int)
     try:
         with open(path) as info:
-            lines = [ l.split("=", 1) for l in info if "=" in l ]
-            res.update({ key:int(val.strip()) for (key, val) in lines })
+            lines = [l.split("=", 1) for l in info if "=" in l]
+            res.update({key: int(val.strip()) for (key, val) in lines})
     finally:
         return res
 
@@ -75,7 +78,7 @@ class Xprt:
         if not self.path.exists():
             return f"{self.name}: has been removed"
         return "\n".join([self._xprt(), self._src_reqs(),
-                          self._cong_slots(), self._queues() ])
+                          self._cong_slots(), self._queues()])
 
     def read_state(self):
         if self.path.exists():
@@ -132,7 +135,7 @@ class Xprt:
 
     def get_by_name(name):
         glob = f"**/{name}-*" if name else "**/xprt-*"
-        res = [ Xprt(x) for x in (sunrpc / "xprt-switches").glob(glob) ]
+        res = [Xprt(x) for x in (sunrpc / "xprt-switches").glob(glob)]
         if name and len(res) == 0:
             raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT),
                                     f"{sunrpc / 'xprt-switches' / glob}")
@@ -158,19 +161,19 @@ class XprtSwitch:
         self.path = path
         self.name = path.stem
         self.info = read_info_file(path / "xprt_switch_info")
-        self.xprts = sorted([ Xprt(p) for p in self.path.iterdir() if p.is_dir() ])
+        self.xprts = sorted([Xprt(p) for p in self.path.iterdir() if p.is_dir()])
         self.sep = sep
 
     def __lt__(self, rhs):
         return self.name < rhs.name
 
     def __str__(self):
-        switch =  f"{self.name}{self.sep} " \
-                  f"xprts {self.info['num_xprts']}, " \
-                  f"active {self.info['num_active']}, " \
-                  f"queue {self.info['queue_len']}"
-        xprts = [ f"	{x.small_str()}" for x in self.xprts ]
-        return "\n".join([ switch ] + xprts)
+        switch = f"{self.name}{self.sep} " \
+                 f"xprts {self.info['num_xprts']}, " \
+                 f"active {self.info['num_active']}, " \
+                 f"queue {self.info['queue_len']}"
+        xprts = [f"	{x.small_str()}" for x in self.xprts]
+        return "\n".join([switch] + xprts)
 
     def add_command(subparser):
         parser = subparser.add_parser("switch", help="Commands for xprt switches")
@@ -194,8 +197,8 @@ class XprtSwitch:
     def get_by_name(name):
         xprt_switches = sunrpc / "xprt-switches"
         if name:
-            return [ XprtSwitch(xprt_switches / name) ]
-        return [ XprtSwitch(f) for f in sorted(xprt_switches.iterdir()) ]
+            return [XprtSwitch(xprt_switches / name)]
+        return [XprtSwitch(f) for f in sorted(xprt_switches.iterdir())]
 
     def show(args):
         for switch in XprtSwitch.get_by_name(args.switch):
@@ -234,8 +237,8 @@ class RpcClient:
     def get_by_name(name):
         rpc_clients = sunrpc / "rpc-clients"
         if name:
-            return [ RpcClient(rpc_clients / name) ]
-        return [ RpcClient(f) for f in sorted(rpc_clients.iterdir()) ]
+            return [RpcClient(rpc_clients / name)]
+        return [RpcClient(f) for f in sorted(rpc_clients.iterdir())]
 
     def show(args):
         for client in RpcClient.get_by_name(args.client):
@@ -244,9 +247,12 @@ class RpcClient:
 
 parser = argparse.ArgumentParser()
 
+
 def show_small_help(args):
     parser.print_usage()
     print("sunrpc dir:", sunrpc)
+
+
 parser.set_defaults(func=show_small_help)
 
 subparser = parser.add_subparsers(title="commands")
-- 
2.48.1

[PATCH nfs-utils v3 2/7] rpcctl: Fix flake8 line-too-long errors

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index 92a851c2278b..ec43d12afc41 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -66,13 +66,17 @@ class Xprt:
                f"Requests: {self.info['num_reqs']}"
 
     def _cong_slots(self):
-        return f"	Congestion: cur {self.info['cur_cong']}, win {self.info['cong_win']}, " \
-               f"Slots: min {self.info['min_num_slots']}, max {self.info['max_num_slots']}"
+        return f"	Congestion: cur {self.info['cur_cong']}, " \
+               f"win {self.info['cong_win']}, " \
+               f"Slots: min {self.info['min_num_slots']}, " \
+               f"max {self.info['max_num_slots']}"
 
     def _queues(self):
         return f"	Queues: binding {self.info['binding_q_len']}, " \
-               f"sending {self.info['sending_q_len']}, pending {self.info['pending_q_len']}, " \
-               f"backlog {self.info['backlog_q_len']}, tasks {self.info['tasks_queuelen']}"
+               f"sending {self.info['sending_q_len']}, " \
+               f"pending {self.info['pending_q_len']}, " \
+               f"backlog {self.info['backlog_q_len']}, " \
+               f"tasks {self.info['tasks_queuelen']}"
 
     def __str__(self):
         if not self.path.exists():
@@ -106,7 +110,8 @@ class Xprt:
         self.set_state("remove")
 
     def add_command(subparser):
-        parser = subparser.add_parser("xprt", help="Commands for individual xprts")
+        parser = subparser.add_parser("xprt",
+                                      help="Commands for individual xprts")
         parser.set_defaults(func=Xprt.show, xprt=None)
         subparser = parser.add_subparsers()
 
@@ -128,7 +133,8 @@ class Xprt:
         online.set_defaults(func=Xprt.set_property, property="online")
         offline = subparser.add_parser("offline", help="Set an xprt offline")
         offline.set_defaults(func=Xprt.set_property, property="offline")
-        dstaddr = subparser.add_parser("dstaddr", help="Change an xprt's dstaddr")
+        dstaddr = subparser.add_parser("dstaddr",
+                                       help="Change an xprt's dstaddr")
         dstaddr.add_argument("newaddr", metavar="NEWADDR", nargs=1,
                              help="The new address for the xprt")
         dstaddr.set_defaults(func=Xprt.set_property, property="dstaddr")
@@ -161,7 +167,8 @@ class XprtSwitch:
         self.path = path
         self.name = path.stem
         self.info = read_info_file(path / "xprt_switch_info")
-        self.xprts = sorted([Xprt(p) for p in self.path.iterdir() if p.is_dir()])
+        self.xprts = sorted([Xprt(p) for p in self.path.iterdir()
+                             if p.is_dir()])
         self.sep = sep
 
     def __lt__(self, rhs):
@@ -176,7 +183,8 @@ class XprtSwitch:
         return "\n".join([switch] + xprts)
 
     def add_command(subparser):
-        parser = subparser.add_parser("switch", help="Commands for xprt switches")
+        parser = subparser.add_parser("switch",
+                                      help="Commands for xprt switches")
         parser.set_defaults(func=XprtSwitch.show, switch=None)
         subparser = parser.add_subparsers()
 
@@ -185,11 +193,13 @@ class XprtSwitch:
                           help="Name of a specific switch to show")
         show.set_defaults(func=XprtSwitch.show)
 
-        set = subparser.add_parser("set", help="Change an xprt switch property")
+        set = subparser.add_parser("set",
+                                   help="Change an xprt switch property")
         set.add_argument("switch", metavar="SWITCH", nargs=1,
                          help="Name of a specific xprt switch to modify")
         subparser = set.add_subparsers(required=True)
-        dstaddr = subparser.add_parser("dstaddr", help="Change an xprt switch's dstaddr")
+        dstaddr = subparser.add_parser("dstaddr",
+                                       help="Change an xprt switch's dstaddr")
         dstaddr.add_argument("newaddr", metavar="NEWADDR", nargs=1,
                              help="The new address for the xprt switch")
         dstaddr.set_defaults(func=XprtSwitch.set_property, property="dstaddr")
@@ -225,7 +235,8 @@ class RpcClient:
         return f"{self.name}: {self.switch}"
 
     def add_command(subparser):
-        parser = subparser.add_parser("client", help="Commands for rpc clients")
+        parser = subparser.add_parser("client",
+                                      help="Commands for rpc clients")
         parser.set_defaults(func=RpcClient.show, client=None)
         subparser = parser.add_subparsers()
 
-- 
2.48.1

[PATCH nfs-utils v3 3/7] rpcctl: Fix flake8 bare exception error

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index ec43d12afc41..c6e73aad8bb9 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -23,7 +23,7 @@ def read_addr_file(path):
     try:
         with open(path, 'r') as f:
             return f.readline().strip()
-    except:
+    except FileNotFoundError:
         return "(enoent)"
 
 
-- 
2.48.1

[PATCH nfs-utils v3 4/7] rpcctl: Fix flake8 ambiguous-variable-name error

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index c6e73aad8bb9..435f4be6623a 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -37,7 +37,7 @@ def read_info_file(path):
     res = collections.defaultdict(int)
     try:
         with open(path) as info:
-            lines = [l.split("=", 1) for l in info if "=" in l]
+            lines = [line.split("=", 1) for line in info if "=" in line]
             res.update({key: int(val.strip()) for (key, val) in lines})
     finally:
         return res
-- 
2.48.1

[PATCH nfs-utils v3 5/7] rpcctl: Add missing docstrings to the Xprt class

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index 435f4be6623a..b8808787b51d 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -44,7 +44,10 @@ def read_info_file(path):
 
 
 class Xprt:
+    """Represents a single sunrpc connection."""
+
     def __init__(self, path):
+        """Read in xprt information from sysfs."""
         self.path = path
         self.name = path.stem.rsplit("-", 1)[0]
         self.type = path.stem.split("-")[2]
@@ -54,6 +57,7 @@ class Xprt:
         self.read_state()
 
     def __lt__(self, rhs):
+        """Compare the names of two xprt instances."""
         return self.name < rhs.name
 
     def _xprt(self):
@@ -79,24 +83,29 @@ class Xprt:
                f"tasks {self.info['tasks_queuelen']}"
 
     def __str__(self):
+        """Return a string representation of an xprt."""
         if not self.path.exists():
             return f"{self.name}: has been removed"
         return "\n".join([self._xprt(), self._src_reqs(),
                           self._cong_slots(), self._queues()])
 
     def read_state(self):
+        """Read in the xprt_state file."""
         if self.path.exists():
             with open(self.path / "xprt_state") as f:
                 self.state = ','.join(f.readline().split()[1:])
 
     def small_str(self):
+        """Return a short string summary of an xprt."""
         main = " [main]" if self.info.get("main_xprt") else ""
         return f"{self.name}: {self.type}, {self.dstaddr}{main}"
 
     def set_dstaddr(self, newaddr):
+        """Change the dstaddr of an xprt."""
         self.dstaddr = write_addr_file(self.path / "dstaddr", newaddr)
 
     def set_state(self, state):
+        """Change the state of an xprt."""
         if self.info.get("main_xprt"):
             raise Exception(f"Main xprts cannot be set {state}")
         with open(self.path / "xprt_state", 'w') as f:
@@ -104,12 +113,14 @@ class Xprt:
         self.read_state()
 
     def remove(self):
+        """Remove an xprt."""
         if self.info.get("main_xprt"):
             raise Exception("Main xprts cannot be removed")
         self.set_state("offline")
         self.set_state("remove")
 
     def add_command(subparser):
+        """Add parser options for the `rpcctl xprt` command."""
         parser = subparser.add_parser("xprt",
                                       help="Commands for individual xprts")
         parser.set_defaults(func=Xprt.show, xprt=None)
@@ -140,6 +151,7 @@ class Xprt:
         dstaddr.set_defaults(func=Xprt.set_property, property="dstaddr")
 
     def get_by_name(name):
+        """Find a (sorted) list of Xprts matching the given name."""
         glob = f"**/{name}-*" if name else "**/xprt-*"
         res = [Xprt(x) for x in (sunrpc / "xprt-switches").glob(glob)]
         if name and len(res) == 0:
@@ -148,10 +160,12 @@ class Xprt:
         return sorted(res)
 
     def show(args):
+        """Handle the `rpcctl xprt show` command."""
         for xprt in Xprt.get_by_name(args.xprt):
             print(xprt)
 
     def set_property(args):
+        """Handle the `rpcctl xprt set` command."""
         for xprt in Xprt.get_by_name(args.xprt[0]):
             if args.property == "dstaddr":
                 xprt.set_dstaddr(socket.gethostbyname(args.newaddr[0]))
-- 
2.48.1

[PATCH nfs-utils v3 6/7] rpcctl: Add missing docstrings to the XprtSwitch class

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index b8808787b51d..adeb26d51f0e 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -177,7 +177,10 @@ class Xprt:
 
 
 class XprtSwitch:
+    """Represents a group of xprt connections."""
+
     def __init__(self, path, sep=":"):
+        """Read in xprt switch information from sysfs."""
         self.path = path
         self.name = path.stem
         self.info = read_info_file(path / "xprt_switch_info")
@@ -186,9 +189,11 @@ class XprtSwitch:
         self.sep = sep
 
     def __lt__(self, rhs):
+        """Compare the name of two xprt switch instances."""
         return self.name < rhs.name
 
     def __str__(self):
+        """Return a string representation of an xprt switch."""
         switch = f"{self.name}{self.sep} " \
                  f"xprts {self.info['num_xprts']}, " \
                  f"active {self.info['num_active']}, " \
@@ -197,6 +202,7 @@ class XprtSwitch:
         return "\n".join([switch] + xprts)
 
     def add_command(subparser):
+        """Add parser options for the `rpcctl switch` command."""
         parser = subparser.add_parser("switch",
                                       help="Commands for xprt switches")
         parser.set_defaults(func=XprtSwitch.show, switch=None)
@@ -219,16 +225,19 @@ class XprtSwitch:
         dstaddr.set_defaults(func=XprtSwitch.set_property, property="dstaddr")
 
     def get_by_name(name):
+        """Find a (sorted) list of XprtSwitches matching the given name."""
         xprt_switches = sunrpc / "xprt-switches"
         if name:
             return [XprtSwitch(xprt_switches / name)]
         return [XprtSwitch(f) for f in sorted(xprt_switches.iterdir())]
 
     def show(args):
+        """Handle the `rpcctl switch show` command."""
         for switch in XprtSwitch.get_by_name(args.switch):
             print(switch)
 
     def set_property(args):
+        """Handle the `rpcctl switch set` command."""
         for switch in XprtSwitch.get_by_name(args.switch[0]):
             resolved = socket.gethostbyname(args.newaddr[0])
             for xprt in switch.xprts:
-- 
2.48.1

[PATCH nfs-utils v3 7/7] rpcctl: Add remaining missing docstrings

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@oracle.com>

Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
---
 tools/rpcctl/rpcctl.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/rpcctl/rpcctl.py b/tools/rpcctl/rpcctl.py
index adeb26d51f0e..0221fbb68be1 100755
--- a/tools/rpcctl/rpcctl.py
+++ b/tools/rpcctl/rpcctl.py
@@ -1,4 +1,5 @@
 #!/usr/bin/python3
+"""Utility for working with the sunrpc sysfs files."""
 import argparse
 import collections
 import errno
@@ -20,6 +21,7 @@ if not sunrpc.is_dir():
 
 
 def read_addr_file(path):
+    """Read an xprt address file."""
     try:
         with open(path, 'r') as f:
             return f.readline().strip()
@@ -28,12 +30,14 @@ def read_addr_file(path):
 
 
 def write_addr_file(path, newaddr):
+    """Write a new address to an xprt address file."""
     with open(path, 'w') as f:
         f.write(newaddr)
     return read_addr_file(path)
 
 
 def read_info_file(path):
+    """Read an xprt or xprt switch information file."""
     res = collections.defaultdict(int)
     try:
         with open(path) as info:
@@ -246,18 +250,24 @@ class XprtSwitch:
 
 
 class RpcClient:
+    """Represents an rpc client instance."""
+
     def __init__(self, path):
+        """Read in rpc client information from sysfs."""
         self.path = path
         self.name = path.stem
         self.switch = XprtSwitch(path / (path / "switch").readlink(), sep=",")
 
     def __lt__(self, rhs):
+        """Compare the name of two rpc client instances."""
         return self.name < rhs.name
 
     def __str__(self):
+        """Return a string representation of an rpc client."""
         return f"{self.name}: {self.switch}"
 
     def add_command(subparser):
+        """Add parser options for the `rpcctl client` command."""
         parser = subparser.add_parser("client",
                                       help="Commands for rpc clients")
         parser.set_defaults(func=RpcClient.show, client=None)
@@ -269,12 +279,14 @@ class RpcClient:
         parser.set_defaults(func=RpcClient.show)
 
     def get_by_name(name):
+        """Find a (sorted) list of RpcClients matching the given name."""
         rpc_clients = sunrpc / "rpc-clients"
         if name:
             return [RpcClient(rpc_clients / name)]
         return [RpcClient(f) for f in sorted(rpc_clients.iterdir())]
 
     def show(args):
+        """Handle the `rpcctl client show` command."""
         for client in RpcClient.get_by_name(args.client):
             print(client)
 
@@ -283,6 +295,7 @@ parser = argparse.ArgumentParser()
 
 
 def show_small_help(args):
+    """Show a one-line usage summary if no subcommand is given."""
     parser.print_usage()
     print("sunrpc dir:", sunrpc)
 
-- 
2.48.1

Re: [PATCH nfs-utils v3 0/7] rpcctl: Flake8 cleanups

[Steve Dickson]

On 1/15/25 3:29 PM, Anna Schumaker wrote:
> From: Anna Schumaker <anna.schumaker@oracle.com>
> 
> Apologies for the noise. I just realized I gave the wrong range to `git
> format-patch`, and accidentally resent somebody else's patch that has
> already been merged.
> 
> This is a series of cleanups for rpcctl.py to fix up various style
> issues after running `flake8` on the code.
> 
> Thoughts?
> Anna
> 
> 
> Anna Schumaker (7):
>    rpcctl: Fix flake8 whitespace errors
>    rpcctl: Fix flake8 line-too-long errors
>    rpcctl: Fix flake8 bare exception error
>    rpcctl: Fix flake8 ambiguous-variable-name error
>    rpcctl: Add missing docstrings to the Xprt class
>    rpcctl: Add missing docstrings to the XprtSwitch class
>    rpcctl: Add remaining missing docstrings
> 
>   tools/rpcctl/rpcctl.py | 107 ++++++++++++++++++++++++++++++-----------
>   1 file changed, 80 insertions(+), 27 deletions(-)
> 
Committed... (tag: nfs-utils-2-8-3-rc4)

steved.