[ovs-dev] [bug7533 1/2] ovs.daemon: Fix semantics of --pidfile option.

[Ethan Jackson]

Looks good,

Ethan

On Wed, Sep 28, 2011 at 23:11, Ben Pfaff <blp at nicira.com> wrote:
> The --pidfile option is supposed to work like this:
>
>   * Without --pidfile, you don't get a pidfile.
>   * With --pidfile, you get the default pidfile.
>   * With --pidfile=FILE, you get FILE as your pidfile.
>
> However, it actually worked like this:
>
>   * Without --pidfile, you got the default pidfile.
>   * With --pidfile, you got no pidfile at all.
>   * With --pidfile=FILE, you got FILE as your pidfile.
>
> This is because of the semantics of "default" in argparse.  It is
> documented as:
>
>    The default keyword argument of add_argument(), whose value defaults to
>    None, specifies what value should be used if the command-line argument
>    is not present.  For optional arguments, the default value is used when
>    the option string was not present at the command line.
>
> We actually want "const", which is documented under the description of
> nargs="?" as:
>
>    If no command-line argument is present, the value from default will be
>    produced.  Note that for optional arguments, there is an additional
>    case - the option string is present but not followed by a command-line
>    argument.  In this case the value from const will be produced.
>
> Bug #7533.
> ---
>  python/ovs/daemon.py |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py
> index 85db050..a919c53 100644
> --- a/python/ovs/daemon.py
> +++ b/python/ovs/daemon.py
> @@ -503,7 +503,7 @@ def add_args(parser):
>             help="Do not chdir to '/'.")
>     group.add_argument("--monitor", action="store_true",
>             help="Monitor %s process." % ovs.util.PROGRAM_NAME)
> -    group.add_argument("--pidfile", nargs="?", default=pidfile,
> +    group.add_argument("--pidfile", nargs="?", const=pidfile,
>             help="Create pidfile (default %s)." % pidfile)
>     group.add_argument("--overwrite-pidfile", action="store_true",
>             help="With --pidfile, start even if already running.")
> --
> 1.7.2.5
>
>