On Nov 6, 2014, at 9:44 AM, Scott Mayhew <smayhew@xxxxxxxxxx> wrote: > On Wed, 05 Nov 2014, Chuck Lever wrote: > >> >> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <smayhew@xxxxxxxxxx> wrote: >> >> I?m not clear why you need to replace this code. But if >> you really do need to replace it, why not use the more >> Python-esque argparse module, which would replace both >> the option parsing and help text? > > This started out as a hacked up version of the mountstats program that I > was really planning on sending anywhere, and originally I had only done > the nfsstat options using getopt. When I redid the changes in git I > decided to convert the rest of them to use getopt as well. I can change > it back to the original parsing or I can use argparse. Let me know > which you prefer (I have no preference either way). I wrote mountstats almost 10 years ago, and Python has changed a lot since then. It makes sense to modernize this a bit. I think argparse: + is more idiomatic Python + allows you to fix up the help text and attach it directly to each option + maybe makes it easier to manage the coexistence of the different commands in the same source code? Give it a try. Here’s some sample code: http://git.linux-nfs.org/?p=cel/fedfs-utils.git;a=blob;f=src/domainroot/fedfs-domainroot.in;h=6db32eb0fdb47b90e25189929ec9588dbf342eda;hb=HEAD If it starts looking crazy, then stick with getopt. >> >>> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx> >>> --- >>> tools/mountstats/mountstats.py | 62 +++++++++++++++++++++--------------------- >>> 1 file changed, 31 insertions(+), 31 deletions(-) >>> >>> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py >>> index 2fe1362..272a8f9 100755 >>> --- a/tools/mountstats/mountstats.py >>> +++ b/tools/mountstats/mountstats.py >>> @@ -24,6 +24,7 @@ MA 02110-1301 USA >>> """ >>> >>> import sys, os, time >>> +import getopt >>> >>> Mountstats_version = '0.2' >>> >>> @@ -547,37 +548,33 @@ def print_mountstats_help(name): >>> def mountstats_command(): >>> """Mountstats command >>> """ >>> + try: >>> + opts, args = getopt.getopt(sys.argv[1:], "ehnrsv", ["end", "help", "nfs", "rpc", "start", "version"]) >>> + except getopt.GetoptError as err: >>> + print_mountstats_help(prog) >>> + >>> mountpoints = [] >>> nfs_only = False >>> rpc_only = False >>> >>> - for arg in sys.argv: >>> - if arg in ['-h', '--help', 'help', 'usage']: >>> + for o, a in opts: >>> + if o in ("-e", "--end"): >>> + raise Exception('Sampling is not yet implemented') >>> + elif o in ("-h", "--help"): >>> print_mountstats_help(prog) >>> - return >>> - >>> - if arg in ['-v', '--version', 'version']: >>> - print('%s version %s' % (sys.argv[0], Mountstats_version)) >>> sys.exit(0) >>> - >>> - if arg in ['-n', '--nfs']: >>> + elif o in ("-n", "--nfs"): >>> nfs_only = True >>> - continue >>> - >>> - if arg in ['-r', '--rpc']: >>> + elif o in ("-r", "--rpc"): >>> rpc_only = True >>> - continue >>> - >>> - if arg in ['-s', '--start']: >>> - raise Exception('Sampling is not yet implemented') >>> - >>> - if arg in ['-e', '--end']: >>> + elif o in ("-s", "--start"): >>> raise Exception('Sampling is not yet implemented') >>> - >>> - if arg == sys.argv[0]: >>> - continue >>> - >>> - mountpoints += [arg] >>> + elif o in ("-v", "--version"): >>> + print('%s version %s' % (sys.argv[0], Mountstats_version)) >>> + sys.exit(0) >>> + else: >>> + assert False, "unhandled option" >>> + mountpoints += args >>> >>> if mountpoints == []: >>> print_mountstats_help(prog) >>> @@ -662,23 +659,26 @@ def print_iostat_summary(old, new, devices, time): >>> def iostat_command(): >>> """iostat-like command for NFS mount points >>> """ >>> + try: >>> + opts, args = getopt.getopt(sys.argv[1:], "hv", ["help", "version"]) >>> + except getopt.GetoptError as err: >>> + print_iostat_help(prog) >>> mountstats = parse_stats_file('/proc/self/mountstats') >>> devices = [] >>> interval_seen = False >>> count_seen = False >>> >>> - for arg in sys.argv: >>> - if arg in ['-h', '--help', 'help', 'usage']: >>> + for o, a in opts: >>> + if o in ("-h", "--help"): >>> print_iostat_help(prog) >>> - return >>> - >>> - if arg in ['-v', '--version', 'version']: >>> + sys.exit(0) >>> + elif o in ("-v", "--version"): >>> print('%s version %s' % (sys.argv[0], Mountstats_version)) >>> - return >>> - >>> - if arg == sys.argv[0]: >>> - continue >>> + sys.exit(0) >>> + else: >>> + assert False, "unhandled option" >>> >>> + for arg in args: >>> if arg in mountstats: >>> devices += [arg] >>> elif not interval_seen: >>> -- >>> 1.9.3 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> Chuck Lever >> chuck[dot]lever[at]oracle[dot]com >> >> >> -- Chuck Lever chuck[dot]lever[at]oracle[dot]com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
