On Mon, Dec 02, 2019 at 11:30:51AM -0300, Beraldo Leal wrote: > 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. > > > > There is no convention on the naming pattern used either on configuration > > > files > > > or on command-line options. Besides the name convention, there is also a > > > lack > > > of convention for some argument types. For instance:: > > > > > > $ avocado run -d > > > > > > and:: > > > > > > $ avocado run --sysinfo on > > > > > > Both are boolean variables, but with different "execution model" (the > > > former > > > doesn't need arguments and the latter needs `on` or `off` as argument). > > > > > > > Actually we do follow the pattern for booleans. The "--sysinfo" is a > > tri-state. > > I tried to see the third state but I might be missing something: > > -- > $ grep "\-\-sysinfo" * -R > plugins/run.py: parser.add_argument('--sysinfo', choices=('on', 'off'), > -- > > Also from `avocado run --help`: > > -- > --sysinfo {on,off} Enable or disable system information (hardware > details, profilers, etc.). Current: on > -- > > And here "Current" should be "Default". > > Could you please, point me to the third state? > > Keep in mind that I used `--sysinfo` just as an example, there are few > other cases: `--keep-tmp`, `--ignore-missing-references` and > `--failfast`. > > > > > 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 ...]] > > > > > > > Well you might want to specify no loaders (to override the default), > > although the only usecase I see is self-testing. But how about: > > > > avocado run --loaders LOADERS [LOADERS ...] > > > > is that acceptable? > > Yes, and I liked this one, it is better than my suggestions. > > > > 2. **Lists**: When an option argument has multiple values we should use > > > the > > > space as the separator. > > > > > > > This basically means: > > > > avocado run --loaders LOADERS [LOADERS ...] > > > > right? > > Yes, you are right. > > > > Presentation > > > ~~~~~~~~~~~~ > > > > > > Finding options easily, either in the manual or in the help, favor > > > usability > > > and avoids chaos. > > > > > > We can arrange the display of these options in alphabetical order within > > > each > > > section. > > > > > > > I'd love to (more-less), but sometimes people forget. It's hard to > > enforce this. Also there are exceptions where we want to make some > > options more visible, but in majority cases it should be A-Z. > > I think that if we have the options in alphabetical order, this will be > used as an example and new options will tend to be in order too. > > Also, If we agree with this suggestion and the general idea of this RFC, > I would suggest storing this RFC on something more concrete/permanent, > such as a "blueprint" and keeping in one of ours repositories, linking > to our dev/contributor guide. > > If, during the PR, the new argument option it is not in the order that > we expect, we can suggest this small change. But I understand that this > should be a common agreement. > > I'm pointing this here because, for a new developer and/or user, it > might be "painful" to find the right option without any logical order. > > > > Standards for Config File Interface > > > ----------------------------------- > > > > > > .. note:: Many other config file options could be used here, but since > > > that > > > this is another discussion, I'm assuming that we are going to > > > keep > > > using `configparser` for a while. > > > > > > As one of the main motivations of this RFC is to create a convention to > > > avoid > > > chaos and make the job execution API use as straightforward as possible, I > > > believe that the config file should be as close as possible to the > > > dictionary > > > that will be passed to this API. > > > > > > For this reason, this may be the most critical point of this RFC. We > > > should > > > create a pattern that is intuitive for the developer to convert from one > > > format > > > to another without much juggling. > > > > > > Nested Sections > > > ~~~~~~~~~~~~~~~ > > > > > > While the current `configparser` library does not support nested sections, > > > Avocado can use the dot character as a convention for that. i.e: > > > `[runner.output]`. > > > > > > This convention will be important soon, when converting a dictionary into > > > a > > > config file and vice-versa. > > > > > > > This is the only mentioning of args->config mapping. Can you please > > elaborate a bit more? > > Sorry if I was not clear, maybe I got your point now. But it is > straightforward: > > All default values (in source code) are overridden by config file > options, and then by the argument options (in this order). Probably we > will not have 1:1 mapping of all config file options to argument > options. So far I understood, the command-line argument options are the > basic ones and the most used. > > > > And since almost everything in Avocado is a plugin, each plugin section > > > should > > > **not** use the "plugins" prefix and **must** respect the reserved > > > sections > > > mentioned before. Currently, we have a mix of sections that start with > > > "plugins" and sections that don't. > > > > > > > So basically > > > > [vt] > > > > vt-related-option > > > > [vt.generic] > > > > generic-vt-related-option > > > > [runner] > > > > runner-related-option > > > > > > yes, the plugins section seems redundant as many parts are actually > > implemented as plugins. > > Yes, you got the point. > > > > > 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. > > > > > > I believe that having a convention here helps when writing more complex > > > tests, > > > configfiles, as well as easily finding plugins in various parts of the > > > project, > > > either on a manual page or during the installation procedure. > > > > > > I understand that the name of the plugin is different from the module > > > name in > > > python, but anyway, should we follow PEP8 in this case? > > > > > > From PEP8: Modules should have short, all-lowercase names. > > > Underscores > > > can be used in the module name if it improves readability. Python > > > packages should also have short, all-lowercase names, although > > > the use > > > of underscores is discouraged. > > > > > > > I'm not sure I understand properly this section. Can you please > > elaborate a bit more? Is the "_" -> "-" the problem you want to > > avoid?. > > Let's get the `human` example: > > Python module name: human > Plugin name: human > > Let's get the `task_run_recipe` example: > > Python module name: task_run_recipe > Plugin name: task-run-recipe > > Let's get another example: > > Python module name: archive > Plugin name: zip_archive >
This little mess was yours truly taking advantage of the lexicographic order of default plugin execution order... I wanted the archival of results to happen, by default, after all other result formats were executed. Anyway, I'm agreement that we should, as a general rule, have a better naming convention for the Python modules. A point raised in another response is the 1:1 .vs. 1:N relation between modules and plugins. > It is not a *big problem* just a convention in order to avoid chaos in > the near future. It is a bit more clear now? > > > > 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`. > > > > > > > I'd encourage people to use one, but we should attempt to accept all. > > Yes, I agree. And the good news is that `getboolean()` already support > all options. > > > > > 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? > > > > Security Implications > > > ##################### > > > > > > Avocado users should have the warranty that their jobs are running on > > > isolated > > > environment. > > > > > > We should consider this and keep in mind that any moves here should > > > continue > > > with this assumption. > > > > > > > I don't really understand this section, can you expand it (or remove > > it, if not necessary?) > > Maybe we can remove it. It is just a disclaimer in case someone brings > a completely new approach. > > > > How to Teach This > > > ################# > > > > > > We should provide a complete configuration reference guide section > > > in ourThis section doesn't seem important to me. User's > > > Documentation. > > > > > > In the future, the Job API should also be very well detailed so sphinx > > > could > > > generate good documentation on our Test Writer's Guide. > > > > > > Besides a good documentation, there is no better way to learn than by > > > example. > > > If our plugins, options and settings follow a good convention it will > > > serve as > > > template to new plugins. > > > > > > If these changes are accepted by the community and implemented, this RFC > > > could > > > be adapted to become a section on one of our guides, maybe something like > > > the a > > > Python PEP that should be followed when developing new plugins. > > > > > > > IIUC this section just says we should keep thinks as they are, right? > > If so than it doesn't need to be here, does it? Maybe one thing to add > > is that this basically follows the job API, right? And it's outcome > > should be well defined Job "args", right? Then the outcome of this > > should be a written standard of mandatory and optional sections of the > > "args" and all the relations between "args" and "options". Written, > > stable and flexible enough to suite all extra plugins needs. > > 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. > It doesn't hurt to keep an eye on the Job API, though, which I'm sure you are doing. > 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. > > It was not my intention to discuss the future job API on this RFC. This > will be discussed on a different RFC. And I'm already taking some notes > to do this soon. > And here is confirmation to my previous sentence :) - Cleber. > > Thank you, Beraldo, for opening this discussion. > > Lukáš > > > Thank you for spending some time on this, I appreciate. > > Regards, > Beraldo > >