Re: tvd-evaltoplugin

[Justin Mason <jm...@jmason.org>]

Theo Van Dinter writes:
> On Sat, Feb 25, 2006 at 12:52:16AM -0800, Loren Wilton wrote:
> > Without looking at the code, and as something of an outsider, some thoughts
> > that may be useful:
> 
> :)
> 
> > 1.  Is there any notable rule efficiency difference between being an eval
> > and being a plugin?
> 
> The results are exactly the same.
> 
> > 2.  If there isn't any notable performance difference, then this amounts to
> > an interface change.  Assuming the evals use standard plugin interfaces
> > (assuming plugin interfaces ARE standard), then fewer interfaces is probably
> > a good thing.  If the evals require a bunch of hackish special interfaces to
> > work, maybe it is a bad idea.
> 
> There's nothing hackish involved, but I don't have performance numbers.

If anything, it's easier since, as Theo said, it means that there's one
less wierd legacy interface left in the code; the idea of the EvalTests.pm
file inserting methods directly into the PerMsgStatus namespace is
exceedingly hacky.

> > 4. Not splitting evals into separate rules: what effect would this have on
> > trying to priortize and short-circuit rules?  Would it be possible to run
> > some rules in a package and not others?
> 
> None, and yes.  Having the code as plugins instead of in EvalTests just moves
> where the code lives, and allows for code to be disabled or swapped out.
> 
> > 5. It would probably be good to consider the idea of a "welded plugin" that
> > can't be unplugged and doesn't need to be explicitly plugged in someplace.
> 
> I think that's pretty easily doable (add load_plugin() calls to
> M::SA::init()) if we wanted to go that route.  For eval rules, it would
> at least let us have a standard API, and clean up the PerMsgStatus object,
> though I like the idea of having plugins be unpluggable. :)

I don't think there's a need for any EvalTests to *always* be loaded. Bear
in mind that the EvalTests functions are already not guaranteed to be run,
since right now a user can set their scores to 0 and they won't run.

Plus I like the idea of not even loading the DNS code if -L switch is
being used; it'd save memory!

The only thing I can see that could be problematic, is how many new plugin
modules it creates.  If there's more than 3 or so, I'd prefer to see it
around 3.  This really ties into John's concerns.   in my opinion having
a load of small plugin files isn't really a win unless there's real
reasons to have eval-test function A be in a different file from eval-test
function B (such as the network-test example).

Overall though: +1 on this work, and +1 on merging this to trunk if
all tests pass with "make test" and "make disttest".

--j.