-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Daniel Quinlan writes:
>[EMAIL PROTECTED] (Justin Mason) writes:
>
>> - (b) hack at Mail::SPF::Query to support async event polling.
>>
>> - (c) hack at Mail::SPF::Query to use the shared DNS cache. Both
>> of these will probably require us making a
>> Mail::SpamAssassin::Plugin::SPF::SPFQuery class -- our own
>> copy of M:S:Q.
>
>I'm not in favor of making our own copy of Mail::SPF::Query. It's still
>a moving target.
>
>I think we'd be better off making changes to Mail::SPF::Query so that we
>can continue to use it and get those accepted into upstream. Maybe
>something like we a way to the queries and feed the results into
>Mail::SPF::Query for analysis.
>
>At least, I think we need to think about it before we go down the deep
>hole of maintaining an SPF module.
OK, let's ignore that for now, and carry on requiring M:S:Q.
>> start_lookup() {
>> my ($self, $type, $bgsendargs, $done, $state) = @_;
>>
>> type: string representing the lookup type, e.g. "URIBL", "DNSBL",
>> "SPF" etc. Important so we can identify and kill lookups
>> for a particular subsystem if they're taking too long,
>> without killing all other lookups.
>>
>> It may be appropriate to use a numeric type here, although that
>> would mean that future plugins would need to reserve new numbers
>> (a bad thing).
>
>We *could* make a dynamic mapping from names to numbers. Arrays are
>cheaper and smaller than hashes and make sense when you don't really
>need a hash. I'm not saying that's the case, just that a dynamic
>mapping might work.
OK -- but how *big* do we expect this to get? ie. is the cheap/small
nature of arrays worth the lack of built-in extensibility? If we make
that an array, we've blocked any plugin authors from using the async
lookup engine outright, unless they patch the code or just pick a number
and hope we're not planning to use it. Which is a pretty bad idea IMO...
>> $bgsendargs: ref to array of arguments for the $res->bgsend() fn.
>> Also used (in stringified form) as key for {activelookups}
>> hash.
>
>What about order of arguments?
ref to *array*. ie. order preserved ;)
>> if $self->{activelookups}->{$bgsendargs} does NOT exist, a new
>
>s/activelookups/active/
ok.
>Same question about order of arguments and stringification of
>$bgsendargs.
see above.
>> subhash $self->{activelookups}->{$bgsendargs} is created with a
>> sub object $self->{activelookups}->{$bgsendargs}->{$type}, and
>> $self->{activelookups}->{$bgsendargs}->{BGSOCK} =
>> $res->bgsend($bgsendargs), then this method returns.
>
>->{$type} and ->{BGSOCK} might be a good place for an array.
>
>use constant BGSOCK => 0;
>use constant TYPE => 1;
>
>->[TYPE]
>->[BGSOCK]
I need more explanation here.
We're talking about these types:
"URIBL"
"DNSBL"
"SPF" (or maybe not)
So let's say we have these var settings for a typical DNSBL lookup:
$bgsendargs = [ "444.33.22.11.sbl.spamhaus.org.", "TXT" ];
$activekey = join (' ', @{$bgsendargs});
$type = "DNSBL";
$state = $whatever;
$done = \&Mail::SpamAssassin::Dns::dnsbl_query_done;
So you'd be suggesting:
# create the structure
$self->{active}->{$activekey} = [ ];
$self->{active}->{$activekey}->[TYPE] = { };
$self->{active}->{$activekey}->[TYPE]->{$type} = {
done => $done,
state => $state
};
$self->{active}->{$activekey}->[BGSOCK] = $res->bgsend($bgsendargs);
Instead of:
# create the structure
$self->{active}->{$activekey} = { };
$self->{active}->{$activekey}->{$type} = {
done => $done,
state => $state
};
$self->{active}->{$activekey}->{BGSOCK} = $res->bgsend($bgsendargs);
TBH, I can't see the benefit. I think you may have missed what
$type stands for here.
>> subhash $self->{activelookups}->{$bgsendargs} is created with a
>> sub object $self->{activelookups}->{$bgsendargs}->{$type}, and
>> $self->{activelookups}->{$bgsendargs}->{BGSOCK} =
>> $res->bgsend($bgsendargs), then this method returns.
>> count_lookups_of_type ($type)
>> count_lookups ()
>>
>> Return how many lookups are remaining in that type.
>> This allows dynamic timeouts a la Dns.pm.
>
>We'll also need to know how many lookups were started.
ok, that's easily done -- keep track lookups started and completed
on $self->{lookups_started}, $self->{lookups_finished}.
>> That's basically it. I think that should cover all the needs
>> of SPF, Dns.pm and URIBL lookups. comments?
>
>I'm still very concerned about the CPU overhead and I'm now somewhat
>concerned about the very significant additional complexity. It may
>really make more sense to roll everything into the (yes,
>high-specialized) Dns.pm code and also not use any plug-ins for this.
I took a look at the Dns.pm code when writing the URIBL stuff, and it's
just not possible to plug the URIBL into it currently without doing most
of this anyway -- since Dns.pm and URIBL use lookups, *on the same
records*, for different things. Hence the requirement for the $type
variable to describe *why* the lookup is being performed, since the
lookup *itself* does not describe that sufficiently.
But also, again, this ties into the concern I have that we're becoming
over-monolithic. There's little flexibility to add new lookups or
new relatively-complex chunks of code like URIBL, without *lots* of
knowledge about our internals. And *that* blocks people from
contributing to the project.
Instead of just chucking this into Dns.pm, and making that more complex,
we should try to open the engine up a little -- even if that costs 3%
runtime. That's why I added the plugin stuff anyway! ;)
- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Exmh CVS
iD8DBQFAS/HqQTcbUG5Y7woRAsgmAJ0YuRSFePvDRn6KepXEcqudT434KQCcDoN0
+pKahrhvg76gSZULLhAJrKU=
=XohX
-----END PGP SIGNATURE-----
