opensubscriber
   Find in this group all groups
 
Unknown more information…

d : dev@midgard-project.org 3 March 2006 • 6:15AM -0500

Re: [midgard-dev] Fwd: [midgard-cvs] CVS update: /midgard/src/core/midgard/midgard/
by Piotras

REPLY TO AUTHOR
 
REPLY TO GROUP




> Hi,

Hi,

>> > Less code, less bugs... :-)
>>
>> Yes and not. We still need to allocate and free memory for structures.
>> With GObject we do not need to provide special API , g_object_new
>> and g_object_unref should be enough.
>
> But to properly support g_object_new() and g_object_unref(), you need
> to implement at least midgard_connection_get_type(),
> midgard_connection_class_init(), midgard_connection_init(),
> midgard_connection_finalize(), and midgard_connection_dispose().
> That's a reasonable overhead when you want to support a full class
> hierarchy like MgdSchema or need other GObject features, but IMO we
> need none of that for MidgardConnection.

Those must be written anyway. You need to initialize struct and free it anyway.
But this is still proposal , so I do not say +1 or -1.

>> > > extern void midgard_connection_reset(MidgardConnection *mgd);
>> >
>> > Why is this needed?
>>
>> This is very good question, and I ask myself about this too.
>>
>> 1. Internal members like sitegroup and lang can be "reset" in midgard-php's RINIT
>> instead of RSHUTDOWN. I spent some time to review legacy code and you must
>> say the same : this is chaotic. You can confuse yourself quite soon when you try
>> to understand midgard-core&midgard-php 1.x logic.
>> This function is proposed for midgard-php's RSHUTDOWN mainly.
>
> Yeah, the legacy stuff is really quite hairy. In any case I think it's
> cleaner if we close the connection in RSHUTDOWN and open a new one in
> RINIT. Persistent connections are probably better handled on a lower
> level.
>
>> 2. Once you call mgd_auth_midgard which sets person member as GObject , you need
>> to free it at request end. This function should be smart about this because the
>> same
>> internals are used for mgd_auth_midgard and Apache basic auth.
>
> One more reason for not having the reset() function. Just close the
> connection when the request ends and open a new one for the next
> request. No chance for accidentally leaking information between
> requests. Such a solution is also much more resilient against
> connection errors and other inconsistencies.
>
> If the connection overhead is considered too high, we can always use a
> connection pool to transparently hide the cost of opening new
> connections.

I am not sure if I follow you here. If you want to open and close connection
without a pool then we can forget about duplicating structures for rcfg.
If you want to use connection's pool then duplicated structures for rcfg are
*a must*.
You may relay on MidgardConnection in exorcist or any other app, but for
request time in midgard-php it may be quite useless.

>> > Why GValue? The more specific the API is the better. How about
>> > something like this:
>>
>> Because GValue has type , so I could start to use it in 1.8 branch
>> and change 0 lines of API  part in 1.9/2.0.
>
> What do you need the type for? To differentiate sitegroup ids from
> sitegroup guids (or names)? Wouldn't it be better if we just selected
> one of these options as the standard way to identify a sitegroup and
> designed the API accordingly.

That would be perfect , but I doubt we can change so many things before 1.8
is out. But anyway we can hide some "overhead" in this API now.

>> > > extern MidgardConfig *midgard_connection_config_get(MidgardConnection *mgd);
>> >
>> > I'd rather not have this. Is there any use case where the connection
>> > configuration is needed after the connection has been opened?
>>
>> Blobdir for example. Personally I do not want to get blobdir as part of
>> MidgardConnection API as this is simple as logically as Midgard 1.x API.
>
> A client should never need to access the blobdir setting directly.
> Instead we should have an API method for accessing attachments.
> Something like this:
>
>     GIOChannel *midgard_connection_get_attachment_data(
>             MidgardConnection *connection, MidgardObject *attachment_object);
>
> This way the client application doesn't need to know where the blobs
> are actually being stored.

This is what I really would like to avoid. It's not logically IMO.
It's not related to connection and forces us to make MidgardConnection as a
"wrapper" for configuration itself. And in a long run it will be something with long
listed API like it's now with all mgd_xxx in core.

If you close connection at RSHUTDOWN you need to reuse configuration again
in RINIT, so you need to handle it somehow. Or reopen configuration.

>> wait wait :) did you notice midgard_set instead of midgard_connection_set ?
>> Was it Chuck Norris or intuitive API part ? ;)
>> You know what I mean :)
>
> Nope, I'm quite serious.

I was reffering to midgard_set vs midgard_connection_set.

Probably this is too complicated for this purpose , but I would like to see it that
way:

Midgard
    |
    ---MidgardConnection
        |
        ---MidgardConfig

Then config and connection may be typical structures , while midgard can be an object
which provides access methods for configuration. Or connection may be replaced with
midgard object which can provide access to midgard internals and is easy for
bindings,
while connection handler is transparent hidden in midgard object internals.

All in all we never use connection handler, what we mostly need is to access
configuration or runtime data.

> The GLib logging framework only allows you to
> specify log handlers per logging domain, not per Midgard connection.
> Even the current default log handler hack in Midgard fails miserably
> if you want to simultaneously connect to two databases with different
> log configurations.

The same connection for two databases?

Piotras


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@midg...
For additional commands, e-mail: dev-help@midg...

Bookmark with:

Delicious   Digg   reddit   Facebook   StumbleUpon

Related Messages

opensubscriber is not affiliated with the authors of this message nor responsible for its content.