Developers of Sympa

[IKEDA Soji <address@concealed>]

Hi,

On Mon, 31 Mar 2014 17:57:33 +0200
Guillaume Rousse <address@concealed> wrote:

> Le 24/03/2014 14:51, IKEDA Soji a écrit :
> > Hi,
> >
> > On Thu, 06 Mar 2014 18:09:00 +0100
> > David Verdin <address@concealed> wrote:
> >
> >>    * Separate clearly oo and non oo paradigm. Stop using classes to store
> >>      states (use singletons instead)
> >
> > I have some ideas on this issue.
> >
> > * Conf
> >
> > I suppose that Conf (Sympa::Configuration) would be purified to a
> > set of helper functions parsing some sorts of configration format.
> > So it may not be an OO package.
> In this case, it should probably be renamed as Sympa::Tools::Configuration.
> 
> > * Site and Log
> >
> > Site would be instantiated as singleton.  Log would be, too.  And it
> > would be initialized explicitly (not referring Site parameters
> > internally).  E.g. at the start of a program:
> >
> >    our ($site, $log);
> >
> >    # Load site config.
> >    $site = Sympa::Site->new();
> >    $site->load() or die 'Error in config';
> >
> >    # Open log (syslog).
> >    $log = Sympa::Log->new();
> >    $log->set_log_level($site->log_level);
> >    $log->do_openlog($site->log_facility, $site->log_socket_type, 
> > "MYNAME");
> >
> >    # Attach DB log.
> >    my $dbm = Sympa::DatabaseManager->new(
> >        $site->db_type, $site->db_name, $site->db_user, $site->db_passwd);
> >    $log->attach_action_log($dbm);
> >    $log->attach_stat_log($dbm);
> >
> >    # The first speech.
> >    $log->do_log(
> >        'notice', 'MYNAME started, default log level %d', $log->log_level);
> >
> >    # Do the work
> >    ...
> That's also what I expected, with a few minor differences.
> 
> 1) a few renaming would help making the code more litterate:
> - Sympa::Log package may bet renamed as Sympa::Logger
> - do_log function/method should be renamed to something write_message()
> 
> Just compare the following pseudo-code:
> my $logger = Sympa::Logger->new();
> $logger->write_message($message);
> 
> my $log = Sympa::Log->new();
> $log->do_log($message);
> 
> 2) the current Sympa::Log::Syslog package should be splitted in two 
> different classes (Sympa::Log::Syslog and Sympa::Log::Stdout), 
> implementing a common interface (Sympa::Log), so as to be used as 
> alternatives.
> 
> for a daemon
> my $log = Sympa::Log::Syslog->new();
> $log->do_log($message)
> 
> for a run-and-quit admin tool:
> my $log = Sympa::Log::Stderr->new();
> $log->do_log($message)
> 
> 3) I'd rather not have an explicit do_openlog() initialisation methods, 
> and I'd merge it with constructor, to prevent the case of 
> non-initialized logger.
> 
> 4) I'd rather find another namespace (Sympa::PerfManager, 
> Sympa::StatsManager, ...) for DB logs, given it is used for different 
> purpose than event logging.

Trace log is often represented by debug2 log level in current code.
Sympa::PerfManager (or Sympa::Trace?) looks better for me.

IMHO statistics may be derived from generic action log.


> > * DatabaseManager and DataSource
> >
> > I suppose the both would be unified and their instances wouldn't be
> > singleton.
> >
> > Because, for example, currently session_table will be accessed/
> > updated everytime when the requests by clients occur: Administrator
> > may wish to separate this table into local storage (SQLite) if
> > possible.
> > And for example, s/he wish to use particular datasource for
> > mysql_alias_manager, instead of that provided by default site config.
> This kind of corner case won't help making the code simple, readable, 
> and efficient.

Another example: Administrator may wish to manage user_table (with
custom columns) on organization-central database... well, how abouot
plugins such as OAuth?  Yet another: S/he can put list_table (this
is just a cache) into SSD....

After all, I suppose mandatory tables may be only admin_table,
bulk*_table and subscriber_table.  The "corner cases" seem rather
usual cases.


> > So they would be instantiated by each, and I believe there are no
> > use to determine DataSource and DatabaseManager (SDM).
> >
> > However, each of their instances should be able to share common
> > connection cache and statement cache.*
> if the choice is between "let's keep the current code to allow specific 
> use case" and "let's get rid of half the complexity in order to make 
> sympa more efficient for everyone", I'm sure you can guess my personal 
> opinion on this subject.

I don't stick to the current code: You saw my experimental code that
is similar to neither current code nor your "simplified" code.

However, I at large prefer to current interface of SDM
(DatabaseManager) which successfully hides intricate details on
numerous workarounds and cache mechanisms.  In this meaning, I
stick to the current code.

I don't wish dichotomous discussion.


Regards,

--- Soji

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