launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/local-latency-port into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #821482 in Launchpad itself: "local-latency script should permit setting port"
  https://bugs.launchpad.net/launchpad/+bug/821482

For more details, see:
https://code.launchpad.net/~abentley/launchpad/local-latency-port/+merge/70572

= Summary =
Add a --port option to the local-latency script

== Proposed fix ==
See above

== Pre-implementation notes ==
None

== Implementation details ==
Revised the scripting framework to make implementation simpler. Now commands are just functions with decoration, and option defaults are set from parameter defaults. No special OptionParser needed.

== Tests ==
None

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  utilities/local-latency
  utilities/script_commands.py
-- 
https://code.launchpad.net/~abentley/launchpad/local-latency-port/+merge/70572
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/local-latency-port into lp:launchpad.

=== modified file 'utilities/local-latency'
--- utilities/local-latency	2011-08-04 15:50:52 +0000
+++ utilities/local-latency	2011-08-05 14:06:46 +0000
@@ -6,9 +6,10 @@
 import subprocess
 
 from script_commands import (
-    Command,
-    OptionParser,
+    helps,
+    run_subcommand,
     UserError,
+    types,
     )
 
 
@@ -16,47 +17,31 @@
      subprocess.call('sudo tc ' + command, shell=True)
 
 
-class StartCommand(Command):
-
-    @classmethod
-    def get_parser(cls):
-        parser = OptionParser()
-        parser.add_option(
-            '-d', '--delay', dest='delay', type='int',
-            help='Length of delay in miliseconds (each way).')
-        return parser
-
-    @staticmethod
-    def run(delay=500, port=443):
-        tc('qdisc add dev lo root handle 1: prio')
-        tc('qdisc add dev lo parent 1:3 handle 30: netem delay %dms' % delay)
-        tc('filter add dev lo protocol ip parent 1:0 prio 3 u32 match ip'
-           ' dport %d 0xffff flowid 1:3' % port)
-        tc('filter add dev lo protocol ip parent 1:0 prio 3 u32 match ip'
-           ' sport %d 0xffff flowid 1:3' % port)
-
-
-Command.commands['start'] = StartCommand
-
-
-class StopCommand(Command):
-
-    @staticmethod
-    def get_parser():
-        parser = OptionParser()
-        return parser
-
-    @staticmethod
-    def run():
-        tc('qdisc del dev lo root')
-
-
-Command.commands['stop'] = StopCommand
+@types(delay='int', port='int')
+@helps(delay='Length of delay in miliseconds (each way).',
+       port='Port to induce delay on.')
+def start(delay=500, port=443):
+    tc('qdisc add dev lo root handle 1: prio')
+    tc('qdisc add dev lo parent 1:3 handle 30: netem delay %dms' % delay)
+    tc('filter add dev lo protocol ip parent 1:0 prio 3 u32 match ip'
+       ' dport %d 0xffff flowid 1:3' % port)
+    tc('filter add dev lo protocol ip parent 1:0 prio 3 u32 match ip'
+       ' sport %d 0xffff flowid 1:3' % port)
+
+
+def stop():
+    tc('qdisc del dev lo root')
+
+
+subcommands = {
+    'start': start,
+    'stop': stop,
+    }
 
 
 
 if __name__ == "__main__":
     try:
-        Command.run_subcommand(sys.argv[1:])
+        run_subcommand(subcommands, sys.argv[1:])
     except UserError as e:
         sys.stderr.write(str(e)+'\n')

=== modified file 'utilities/script_commands.py'
--- utilities/script_commands.py	2011-08-04 15:50:52 +0000
+++ utilities/script_commands.py	2011-08-05 14:06:46 +0000
@@ -1,56 +1,78 @@
-import optparse
+import inspect
+from optparse import OptionParser
 
 
 class UserError(Exception):
     pass
 
 
-class OptionParser(optparse.OptionParser):
-
-    UNSPECIFIED = object()
-
-    def add_option(self, *args, **kwargs):
-        """Add an option, with the default that it not be included."""
-        kwargs['default'] = kwargs.get('default', self.UNSPECIFIED)
-        optparse.OptionParser.add_option(self, *args, **kwargs)
-
-    def parse_args_dict(self, cmd_args):
-        """Return a dict of specified options.
-
-        Unspecified options with no explicit default are not included in the
-        dict."""
-        options, args = self.parse_args(cmd_args)
-        option_dict = dict(
-            item for item in options.__dict__.items()
-            if item[1] is not self.UNSPECIFIED)
-        return args, option_dict
-
-
-class Command:
-    """Base class for subcommands."""
-
-    commands = {}
-
-    @classmethod
-    def parse_args(cls, args):
-        if len(args) != 0:
-            raise UserError('Too many arguments.')
-        return {}
-
-    @classmethod
-    def run_from_args(cls, cmd_args):
-        args, kwargs = cls.get_parser().parse_args_dict(cmd_args)
-        kwargs.update(cls.parse_args(args))
-        cls.run(**kwargs)
-
-    @classmethod
-    def run_subcommand(cls, argv):
-        if len(argv) < 1:
-            raise UserError('Must supply a command: %s.' %
-                            ', '.join(cls.commands.keys()))
-        try:
-            command = cls.commands[argv[0]]
-        except KeyError:
-            raise UserError('%s invalid.  Valid commands: %s.' %
-                            (argv[0], ', '.join(cls.commands.keys())))
-        command.run_from_args(argv[1:])
+def add_dict(name, **kwargs):
+    def decorator(func):
+        setattr(func, name, kwargs)
+        return func
+    return decorator
+
+
+def types(**kwargs):
+    return add_dict('_types', **kwargs)
+
+
+def helps(**kwargs):
+    return add_dict('_helps', **kwargs)
+
+
+def get_function_parser(function):
+    """Generate an OptionParser for a function.
+
+    Defaults come from the parameter defaults.
+    For every permitted to provide as an option, the type must be specified,
+    using the types decorator.
+    Help may be specified using the helps decorator.
+    """
+    parser = OptionParser()
+    args, ignore, ignored, defaults = inspect.getargspec(function)
+    for arg in args:
+        arg_type = function._types.get(arg)
+        if arg_type is None:
+            continue
+        arg_help = getattr(function, '_helps', {}).get(arg)
+        if arg_help is not None:
+            arg_help += ' Default: %default.'
+        parser.add_option('--%s' % arg, type=arg_type, help=arg_help)
+    if defaults is not None:
+        defaults_dict = dict(zip(args, defaults))
+        option_defaults = dict(
+            (key, value) for key, value in defaults_dict.items()
+            if parser.defaults.get(key, '') is None)
+        parser.set_defaults(**option_defaults)
+    return parser
+
+
+def parse_args(command, args):
+    """Return the positional arguments as a dict."""
+    # TODO: implement!
+    if len(args) != 0:
+        raise UserError('Too many arguments.')
+    return {}
+
+
+def run_from_args(command, cmd_args):
+    """Run a command function using the specified commandline arguments."""
+    parser = get_function_parser(command)
+    options, args = parser.parse_args(cmd_args)
+    kwargs = parse_args(command, args)
+    kwargs.update(options.__dict__)
+    command(**kwargs)
+
+
+def run_subcommand(subcommands, argv):
+    """Run a subcommand as specified by argv."""
+    if len(argv) < 1:
+        raise UserError('Must supply a command: %s.' %
+                        ', '.join(subcommands.keys()))
+    try:
+        command = subcommands[argv[0]]
+    except KeyError:
+        raise UserError('%s invalid.  Valid commands: %s.' %
+                        (argv[0], ', '.join(subcommands.keys())))
+    run_from_args(command, argv[1:])