[Avocado-devel] RFC: Plugin execution order

Mon Oct 3 11:26:05 UTC 2016

Dne 30.9.2016 v 17:21 Jeff Nelson napsal(a):
> On Fri, Sep 30, 2016 at 07:24:53AM -0300, Cleber Rosa wrote:
>>
>> On 09/29/2016 04:14 PM, Jeff Nelson wrote:
>>> On Thu, Sep 29, 2016 at 01:24:22PM -0300, Cleber Rosa wrote:
>>>> As detailed in the following card:
>>>>
>>>>  https://trello.com/c/oWzrV48E/837-execution-order-support-in-plugins
>>>>
>>>> It should be possible to specify a custom order for plugins to be
>>>> executed by setting specific configuration.
>>>>
>>>> The first observed approach would be to create a section
>>>> called `[plugins.<type>]` where the `<type>`  conforms to the
>>>> description on fully qualified plugin names given here:
>>>>
>>>>
>>>> https://github.com/avocado-framework/avocado/pull/1495/commits/193a10ce98cb5747395eefcb485dd452696b4b11#diff-0f4f89ace79fa15278d9b283c2d9d9b2R84
>>>>
>>>>
>>>>
>>>> Then, by creating a key named `order`, containing the short names as a
>>>> list.  Enabled plugins not listed will be executed *after* plugins
>>>> listed, but in non-determined order.
>>>
>>> The phrase "enabled plugins" implies that there can be disabled
>>> plugins as well.
>>>
>>>
>>
>>
>> Yeah, support for that has just been added:
>>
>> http://avocado-framework.readthedocs.io/en/latest/Plugins.html#disabling-a-plugin
>>
>>
>>>
>>>> For instance, consider the following entry points::
>>>>
>>>>  'avocado.plugins.result' : [
>>>>     'xunit = avocado.plugins.xunit:XUnitResult',
>>>>     'json = avocado.plugins.jsonresult:JSONResult',
>>>>     'archive = avocado.plugins.archive:Archive',
>>>>     'mail = avocado.plugins.mail:Mail',
>>>>     'html = avocado_result_html:HTMLResult'
>>>>   ]
>>>>
>>>> We can say that:
>>>>
>>>> * The plugin type, according to the fully qualified plugin name
>>>>  definition here is `result`.
>>>>
>>>> * The plugin fully qualified names are:
>>>>  - result.xunit
>>>>  - result.json
>>>>  - result.archive
>>>>  - result.mail
>>>>  - result.html
>>>>
>>>> * The short names for plugins of type "result" are:
>>>>  - xunit
>>>>  - json
>>>>  - archive
>>>>  - mail
>>>>  - html
>>>
>>> I like this use of examples. The illustrations are clear and easy to
>>> understand.
>>>
>>>
>>
>> Thanks.
>>
>>>> To make sure that the mail plugin is run after (and thus includes)
>>>> the HTML result, the following configuration entry can be set::
>>>>
>>>>  [plugins.result]
>>>>  order = html, archive
>>>
>>> What does it mean for a plugin to "include" the results of an earlier
>>> plugin?
>>>
Yep, I like this. Simple set of all plugins, then a few ordered `pop`s 
from it followed by loop over the remnant.

>>>
>>
>> I meant that a imaginary "mail" plugin, would include all previously
>> generated results, so it would include the HTML report.  That wasn't a
>> really good example when it comes to *core* functionality, that is, the
>> plugin system wouldn't by its own do any of those inclusions.
>
> Understood. I just wanted to make sure there wasn't any special magic
> going on, and there isn't.
>
>
>>>> The other result plugins, namely xunit, json and mail, will still
>>>> be run.  It's guaranteed they'll be run *after* the other result
>>>> plugins.  The order in which they'll run after the explicitly
>>>> ordered plugins is undefined.
>>>
>>> Can a plugin determine what plugin(s) have run before it? (I don't
>>> think it's necessary.)
>>>
>>>
>>
>> In theory, a plugin could look at the configuration system and get that
>> order.  But, I don't think that's going to be a common use case.
>
> Agreed, it's not a use case we care about.
>
>
>>>> Other possible approach
>>>> -----------------------
>>>>
>>>> The other approach possible, would require a default order value
>>>> for plugins.  This would still preferably be done in configuration
>>>> rather than in code.  Then, the fully qualified name for a plugin could
>>>> be used as part of the configuration section.  Example::
>>>>
>>>>  [plugin.result.archive]
>>>>  order = 50
>>>>
>>>>  [plugin.result.html]
>>>>  order = 30
>>>
My first idea was something like this, but it'd be actually slower to 
define, harder to get the final order and slower to execute than the 
first approach, therefor I vote for the first one.

>>> Small typo: "plugins.result" not "plugin.result" (two lines).
>>>
>>>
>>
>> Yep, thanks!
>>
>>>> This would make the `html` plugin run before the `archive` plugin.
>>>> While more verbose, it would allow for external plugins to ship with
>>>> stock configuration files that would set, by default, its ordering.
>>>
>>> Order is here is used as a numerical value to indicate a relative
>>> ordering, correct? I'm not sure I like the name "order"; how about
>>> "sequence"?
>>>
If we are to go this way I'd chose the name priority. It was already 
there before the new plugin system implemented as plugin's attribute.

>>> A drawback to this approach is that you still have to come up with a
>>> rule for what happens when two plugins have the same sequence number.
>>>
>>
>> Then the order is undefined.  I don't see a clean way around this.
>
> Nor do I. Which makes this a less-than-optimal solution as far as I am
> concerned.
>
Well this has been working well for decades in SystemV init scripts 
(although there the order is lexical, which we can also simulate, but it 
does not make much sense here as changing the plugins entry point just 
to change the order is not really feasible. Plugins are not init scripts 
we could just rename. I mean they can be renamed, but it'd be quite ugly 
and deep change with possible consequences)

>
>>>> Feedback is highly appreciated!
>>>
>>> Another way to specify the order is to use an attribute at the
>>> [plugins] or [plugins.result] level with certain expected values:
>>>
>>> [plugins]
>>> execution-order = random | lexical | user-defined
>>>
>>> where 'user-defined' would require yet another attribute
>>> that defines the sequence. 'user-defined' would also have to
>>> handle the 'unspecified' condition.
>>>
>>> I think this is too complicated, but I offer it as a counter-example.
>>>
>>
>> Random is default, and I fail to see the practical use of lexical.
>> About user-defined, I was trying to avoid (at this point) code based
>> ordering.
>
> Lexical ordering gives you a non-random, (mostly) easy-to-reason-about
> execution order. One downside is that the order is implied in the name
> of the plugin, it's not specified explicitly. Another drawback:
> problems when the lexical ordering is not what the user expected are
> hard to debug. For example, the order can change just by switching the
> underlying lexical system.
>
> For these reasons, I don't like lexical ordering. In my opinion, the
> best proposal is the first: random ordering unless the user defines an
> explicit sequence with the "plugins.<type>.order" attribute.
>
>
>>> Finally, the typo got me to thinking: should it be "plugin" or
>>> "plugins"? I don't care one way or the other. However, I do believe
>>> that the attribute should be named consistent with the rest of
>>> avocado. If there isn't a style guide for avocado proposals and naming
>>> conventions, it would be good to have one. Consistency is going to be
>>> hard to establish and maintain without it.
>>>
>>
>> I would go with "plugins", because we already have a global section that
>> configures "all plugins behavior", and that section is called "plugins".
>> Then, I see "plugins.<type>" as "sub section" (logically only) of that
>> main one, that configures plugins of a given type.
>
> OK.
>
Ack

>>> -Jeff
>>>
>>>
>>
>> Thanks for the feedback!
>
> You're welcome.
>
> -Jeff
>
>
>>>>
>>>> --
>>>> Cleber Rosa
>>>> [ Sr Software Engineer - Virtualization Team - Red Hat ]
>>>> [ Avocado Test Framework - avocado-framework.github.io ]
>>>>
>>
>> --
>> Cleber Rosa
>> [ Sr Software Engineer - Virtualization Team - Red Hat ]
>> [ Avocado Test Framework - avocado-framework.github.io ]
>>
>
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 502 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20161003/c8d737b6/attachment.sig>