Re: MidgardConfig comments (Was: [midgard-dev] Automake and directories)

"Jukka Zitting" <[email protected]> wrote:

> Hi,

Hi,
 
> > Something else except database directives?
> 
> Details like:
> 
>    a) Location of the system configuration files

We can add them whenever we want and current GHashTable lookup or 
GKeyFile propper usage doesn't matter I think. Does it?

>    b) Support for per user configuration files

Per user in db config file?

>    c) Comment syntax

Don't all comments begin with '#' ?

>    d) Whether spaces around "=" in "name=value" are ignored

This is issue which can be ignored right now I think.
In 1.8 you should "edit" config file from dg level or manually , if you know what you are doing.

>    e) Format of boolean values

I do not understand.
 
> None of these is too hard by itself, but they do add up to a real
> backwards compatibility issue. If we endorse the current MidgardConfig
> in 1.8, then IMO we should not change it later on.

But benefit of MidgardConfig is that you do not have to worry about what config file 
looks like and how is parsed. Right?

> > > My point is that midgard-core should not try to decide which levels
> > > are being logged. It should just log messages using the logging macros
> > > and let the application decide the log level.

You are always able to change loglevel and default log handler. So I do not see 
any problem here.

> > I am not sure if I understand you here. What I am talking about is *using*
> > GLib log macros with mgd_log_debug_default support which allows you
> > print only those log levels which are set in configuration.
> 
> My point is that midgard-core should *not* set it's own log handler.
> We can provide a function like the following, but it should be the
> client application that provides the log handler.

OK, I still do not see a problem as you can set application's one.
 
> /**
>  * Sets the log handler for the messages logged by the Midgard core.
>  */
> guint midgard_set_log_handler(
>         GLogLevelFlags levels, GLogFunc handler, gpointer data) {
>         return g_log_set_handler(G_LOG_DOMAIN, levels, handler, data);
> }
> 
> /**
>  * Removes the identified log handler from the Midgard core.
>  */
> void midgard_remove_log_handler(guint handler_id) {
>         g_log_remove_handler(G_LOG_DOMAIN, handler_id);
> }

This is similiar what is made in php's mgd_debug_start and mgd_debug_stop.

> With such an API midgard-apache2 could easily redirect all
> midgard-core log messages to the correct Apache log handler:

         }
>         ap_log_rerror("midgard", 0, aplog_level, 0, r, "%s", message);
> }

No. Personally I am very tired of midgard-php being actively used while 
being something between php and apache extension.
Once we removed that dependency in 1.7 we should never come back there.

Such log handler should be set from php level , cli or web app.
And you shouldn't depend on ap_ calls there.

Perfect log should looks like this ( set from midcom level ) :

midcom: request failed midcom_parse_url
midgard-php: midgard_host
midgard-apache:  midgard_get_host
midgard-core: query  $query failed: $reason

> 
> ------------------------------------------------------------
> 
> when request starts:
>         request_rec *r = ...;
>         guint log_handler_id = midgard_set_log_handler(
>                 G_LOG_LEVEL_MASK, log_handler, r);
> 
> when request ends:
>         midgard_remove_log_handler(log_handler_id);

We shouldn't set this when request starts. You must be able to set 
handler anytime you need. Even in the middle of the request.
 
> > So do not define midgard logfile  and all messages will be logged to file
> > defined for Apache's ErrorLog directive. It works.
> 
> Nope. Consider the following configuration:
> 
>     LogLevel warn
>     ErrorLog /var/log/httpd/main.error.log
>     <VirtualHost foo.com>
>         LogLevel debug
>         ErrorLog /var/log/httpd/foo.error.log

OK, how would you like to manage loglevel , logfile and log handler then?
We need to be able to set it from midcom level while being completly 
Apache unaware. The other thing is php-cli.

> In this setup the midgard-core log messages would end up in
> main.error.log regardless of the site being processed. What's also
> troublesome is that if the midgard configuration uses the "debug" log
> level then the main.error.log will end up containing midgard debug
> messages even if the Apache LogLevel setting is higher.

Which is logic IMO , you define midgard log level , not Apache's one.
In practice ( look back at midgard user and dev list ( ~ 3 years ) )
we usually need to set midgard log level and handler from *any* level.
Apache loglevel is tha last thing we usually need to touch.
 
> > What I have to do to make MidgardConfig available on PHP level is to define
> > Zend class and its methods ( if exists ). Property setter and getter may be used the
> > same which is used for MgdObject(s).
> 
> There should be no need to handle the config objects in a normal
> Apache/PHP enviroment. 

Yes , you are absolutely right. But as long as we think about midgard&apache&php
we do not even use midgard-php with php-cli. And we will start to use because 
we need to write php datagard.

> The midgard-apache module just uses
> MidgardConfigFile and passes the given parameter to
> midgard_connection_open(const char *, GError **). No need to
> instantiate or manipulate any config objects.
> 
> The only reason I can think of for using the config objects in PHP is
> using PHP-CLI and manually setting up the midgard connection with
> mgd_connect(). I think a normal PHP array would be good enough to pass
> any configuration options to mgd_connect().

I look at Torben now.

1.
	$midgard = new midgard();
	$midgard->read_config("filename");
	$midgard->connect();

is much preffered way than mgd_connect.

2. 
	Core uses GKeyFile
	midgard-php arrays
	midgard-java objects

I do not like it.
Especially when we are going to use PHP5 soon with improved OO mechnisms.

Piotras