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

Hi,

On 2/17/06, Piotras <[email protected]> wrote:
> We need to use new config files at least for midgard-schema.
> After 1.8 is out this mechanism will be the only one ( preffered ).

OK, cool.

> > Couldn't we use something like g_key_file_load_from_data_dirs() to
> > automatically find the configuration files from the default system
> > locations?
>
> I haven't available docs here. What's minimum GLib version?

GLib 2.6, but this can be relaxed as described below. The
freedesktop.org standard basedir locations
(http://www.freedesktop.org/Standards/basedir-spec) used by
g_key_file_load_from_data_dirs() don't seem to match our needs very
well, using a configuration search path like
$HOME/.midgard:$prefix/etc/midgard/conf.d would be nice.

> > Requiring glib 2.6 or even using a local copy of the glib 2.6 GKeyFile
> > sources would be a small price for this.
>
> Can we do this?

Yes. GLib is LGPL and the gkeyfile.c is mostly quite modular. We can
use a configure option to activate a local copy of the GKeyFile
implementation if GLib 2.6 is not available. If you want I can write
the configure magic for this.

> > 6) Are there any ideas on how the configuration mechanism will change
> > when we switch to libgda? There is a full data source configuration
> > mechanism in libgda so we could in principle just use a single data
> > source name to get the database connection.
>
> But we need midgard releated options , very important for me is for example
> loglevel which become be usefull.

True. I was thinking that we could for example deprecate the old
database configuration parameters and prefer the libgda data source
configuration instead. Then we would just have a single DataSource
configuration option in the Midgard configuration file that refers to
the more detailed libgda configuration.

> >     GError *error = NULL;
> >     MidgardConnection *connection =
> > midgard_connect("my-midgard-database", &error);
>
> I hoped that I will propose it ;)
> Almost no more no less and +1000 from me.

Cool. :-)

> Briefly I would like to initialize configuration and log handler before I will
> initialize connection.
> And for example I would like to change runtime loglevel so midgard_config object
> could be helpfull here. But on the other hand I am not quite sure as we need some
> persistant ( application's configuration , let's say vhost ) and runtime's one ( let's
> say request ).

I'd prefer if we didn't have any specific log handlers or logging
configuration in midgard-core. Just use the normal
g_{debug,message,warning} macros for logging within midgard-core (the
core should never have critical or fatal errors) and let the
application define a log handler for the "midgard-core" domain if it
wants to. For example midgard-apache may want to define a log handler
that writes to the Apache error log while a Java application might use
log4j for logging. It's inconsistent if midgard-core would write a
separate log file.

> Another question is , if we need change midgard connection data while changing
> its configuration? Rather silly question as there should be another instance, but I
> prefer to ask just in case.

It's always good to clarify. I agree that it's better to start another
connection if configuration changes are needed. No need to complicate
the API unnecessarily.

> And another one, how to handle MidgardPerson while config is hidden in connection?

What do you mean? Which MidgardPerson?

> I would like to vote for midgard_config passed as connection argument and gerror
> as another one.

How about if we allowed both a name string and a full config object as
alternative connection options. The preferred interface would be to
use the higher level function with a database configuration name, but
a config object could also be used to allow more flexibility for
applications that need it:

------------------------------------------------------------

/**
 * Opens a connection to the named Midgard database. The configuration file
 * for the named database is read from the standard configuration directories
 * and used as the configuration for the created connection.
 *
 * If the named database configuration can not be read or the connection fails,
 * then \c NULL is returned and an error message is written to the optional
 * \c error argument.
 *
 * \param[in]  name  name of the Midgard database configuration
 * \param[out] error placeholder for an error message, or \c NULL
 * \return Midgard connection, or \c NULL
 */
MidgardConnection *midgard_connection_open(const char *name, GError **error);

/**
 * Opens a Midgard connection with the given configuration. If the connection
 * fails, then \c NULL is returned and an error message is written to the
 * optional \c error argument.
 *
 * \param[in]  config Midgard configuration object
 * \param[out] error  placeholder for an error message, or \c NULL
 * \return Midgard connection, or \c NULL
 */
MidgardConnection *midgard_connection_open_config(
        MidgardConfig *config, GError **error);

------------------------------------------------------------

The configuration API should support both parsing a configuration file
and creating the config object from scratch:

------------------------------------------------------------

/**
 * Creates an empty Midgard configuration object. The application
 * can fill in the configuration parameters before passing the
 * config instance to midgard_connection_open_config().
 *
 * \return empty Midgard configuration object
 */
MidgardConfig *midgard_config_new(void);

/**
 * Reads the given Midgard configuration file and returns the parsed
 * Midgard configuration object. The application can inspect or modify
 * the config instance before passing it to midgard_connection_open_config().
 *
 * If the configuration file can not be read or parsed, then \c NULL is
 * returned and an error message is written to the optional \c error argument.
 *
 * \param[in]  filename the configuration file name
 * \param[out] error    placeholdef for an error message, or \c NULL
 * \return parsed Midgard configuration object, or \c NULL
 */
MidgardConfig *midgard_config_read(const char *filename, GError **error);

------------------------------------------------------------

Note also that IMHO the MidgardConfig API seems a bit unnecessary. We
could just replace it with GKeyFile and get the exact same
functionality.

BR,

Jukka Zitting

--
Yukatan - http://yukatan.fi/ - [email protected]
Software craftmanship, JCR consulting, and Java development