[Avocado-devel] RFC: Configuration by convention

[Beraldo Leal]

Hi Lukáš,

My comments are below. If I didn't reply to a specific topic is because
I'm waiting for more replies to take into consideration.

On Mon, Dec 02, 2019 at 04:57:28PM +0100, Lukáš Doktor wrote:
> Dne 02. 12. 19 v 15:30 Beraldo Leal napsal(a):
> > On Thu, Nov 28, 2019 at 04:00:17PM +0100, Lukáš Doktor wrote:
> >> Dne 21. 11. 19 v 22:23 Beraldo Leal napsal(a):
> >>> Motivation
> >>> ##########
> >>>
> >>> An Avocado Job is primarily executed through the `avocado run` command line.
> >>> The behavior of such an Avocado Job is determined by parsing the following
> >>> settings (listed in parsed order):
> >>>
> >>>  1) Default values in source code
> >>>  2) Configuration file contents
> >>>  3) Command-line options
> >>>
> >>
> >> I'm missing in this RFC some kind of mapping the above. I think if we
> >> are to do those intrusive changes, we should spend some time on
> >> specifying the relations. (but maybe I just missed it)
> > 
> > Not sure if I understood. What kind of relations do you suggest? Maybe
> > now, I'm missing something. The idea was to simply show the order that
> > options are parsed and overridden, just to give some context.
> > 
> 
> While talking about standardization of args/conf options I though it'd
> be nice to define some standard mapping to override configuration
> options from cmdline arguments. What I mean is eg the `--sysinfo`.
> Obviously it whouldn't fit everything, but would be nice to know that:
> 
> --sysinfo $VALUE
> 
> is the same as
> 
> ```
> [sysinfo.collect]
> enabled = $VALUE
> ```
> 
> and maps to:
> 
> {sysinfo: $VALUE}
> 
> job param. Obviously there would be exceptions, but we could create
> guidelines to cover most cases. Additionally even discussing this
> might help identify the sections/arguments.

Yes, I see and agree with you. The idea was to add this in a
"Configuration Reference" section in one of our guides. That was what I
tried to explain in the "How to Teach This".

> The default comes from /etc. You can checkout the `avocado/plugins/run.py` for details:
> 
>         sysinfo_default = settings.get_value('sysinfo.collect',
>                                              'enabled',
>                                              key_type='bool',
>                                              default=True)
>         sysinfo_default = 'on' if sysinfo_default is True else 'off'
>         parser.add_argument('--sysinfo', choices=('on', 'off'),
>                             default=sysinfo_default, help="Enable or disable "
>                             "system information (hardware details, profilers, "
>                             "etc.). Current:  %(default)s")
> 
> 1. `--sysinfo on` => force-enable
> 2. `--sysinfo off` => force-disable
> 3. no `--sysinfo` => use the value from /etc

IIUC, the default comes from the source code:

  settings.get_value(..., default=True)

If the option is missing on config file and there is no --sysinfo option
on command line, the default will be "True", right?

So, there are 3 ways to use, but the possible values are "on" or "off",
right? Fix-me if I'm wrong, but what is the third option other than,
"on" and "off"? The key_type is still a bool, right?

The default option, IMO, should be in source code, not /etc. In that
way, the user can have a minimum config file or an empty one. And I
think that is what is implemented, today.

> >>     avocado run --loaders LOADERS [LOADERS ...]
> >>
> >> is that acceptable?
> > 
> > Yes, and I liked this one, it is better than my suggestions.
> > 
> 
> Great, thanks and +1 to this one. Also note there is one caveat, it
> must not be used for main arguments, otherwise you won't be able to
> specify the `subcommand`. This is why the `--show` accepts only a
> single argument and uses custom split (`,`). Something like:
> 
>     avocado --show foo bar baz run passtest.py
> 
> is not possible, because the argparse can't decide which is the
> subcommand and which is the `--show` argument. For subcommand options
> it is possible, because one can use `--` to indicate end of the
> options:
> 
>     avocado run --loaders foo bar -- passtest.py
> 
> or simply add another option that consumes given amount of arguments
> (but it is hackish :-) ):
> 
>     avocado run --loaders foo bar --sysinfo on passtest.py

Noted and I will take this in consideration. Thanks.

> >>> Presentation
> >>> ------------
> >>>
> >>> As the avocado trend is to have more and more plugins, I believe that to make
> >>> it easier for the user to find where each configuration is, we should split the
> >>> file into smaller files, leaving one file for each plugin. Avocado already
> >>> supports that with the conf.d directory. What do you think?
> >>>
> >>
> >> I'd go with core+core-plugins and plugins, therefor basically the
> >> current situation. I don't think we need to extract the core-plugins
> >> from the core-configuration (talking about the essential set of
> >> plugins like "run").
> > 
> > Not sure if I understood your "formula". Could you please provide an
> > example? And what it is considered "core", in your option?
> > 
> 
> core-plugins are all plugins that are always installed (therefor the
> `avocado.plugins` namespace). So basically we'd only create separate
> files for `optional_plugins` (yaml_loader) or the plugins that live
> outside of our repository (eg. avocado-vt).

I see. Let's wait for more comments on this.

> > I'm sorry it was not clear. I must have expressed myself wrong. This is
> > not about the Job API yet. Just a preparation for that.
> > 
> > This RFC is about a convention for our configuration options:
> > 
> >   1. "implicit" (default values);
> >   2. "more permanent" (config files);
> >   3. "temporary/runtime" (argument options)
> > 
> > This section is about how to teach this new convention to new users. I
> > could not find a detailed configuration reference on our docs, with all
> > default options, for instance.
> > 
> 
> For me the important part is to have the correct defaults in `--help`
> and list of all setting in `avocado config`. As for the documentation,
> perhaps we can generate the `avocado config` output on readthedocs.io,
> if you think it's important to keep them somewhere, but the runtime
> options are more important to me.

We are on the same page here.

Regards,
Beraldo