Developers of Sympa

[IKEDA Soji <address@concealed>]

On Mon, 12 Aug 2013 11:00:39 +0200
Guillaume Rousse <address@concealed> wrote:

> Le 12/08/2013 08:01, IKEDA Soji a écrit :
> > Hi,
> >
> > While grandma is cutting a cake, I'd like to make decision
> > on convention about spool objects.
> >
> > # Honestly, I prefer to use implementation based on filesystem
> > # for all spools.  I worry SQL-based implementation is efficient
> > # enough, especially to handle 10MB or larger messages).
> > # So it is important for me Sympa-7 can use both implementation.
> >
> > Current convention:
> >
> >      my $spool = Sympa::Spool::SQL->new(
> >          name => 'digest',
> >          base => Sympa::Database->get_singleton()
> >      );
> >      my $spool = Sympa::Spool::File::Message->new();
> >      my $spool = Sympa::Spool::File::Task->new();
> >      my $spool = Sympa::Spool::File->new('outgoing');
> That's not really a convention, rather just the need to keep the code 
> compiling. I perfectly agree than backend implementation class should 
> not be hardcoded if we need to make it configurable.
> 
> > I'd like to propose as following:
> >
> > o Spool class would be handled in single style:
> >
> >      my $spool = Sympa::Spool::Digest->new();
> >      my $spool = Sympa::Spool::Incoming->new();
> >      my $spool = Sympa::Spool::Task->new();
> >      my $spool = Sympa::Spool::Archiving->new();
> 'Task' and 'Digest' are nouns, whereas 'Archiving' and 'Incoming' are 
> not, this is not consistent. I guess incoming spool is meant to store 
> messages, so that should rather be:
> my $incoming = Sympa::Spool::Message->new();

Please give appropriate names.  "Incoming" spool contains messages
sent from outer world.  "Archiving" spool contains messages to be
added to archives.

> > o Each class is a subclass of common abstract class (i.e.
> >    Sympa::Spool) and has following variations:
> >
> >     (1) Content object to handle (message digest, Message, Task, ...),
> >       defined by each class itself.
> >     (2) Physical backend (SQL, filesystem, ...) defined by configuration
> >       option.
> >     (3) Domain in the backend (on SQL, value of name_spool column;
> >       on filesystem, name of subdirectories: "digest", "msg", "task",
> >       ...) defined by each class itself.
> It would be clearer to enumerate the needed classes, and to define their 
> exact relationships. For instance, how will you manage backends ? Yet 
> another class hierarchy ?

Of course, another class will be used.  I have not yet understood why
you are rejecting "two class hierarchies" idea.

> And I still object having spool classes hardcoding their settings, which 
> tends to multiply artificially the number of classes needed. If you 
> hardcode the fact than spool 'X' uses domain 'X' in a Sympa::Spool::X 
> class, than you'll need as much classes as you have spools. For me, 
> those settings should be passed at instanciation time by calling code, ie:
> 
> my $incoming = Sympa::Spool::Message->new(
>      storage => $configuration->get_param('spool_storage'),
>      domain  => 'incoming',
>      name    => 'incoming'
> );

"name" looks duplicate of "domain".  So if I wrote in similar style:

my $incoming = Sympa::Spool->new(
    generator => 'Sympa::Message',
    storage => 'Sympa::Spool::Backend::SQL', # taken from 
incoming_spool_storage
    domain  => 'msg',                        # name of spool for incoming 
messages.
);


> > o Configuration parameters for (2) above would be defined for
> >    each spool.  Current possible values are "sql" and "filesystem".
> >
> >     digest_spool_storage
> >     incoming_spool_storage
> >     task_spool_storage
> >     ...
> >
> > Any comments?
> Is it really needed to mix spool storage type ? What about first getting 
> a simple, but working implementation, before reaching into unproven 
> complexity ?

As I remember ---
- In sympa-6.1, filesystem-based spool were used for all spools
  except for bulk sending.
- Then in sympa-6.2-branch, there was an attempt to switch almost
  all spools to SQL-based.
- However, this plan seems to have been partially changed (I don't
  excactly know why).

To keep past works, we logically should cope with both spools.

Certainly, we might have other options such as Guillaume wrote in
other post.


Regards,

--- Soji

-- 
株式会社 コンバージョン  セキュリティ&OSSソリューション部   池田荘児
〒231-0004　神奈川県横浜市中区元浜町3-21-2 ヘリオス関内ビル7F
e-mail address@concealed  TEL 045-640-3550
http://www.conversion.co.jp/