Re: MidgardConfigFile

Jukka Zitting <[email protected]> wrote:

> Hi,

Hi,
 
> > Yes , this is proposal for midgard connection API.
> > Not for configuration because midgard *never* had any configuration handler or API.
> > midgard struct is no configuration handler. If you want to make it configuration and
> > connection handler you need to change API anyway.
> 
> But that doesn't mean that we shouldn't discuss the change, or that
> you wouldn't need to document how the new API works!

Yes, but I am not going to change midgard connection right now.
The point of my example was that we can ( at last ) use one ( and only one ) 
uniform configuration handler.
This is configuration file handler , not runtime configuration one like *mgd.
 
> > > I figured as much, but how do you specify which database configuration
> > > to use? Your code sample contains no connection/database/configuration
> > > file name...
> >
> > Currently dbname, dbuser and password are public , so it depends what property
> > should be private or how should be accessible by internal method.
> 
> It doesn't really matter how the values are handled internally. I just
> want to know *how* I should specify them.

Yes, it matters. You do not want to create public , globally available config struct.
 
> > GObject *connection = g_object_new(MIDGARD_TYPE_CONNECTION, NULL);
> > GObject *config = g_object_new(MIDGARD_TYPE_CONFIG, NULL);
> >
> > midgard_connection_init(connection, config); /* returns TRUE or FALSE */
> 
> Yes, but where do I specify which database to connect to? Is there a
> more verbose connection error message available somewhere if
> midgard_connection_init() returns FALSE?

You define database name in configuration file , so:

GObject *config = g_object_new(MIDGARD_TYPE_CONFIG, NULL);
midgard_config_read_file(config, "myconfig");
midgard_connection_init(connection, config);

If we can merge current midgard.c and config.c in one file , then after failure 
g_error message is used to specify database name error.
 
> > Jukka, I am far from proposing MIdgard connections handlers and API now.
> > What I wrote is the fact that we have ( at least ) *one* , uniform configuration handler.
> 
> I'm not talking about any all-reaching API framework. Just documenting
> the couple of functions and method you mentioned. Even
> midgard_config_read_file() is a configuration API, just a small one.

We should probably run doxygen with HEAD header files. I write as much "doxygen 
ready" docs as possible ATM. 

> > And any language binding for that will be piece of cake when MidgardConfig
> > is an GObject.
> >
> > What is better in Your opinion?
> 
> Why not just use GKeyFile? I think it matches our needs exactly.

How much work do we need to make php and java bindings then?
And midgard-apache one?
 
> > gboolean midgard_config_read_file(MidgardConfig *object, const gchar *filename);
> >
> > Should I elaborate about this?
> > I do not see any reason do add another configuratin init method. Add it, if you need.
> 
> I have a perfectly valid use case: A desktop application for
> administering Midgard databases should allow a user either to choose
> from one of the existing Midgard configurations (in /etc/midgard) or
> to manually specify custom configuration.
> 
> Should I use the following for manual configuration?
> 
>     GObject *config = g_object_new(MIDGARD_TYPE_CONFIG, NULL);
>     g_object_set(config, "dbname", "foo", NULL);
>     g_object_set(config, "dbuser", "bar", NULL);
>     ...
> 
> Should I expect the "dbname", etc. property names to change at some point?

If user selected configuration file then you create new object instance:

$config = new MidgardConfig();
$config->read_file("userfile");

print " You selected '$config->dbname' database"; 

After read_file method was called , all MidgardConfig properties are set.
Class has empty setter , so properties are only readable.

> > > GObject is generally only useful for language bindings when you're
> > > handling generic objects that could have any number of different
> > > properties, signals, etc. I don't think that a Midgard connection or
> > > configuration object would much need things like this.
> >
> > Yes, it needs.
> 
> For what?

Because time needed to write good structure management is the same as time needed 
to write GObject management. 
 
Let's look at MidgardConfig again.

What's the difference between:
midgard_config_get_blobdir(struct MidgardConfig *config);
and 
g_object_get(config, "blobdir", &blobdir, NULL); ?

The time needed to write function is almost the same as needed to write property getter.
If you have generic gobject2language code which sets properties , you need almost 0 time 
to make language binding for that object.

Let's say we have 100 different configuration directives and we want to make API functions
for this configuration ( some internal structure ). We need functions like mgd_sitegroup, 
mgd_lang , etc etc etc. 100 functions which allow us to get members from private structure.
With GObject you still use g_object_get instead of 100 different functions.

> > > > Probably 'config = new MidgardConfig' seems sensible , right?
> > >
> > > What's wrong with using a normal struct for that?

I wrote above. Needed work is the same , but future development is much easier.

Now we have pleeeeeeenty of internal or semi-internal and midgard-php ( made in sources ) 
mgd_something functions. I loose myself many times when I need to get some data especially 
in midgard-apache or midgard-php. What was that? dcfg->blobdir? mgd->blobdir? mgd_blobdir(mgd) ?

> > Which normal one? Point me to that good one exisitng in legacy code. Please do this.
> 
> We don't have one at the moment, but I think creating one would have
> been a lot easier and less error-prone than setting up a full GObject
> type for it. I'm just wondering if you had some special reason for
> doing that?

Yes, I wrote above. I do not want to write 100 API functions for 100 config struct members.
 
> > > I really wouldn't expect to use more than 10-20 lines with the
> > > configuration/connection API in a language binding in any case. The
> > > API is seriously broken if it requires more.
> >
> > So the API will be very good now if you shouldn't use more than 10 lines of code.
> 
> Yes. I have nothing against using GObjects as the API as such. It
> makes for just about as much code on the client side as any other
> choise. I was just worried about the fact that the implementation of a
> GObject API requires a lot more work than a simpler approach.

As I wrote above :) How many time do you need to write 20 API functions + documentation? 
Isn't that time exactly the same as writing another 20 property getters?
 
> > > Are you asking me to propose a configuration/connection API for the
> > > core?
> >
> > Keep in mind:
> >
> >         * must be backward compatible
> >         * must be available in 1.8
> >         * available in 1.9 without many changes
> >         * available for midgard-php with php-cli
> >         * available for apache-php-midgard
> >         * available for any command line
> >         * available for midgard-java
> 
> I'd propose something like this:
> 
>     /**
>      * Connects to a Midgard database. The connection configuration is given
>      * as a GKeyFile with the following settings:
>      *
>      * [name]
>      * dbname=database name
>      * dbuser=database username
>      * ...
>      *
>      * The configuration can contain settings for multiple Midgard
> databases. The
>      * \c name parameter is used to select the database to connect to.
>      *
>      * Connection errors are reported through the optional \c error parameter.
>      *
>      * The returned connection handle should be closed with
> midgard_connection_close()
>      * when it is no longer used.
>      *
>      * \param[in]  config connection configuration
>      * \param[in]  name name of the connection within the configuration
>      * \param[out] error error reporting variable
>      * \return Midgard connection, or \c NULL
>      */
>     MidgardConnection *midgard_connection_new(
>             GKeyFile *config, const gchar *name, GError **error);

1. How much time do we need to set mgd and dcfg members from GKeyFile ?
2. How many changes must we make to set GError everywhere?
3. Can we use GError as member of internal structure like midgard one 
and accesible through some API function or object method?
 
> The GKeyFile configuration can be loaded either from the standard
> configuration locations as "midgard.conf" with
> g_key_file_load_from_data_dirs(), from a custom configuration file
> with g_key_file_load_from_file(), or created manually with
> g_key_file_set_value().

My MidgardConnection was only example. Point was to separate configuration handler 
from connection's one. 

And again I will ask you , how much time do you need to make midgard-java binding using GKeyFile?
 
> > > I'd actually be happy to do that, but I'm somewhat discouraged
> > > by what happened to the MgdSchema API proposals I made last spring.
> >
> > Midgard2 API proposal? We should wait a bit. We just released 1.8alpha1.
> 
> You said much the same thing when I first proposed the API in mRFC
> 0016 more than ten months ago! When is the right time?

After 1.8 is out. 
Your  API proposal is very good , but not practical.

For example in midgard-php I should abuse g_type_from_name to use this API, 
and there is no proposal for midgard like struct handling.

For example:

MidgardObject *midgard_object_find(GType type, guint id);

How should I define mgd struct and thus connection handler for this?
This function returns object so I can not set this as property value or 
method parameter.

Another very important issue is:
We have @#!$%#%@# exception called midgard+apache+php.
What is good for this midgard extension is bad for midgard-java , and what 
is bad for midgard-java is good for midgard-php.

I have almost 0 feedback from you about midgard-java bindings problem.

> > Example please.
> 
> From the top of my head:
> 
> * The excessive use of GHashTables and typecasts

Yes, MgdSchema is "abuser" of hash tables. 

> * The need for custom datatypes for simple things like strings

You mean midgard_hash_string_new ? 

> * Very long source files, for example types.c is over 2500 lines

Should I create 5 files , 500 lines each? :)
What I would like to see is mgdschema.c file which reads schema and creates 
new GTypes, and type.c as the file where object methods are defined ( those fully public ). 

> * Duplicate functionality, for example the midgard_object_list_... vs. the QB

Yes and not. midgard_object_list and list_childs are only shortcuts for QB , 
and thus should be very helpfull in any object browsers like or administration UIs.
 
Piotras