Re: svn commit: r149224 - in spamassassin/trunk: lib/Mail/SpamAssassin/PerMsgStatus.pm lib/Mail/SpamAssassin/Plugin.pm lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm rules/10_misc.cf rules/init.pre

[Michael Parker <pa...@pobox.com>]

On Mon, Jan 31, 2005 at 05:52:35AM -0000, jm@apache.org wrote:
> Author: jm
> Date: Sun Jan 30 21:52:33 2005
> New Revision: 149224
> 
> URL: http://svn.apache.org/viewcvs?view=rev&rev=149224
> Log:
> move default Bayes auto-learn discriminator out of core, into an active-by-default plugin, so that it can be overridden if desired
> 
> Added:
>     spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm
> Modified:
>     spamassassin/trunk/lib/Mail/SpamAssassin/PerMsgStatus.pm
>     spamassassin/trunk/lib/Mail/SpamAssassin/Plugin.pm
>     spamassassin/trunk/rules/10_misc.cf
>     spamassassin/trunk/rules/init.pre
> 

Few things:

1) I thought plugin callss couldn't return values?

2) I like the Plugin API, but why not keep the default in the code and
   allow added plugins to override?  Doesn't need it's own default
   plugin.

3) MANIFEST, assuming the plugin stays.


Michael

Re: optional vs. standard plugins

[Daniel Quinlan <qu...@pathname.com>]

(new Subject:)

Michael Parker wrote:

>> Which might be ok, but I can promise you that someone is going to go
>> through and either rm init.pre or comment out every loadplugin line
>> and then start asking questions about why their system isn't
>> autolearning.

Losing autolearning if someone deletes init.pre is completely
acceptable.  Autolearning *should* be optional and pluggable.  Making it
pluggable allows people to experiment and try out other autolearning
mechanism and I suspect we'll see some usage of the API soon.  ;-)

We could also add a new autolearn state like "notloaded".

Theo Van Dinter <fe...@kluge.net> writes:

>> I think we should shoot for a goal of when all plugins are disabled
>> the system should still do the right thing.  If that means that we at
>> least provide a default inline that can be overridden by a plugin,
>> then that is how we should do it.

Not autolearning if it has been disabled *is* the right thing.  Things
work fine if autolearning is off.

Also, our current autolearning code does not improve results by that
much in practice (which is why it needs to be revisited and other ways
to autolearn to be explored).  See Gordon C.'s paper for those results.

> I'll provide a slightly different version: for code that people are
> likely not to override (such as autolearning), we should probably just
> have it be in the code by default and let plugins override as
> necessary..

I disagree in this case, although I think there are probably some cases
where things are likely to not be overridden.  Users are going to
encounter plugins and they're now a major part of basic SpamAssassin
functionality (much like Apache httpd, incidentally), we should just
document things well enough.  If people comment out stuff without
thinking, then there's not too much we can do about it.

For plugins that are likely to not be overridden, I'd be fine with
splitting init.pre into two or more files, like:

  standard.pre
  optional.pre
  experimental.pre

or whatever.  That would go a long way to guiding people as to how
seriously they need to think before commenting stuff out.

Of course, I agree ** 100% ** that everything should work as in "not
fail" if all plugins are commented out.  There might be a few cases
where plugins have cross-dependencies, but we should make sure our code
deals with those and acts appropriately (warn, die, dbg, or whatever,
but *no* straight Perl interpreter errors!).

Also, putting a line next to the AutoLearnThreshold load line such as:

# at least one AutoLearn plugin needs to be loaded for autolearning to work

is more than enough to prevent a stupid commenting out.  If people just
comment stuff out without thinking or delete init.pre, we can't save
them.

Daniel 

-- 
Daniel Quinlan
http://www.pathname.com/~quinlan/

Re: svn commit: r149224 - in spamassassin/trunk: lib/Mail/SpamAssassin/PerMsgStatus.pm lib/Mail/SpamAssassin/Plugin.pm lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm rules/10_misc.cf rules/init.pre

[Theo Van Dinter <fe...@kluge.net>]

On Mon, Jan 31, 2005 at 05:25:25PM -0500, Theo Van Dinter wrote:
> > Which might be ok, but I can promise you that someone is going to go
> > through and either rm init.pre or comment out every loadplugin line
> > and then start asking questions about why their system isn't
> > autolearning.
> Yeah, but they'll do the exact same thing with SURBL, Razor, etc.

Actually, in thinking about this some more, the more likely thing to
happen is that during an upgrade, init.pre isn't updated with the new
list of plugins, so Autolearn wouldn't be loaded at all.  Think of RPM
and the like which will probably make something like init.pre.rpmnew.

-- 
Randomly Generated Tagline:
... I want a COLOR T.V. and a VIBRATING BED!!!

Re: svn commit: r149224 - in spamassassin/trunk: lib/Mail/SpamAssassin/PerMsgStatus.pm lib/Mail/SpamAssassin/Plugin.pm lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm rules/10_misc.cf rules/init.pre

[Michael Parker <pa...@pobox.com>]

On Mon, Jan 31, 2005 at 05:25:25PM -0500, Theo Van Dinter wrote:
> On Mon, Jan 31, 2005 at 04:16:01PM -0600, Michael Parker wrote:
> > Which might be ok, but I can promise you that someone is going to go
> > through and either rm init.pre or comment out every loadplugin line
> > and then start asking questions about why their system isn't
> > autolearning.
> 
> Yeah, but they'll do the exact same thing with SURBL, Razor, etc.
> 

I'm not so worried about those, those are all pretty much self
contained, so if they get shutoff no harm done.  It's turning off
pieces of a system in the core that bothers me.

Michael

Re: svn commit: r149224 - in spamassassin/trunk: lib/Mail/SpamAssassin/PerMsgStatus.pm lib/Mail/SpamAssassin/Plugin.pm lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm rules/10_misc.cf rules/init.pre

[Theo Van Dinter <fe...@kluge.net>]

On Mon, Jan 31, 2005 at 04:16:01PM -0600, Michael Parker wrote:
> Which might be ok, but I can promise you that someone is going to go
> through and either rm init.pre or comment out every loadplugin line
> and then start asking questions about why their system isn't
> autolearning.

Yeah, but they'll do the exact same thing with SURBL, Razor, etc.

> I think we should shoot for a goal of when all plugins are disabled
> the system should still do the right thing.  If that means that we at
> least provide a default inline that can be overridden by a plugin,
> then that is how we should do it.

I'll provide a slightly different version: for code that people are likely not
to override (such as autolearning), we should probably just have it be in the
code by default and let plugins override as necessary..

-- 
Randomly Generated Tagline:
"When proteins get hot they tend to tangle up tighter than teenagers at
 a dance. And when they bond up tight enough, they over coagulate. And
 when they over coagulate, they can curdle. And any cook or parent will
 tell you that leads to trouble."
                         - Alton Brown, Good Eats, "Good Milk Gone Bad"

Re: svn commit: r149224 - in spamassassin/trunk: lib/Mail/SpamAssassin/PerMsgStatus.pm lib/Mail/SpamAssassin/Plugin.pm lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm rules/10_misc.cf rules/init.pre

[Michael Parker <pa...@pobox.com>]

On Mon, Jan 31, 2005 at 01:10:46PM -0800, Justin Mason wrote:
> 
> > If someone disabled the plugin in init.pre and did not install their
> > own plugin, what would happen?
> 
> no autolearning occurs, everything else works as expected. ;)
> 

Which might be ok, but I can promise you that someone is going to go
through and either rm init.pre or comment out every loadplugin line
and then start asking questions about why their system isn't
autolearning.

I think we should shoot for a goal of when all plugins are disabled
the system should still do the right thing.  If that means that we at
least provide a default inline that can be overridden by a plugin,
then that is how we should do it.

Michael

Re: svn commit: r149224 - in spamassassin/trunk: lib/Mail/SpamAssassin/PerMsgStatus.pm lib/Mail/SpamAssassin/Plugin.pm lib/Mail/SpamAssassin/Plugin/DefaultAutoLearnDiscriminator.pm rules/10_misc.cf rules/init.pre

[Michael Parker <pa...@pobox.com>]

On Sun, Jan 30, 2005 at 10:49:04PM -0800, Justin Mason wrote:
> Michael Parker writes:
> > Few things:
> > 
> > 1) I thought plugin callss couldn't return values?
> 
> actually -- they can.  (I think in the config file case, it was the type
> of the return value changing frequently that was problematic).
> 

So, in the case where more than one plugin handles a call, which value
is returned?  last one run wins?

> > 2) I like the Plugin API, but why not keep the default in the code and
> >    allow added plugins to override?  Doesn't need it's own default
> >    plugin.
> 
> well, effectively doing this as a default plugin *does* this, without
> adding extra code for plugins to indicate "do not run the default
> code", which is why I did it this way.  (however perhaps it doesn't
> need to be in the Mail::SpamAssassin::Plugin hierarchy, it could
> be named something else.)
> 

Something like:

$foo = call_plugin

if !defined($foo)
  the default code goes here to set $foo


If someone disabled the plugin in init.pre and did not install their
own plugin, what would happen?

Michael