On Thu, Mar 17, 2011 at 05:17:39PM +0100, Michael Hanselmann wrote:
> Am 17. März 2011 16:09 schrieb Iustin Pop <[email protected]>:
> > On Thu, Mar 17, 2011 at 01:40:09PM +0100, Michael Hanselmann wrote:
> >> --- a/lib/watcher/__init__.py
> >> +++ b/lib/watcher/__init__.py
> >> @@ -693,13 +693,17 @@ def ParseOptions():
> >>
> >> parser.add_option(cli.DEBUG_OPT)
> >> parser.add_option("-A", "--job-age", dest="job_age",
> >> - help="Autoarchive jobs older than this age (default"
> >> - " 6 hours)", default=6*3600)
> >> + help=("Autoarchive jobs older than this age (default"
> >> + " 6 hours)"), default=6 * 3600)
> >
> > We don't use this, and it doesn't help with the layout. Please drop.
>
> Not quite true:
>
> $ git grep -B1 opts.job_age lib/watcher/__init__.py
> lib/watcher/__init__.py- # first archive old jobs
> lib/watcher/__init__.py: self.ArchiveJobs(opts.job_age)
Confused. I'm referring to ( " " \n " " ) inside the args.
> >> - return options, args
> >> +
> >> + if args:
> >> + parser.error("No arguments expected")
> >> +
> >> + return options
> >
> > While raising this here is good, it breaks the rule that our
> > ParseOptions functions always return (option, args).
>
> Also not quite true:
'quite' is the key word :)
> daemons/import-export: (status_file_path, mode) = ParseOptions()
> tools/burnin: self.ParseOptions()
> tools/ganeti-listrunner: password, batch_size) = ParseOptions()
> tools/move-instance: (parser, options, args) = ParseOptions()
>
> Given that the arguments are of no use in watcher, do you really want
> to return them?
I would prefer to say that all our command line parsing functions
have the same return type, for consistency. It's not important, but I
think it would be helpful.
thanks,
iustin
