Re: MidgardConfigFile

Hi,

On 1/5/06, Piotras <[email protected]> wrote:
> 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!

> > 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.

> 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?

> 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.

> 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.

> > > That's why I created MidgardConfig. Reading configuration from file is the first
> > > method. Do you want to add another one? Please do.
> > > Keep in mind that in every case you still hide critical data safe.
> >
> > I'd be glad to do that, but I cannot do that unless you specify the
> > default configuration API in with enough detail.
>
> excuse me?
> 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?

> > 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?

> > > Probably 'config = new MidgardConfig' seems sensible , right?
> >
> > What's wrong with using a normal struct for that?
>
> 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?

> > 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.

> > 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);

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().

> > 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?

Sooner or later we *must* start specifying the Midgard 2.0 API if we
ever want to get there. The current approach of just adding bits and
pieces here and there will end us up in the exact same situation we
are now with the old Midgard API. What's so bad about planning ahead?

> > I'm eagerly waiting for the moment we'll be dumping the legacy code so
> > I see no reason to complain about the mess there. I'm much more
> > worried about the fact that the core Midgard 2.0 functionality is
> > *already* showing signs of degeneration.
>
> Example please.

From the top of my head:

* The excessive use of GHashTables and typecasts
* The need for custom datatypes for simple things like strings
* Very long source files, for example types.c is over 2500 lines
* Duplicate functionality, for example the midgard_object_list_... vs. the QB

> > The fact that we're postponing the core cleanup to post-1.8 feels like
> > a worse decision the longer the 1.8 cycle seems to take. The longer we
> > support the current quirks the more backwards compatibility issues
> > we'll have once we get to start the cleanup.
>
> Do you blame me for Midgard1.x architecture and all these hacks made
> from *the beginning* ?

Of course not. There are quirks in the MgdSchema and Query Builder
code for example. And I'm not blaming you for them, I'm probably as
much (if not more) guilty of making them. My purpose is not to blame
anyone, but to raise an issue that I'm worried about.

BR,

Jukka Zitting

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