Developers of Sympa

[Guillaume Rousse <address@concealed>]

Le 12/08/2013 15:23, IKEDA Soji a écrit :
> On Mon, 12 Aug 2013 13:25:42 +0200
> Guillaume Rousse <address@concealed> wrote:
>
> > Le 12/08/2013 12:36, IKEDA Soji a écrit :
> > > > > 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.
> > You're confusing instance and classes names here, and I'm precisely
> > trying to give classes appropriate names, according to what they store.
> > If both incoming and archiving spool contains messages, they should be
> > instances of the same Sympa::Spool::Message class, ie:
> > my $digest    = Sympa::Spool::Digest->new(...);
> > my $task      = Sympa::Spool::Task->new(...);
> > my $incoming  = Sympa::Spool::Message->new(...);
> > my $archiving = Sympa::Spool::Message->new(...);
> >
> > That is a coherent naming scheme.
>
> If that scheme did not include simplicity, I meant:
>
> my $spool = Sympa::Spool::Messages_to_be_Compiled_into_Digest->new();
> my $spool = Sympa::Spool::Messages_Sent_from_Outer_World->new();
> my $spool = Sympa::Spool::Tasks_Waiting_Execution->new();
> my $spool = Sympa::Spool::Messages_to_be_Added_to_Archives->new();
>
> Would you please give appropriate names?
I don't get your point here. Why would you need three different classes 
to manage three different message spools ? As as spool purpose is just 
to ensure persistancy by serialising data, that's just overkill for me.

[..]
> You suggested:
> | 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 ?
>
> So I rewrote examples in top half of this post to suit to your
> suggestion.  Translation: "$incoming is instance of Sympa::Spool
> class using 'msg' domain of the storage implemented by
> Sympa::Spool::Backend::SQL class and manipulates its contents as
> instances of Sympa::Message class."
You're giving an usage example, whereas my suggestion was to give an 
exhaustive overview of the list of classes involved by your
proposal. For instance, using the nomenclature I used in last month 
discussion:

* Scenario B1:
Sympa::Spool          abstract spool
|- Sympa::Spool::SQL  SQL-based spool spool
|- Sympa::Spool::File files-based spool

One single class hierarchy, for storage backends, and another class for 
spool themselves.

* Scenario C1:
Sympa::Spool                      abstract spool
|- Sympa::Spool::Message          abstract message spool
|  |- Sympa::Spool::Message::File file-based message spool
|  |- Sympa::Spool::Message::SQL  SQL-based message spool
|- Sympa::Spool::Task             abstract task spool
   |- Sympa::Spool::Task::File    file-based task spool
   |- Sympa::Spool::Task::SQL     SQL-based task spool

One single class hierarchy for all classes, with content as most 
important sorting criteria.

* Scenario C2:
Sympa::Spool                      abstract spool
|- Sympa::Spool::File             abstract file spool
|  |- Sympa::Spool::File::Message file-based message spool
|  |- Sympa::Spool::File::Task    file-based task spool
|- Sympa::Spool::SQL              abstract SQL spool
   |- Sympa::Spool::SQL::Message  SQL-based message spool
   |- Sympa::Spool::SQL::Task     SQL-based task spool

The alternative scenario to the previous, with storage as most important 
sorting criteria.

* Scenario C3:
Sympa::Spool                      spool class
Sympa::Spool::Content             abstract spool stored content
|- Sympa::Spool::Content::Message message stored content
|- Sympa::Spool::Content::Task    task stored spool
Sympa::Spool::Backend             abstract spool storage backend
|- Sympa::Spool::Backend::SQL     SQL-based storage backend
|- Sympa::Spool::Backend::File    filesystem-based storage backend

That's a more complex scenario, using two different classes hierarchy, 
one the for storage backend, one for the stored content, and a distinct 
class for spool.

David had strong arguments for either scenario C1 or C2 (saving 
content-specific metadata using specific backend requires specific 
code), whereas I'm more inclined toward B1, for sake of simplicity, as I 
think we can achieve the same result using fewer classes.

One you defined the entites you are going to manipulate, now you can 
discuss API.

Is that clearer ?

> > > > > 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).
> > It didn't supported the load, AFAIK. And the current plan was to allow
> > both types, with a single configuration switch (ie, all file-based, or
> > all sql-based spools).
>
> I quitely presumed switching should be possible by each spool.
>
> Actually, there may not be obvious consensus on this plan.
There isn't any clear specification here, hence the confusion. For me, 
the 7.0 plan was:
- support for both filesystem and SQL spool
- a single configuration directive to select which backend to use globally

That's enough complexity for me already, given our current workload.
--
Guillaume Rousse
INRIA, Direction des systèmes d'information
Domaine de Voluceau
Rocquencourt - BP 105
78153 Le Chesnay
Tel: 01 39 63 58 31

Attachment: smime.p7s
Description: Signature cryptographique S/MIME