[Avocado-devel] RFC: Configuration by convention

Wed Dec 4 12:11:27 UTC 2019

On Mon, Dec 02, 2019 at 09:08:53PM -0500, Cleber Rosa wrote:
> On Thu, Nov 21, 2019 at 06:23:15PM -0300, Beraldo Leal wrote:
> > 
> >  1) Default values in source code
> 
> There's possibly a lack of convention/order in this item alone.  For
> instance, we have "avocado/core/defaults.py" with some defaults, but
> I'm sure there are other such defaults scattered around the project,
> with ad-hoc names.
> 
> A good starting point for setting a convention (say one of the cards
> you listed) would be to determine how to set the default on
> "avocado/core/defaults.py".  Also, another action item could be to
> make sure that we don't have "default worthy" variables set elsewhere.
> For instance, in "avocado/core/runner.py" I see:
> 
>    DEFAULT_TIMEOUT = 86400
> 
> Which is very similar to some of the default values in defaults.py.

Yes, I noticed that and I agree with you.

I replied to another email here in this thread suggesting to use default
values in source code, like a "config object". This is just an idea, an
example.

I have the feeling that we don't need a defaults.py. We could simplify
this by putting all the default values inside the parsing code within
the "get_value('foo', default='bar')" or similar. This will save us from
creating a new "variable pattern" just for defaults. The settings object
will be a mapping 1:1 from the "defaults + config file" (here I'm
thinking on our future Job API too). But again, this is just a feeling,
maybe defaults.py is the best way.

So, yes, we have an agreement here, regardless of where we are going to
save this I think that the most import it is to eliminate the
"distributed default values" on different parts of the source code. It
is just a detail on how we are going to implement it. We can discuss
where this will be saved now, or just postpone this discussion to the
specific issue after I split this RFC into small issues. What do you
think about it?

> > Besides the basic ones, here are some recommendations we should try to follow
> > and pay attention to:
> > 
> >   1. Option-arguments should not be optional (Guideline 7, from POSIX). So we
> >      should avoid this::
> >      
> >         avocado run --loaders [LOADERS [LOADERS ...]]
> > 
> >   or::
> >   
> >         avocado run --store-logging-stream [STREAM[:LEVEL] [STREAM[:LEVEL] ...]]
> > 
> >      We can have::
> > 
> >         avocado run --loaders LOADER,LOADER,LOADER,...
> > 
> >      or::
> > 
> >         avocado run --loader LOADER --loader LOADER --loader LOADER
> >
> 
> OK, I see and agree with the main point that a given option *should*
> be given.  Choosing the specific style would be the second issue
> then.  Again, maybe this can be another little card.  In such a card,
> besides its resolution, it would be nice to document what developers
> should follow (this may be the start of either a "Developer Guide"
> section or a "Plugin Developer Guide" itself.

Yes, I agree.

And trying to follow the same logic that I said on the previous
paragraph, I tried to collect all possible "conventions" and document
here to get an agreement. After this dicussion, I would suggest to have
this RFC as a big epic issue with a proper "blueprint/specification".
And then split the issue into small ones and attack/discuss them by
priority.

> >   2. Use hyphens not underscore: Long options consist of ‘--’ followed by a
> >      name made of alphanumeric characters and dashes. Option names are
> >      typically one to three words long, with hyphens to separate words. Users
> >      can abbreviate the option names as long as the abbreviations are unique.
> >      Also, underscore, sometimes it gets "eaten" by a terminal border and
> >      thus looks like space.
> >
> 
> Are you aware of any long options in Avocado that violates this?  Or
> is this just supposed to be part of the guidelines?  If so, it could
> be preserved in a "Developer Guide" like suggested above.

No, it is just part of the guideline, and should be documented. Agree.

> > Argument Types
> > ~~~~~~~~~~~~~~
> > 
> > Basic types, like strings and integers, are clear how to use. But here is a
> > list of what should expect when using other types:
> > 
> >   1. **Booleans**: Boolean options should be expressed as "flags" args (without
> >        the "option-argument"). Flags, when present, should represent a
> >        True/Active value.  This will reduce the command line size. We should
> >        avoid using this::
> > 
> >         avocado run --json-job-result {on,off}
> >
> 
> Let's suppose that the command line options take precedence over the
> built-in defaults and configuration file settings.  How would you say
> that the "json-job-result" feature should be disabled (taking
> precedence over what's defined in the built-in defaults and
> configuration file settings)?
> 
> For instance, autoconf scripts will usually have both a
> '--enable-$(feature)' and '--disable-$(feature)' options.  Is this
> what you're proposing?

Almost... I have a feeling that we don't need both options here in
Avocado. If some option it is disabled by default, we just need one
option: "enable".

But you have debug=False in settings file. In that way, if you comment
this line or remove it, the default it is still disabled.

I like to think as the "verbose" and "debug" options on most of
command-line programs: they don't have a "disable-verbose" or
"disable-debug" if the default it is already disabled.

> > We can arrange the display of these options in alphabetical order within each
> > section.
> >
> 
> I guess you mean that we should/could... which I agree.  But I also
> wonder how.

Yes, my mistake. Thinking on "how", I just realized that we have options
on plugins! Aha! Yes, this might not be as easy as I assumed, but we
could think about it on the specific card/issue.

> > Plugin section name
> > ~~~~~~~~~~~~~~~~~~~
> > 
> > I am not quite sure here and would like to know the opinion of those who are
> > the longest in the project. Perhaps this is a little controversial point. But I
> > believe we can touch here to improve our convention.
> > 
> > Most plugins currently have the same name as the python module. Example: human,
> > diff, tap, nrun, run, journal, replay, sysinfo, etc.
> > 
> > These are examples of "good" names.
> > 
> > However, some other plugins do not follow this convention. Ex: runnable_run,
> > runnable_run_recipe, task_run, task_run_recipe, archive, etc.
> >
> 
> Are you also proposing that every plugin should de implemented in
> separate Python module?  IMO it would improve consistency indeed, but
> could add an overhead and increase the amount of boilerplate code
> repetion.
> 
> One recent example is the "avocado/plugins/resolvers.py" file, which
> initially was a file per-plugin.  The reviewer noticed the amount of
> duplicate imports and that the plugins were very similar in purpose
> and behavior.  In that ocasion, it make sense to me to follow the
> reviewer's point.
> 
> And adding to that point and your comment, the various Python modules
> containing plugins related to the nrunner, "runnable_run",
> "runnable_run_recipe", etc, could easily be on the very same
> "avocado/plugins/nrunner.py" module.

Well, I was thinking only about renaming the plugins, but not thinking
about this particular case, thanks for pointing me to this.  I
understand that it is not a simple problem to solve and involves a few
changes. One suggestion should be to have a namespace like:

resolvers.tests.exec, resolvers.tests.unit.python.

And all the duplicated code could be imported from a common module
inside the plugin. But yes, it is a "delicate issue", I probably this
suggestion has some pitfalls.

But if we have an agreement on doing this and it is a "not so easy" task
we could create a "Things to think soon" section in the blueprint and
start discussing the "how" on a specific thread/card.

> > Config Types
> > ~~~~~~~~~~~~
> > 
> > `configparser` do not guess datatypes of values in configuration files, always
> > storing them internally as strings. This means that if you need other
> > datatypes, you should convert on your own
> > 
> > There are few methods on this library to help us: `getboolean()`, `getint()`
> > and `getfloat()`. Basic types here, are also straightforward.
> > 
> > Regarding boolean values, `getboolean()` can accept `yes/no`, `on/off`,
> > `true/false` or `1/0`. But we should adopt one style and stick with it. I
> > would suggest using `true/false`.
> >
> 
> You talk about configparser only.  What about similar values given on
> the command line?  Should we have a common utility library that can
> check/return the right data types and be used on both configuration
> file and command line parser?
> One example is that
> https://docs.python.org/3/library/argparse.html#type will take any
> callable, and we may be tempted to use "bool" on a value here and then
> getboolean() with configparser, resulting in very different behavior.

On the begging of this RFC I talked about the same section, "Argument
Types", where I discuss the bool option there.

I think that yes, we could get advantages of the both configparse and
argparse options to deal with that, and try to create a common standard.

> Finally, let me say that setting a simple but effetive type system is
> not an easy task, but it's indeed a necessary step here.

Yes, I agree.

> > 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?
> > 
> >
> 
> If it reinforces a convention, so that users will automatically think
> of looking at a given file for a specific configuration, then I think
> this is fine.
> 
> But, I have to say that I think the final parsed content on the config
> file and the structure of that content is much more important.  If I
> would focus on something, I'd focus on the content structure (section
> names, key names, etc), and not necessarily on how the file names
> (given that this would only be a recommendation, right?). 

I agree.

> I mean, if I put "[foo]" inside "/etc/avocado/conf.d/bar.conf", will
> it be read and parsed?

Yes, should be.

> Thanks a lot of the write up.

Thanks Cleber, for your reply on this.

Regards,
Beraldo