| Message ID | 1415206872-864-6-git-send-email-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Nov 5, 2014, at 12:01 PM, Scott Mayhew <smayhew@redhat.com> 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? > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > --- > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 05 Nov 2014, Chuck Lever wrote: > > On Nov 5, 2014, at 12:01 PM, Scott Mayhew <smayhew@redhat.com> 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). > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > > --- > > 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Nov 6, 2014, at 9:44 AM, Scott Mayhew <smayhew@redhat.com> wrote: > On Wed, 05 Nov 2014, Chuck Lever wrote: > >> >> On Nov 5, 2014, at 12:01 PM, Scott Mayhew <smayhew@redhat.com> 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@redhat.com> >>> --- >>> 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@vger.kernel.org >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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:
Signed-off-by: Scott Mayhew <smayhew@redhat.com> --- tools/mountstats/mountstats.py | 62 +++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 31 deletions(-)