[PATCH] lib/ldb minor fixes

[Kamen Mazdrashki]

Hi Jelmer,

On Sat, Nov 15, 2014 at 8:26 PM, Jelmer Vernooij <jelmer at samba.org> wrote:

> On Sat, Nov 15, 2014 at 03:44:49AM +0100, Kamen Mazdrashki wrote:
> > >
> > > Look taken.
> > I can't find out how we could avoid name == NULL in python bindings.
> > PyLdbMessageElement by design allows optional parameters in constructor
> > (name = NULL)
> > Honestly said, I don't see what an empty PyLdbMessageElement could be
> used
> > for
> > (object interface is close to readonly as far as I can see)
> >
> > The only optoin I could see here is to change PyLdbMessageElement
> > constructor to require
> > name and elements to be non-optional.
> > This may affect lots of code though, because making 'elements' and 'name'
> > non-optional
> > and leaving 'flags' optional will require change in interface (and tests
> > and so on)
> >
> > <<including Jelmer as author for this code>>
> This is not one of the classes I am most proud of. :-) The order of the
> arguments as non-keyword arguments is kindof backwards. The main reason
> 'name' is last is so that it can be optional when you do dictionary-style
> assignments, e.g.:
>
> msg["foo"] = MessageElement([1,2,3], ldb.MOD_MODIFY)
>
> I like this one. In this case we don't need the 'name' at all. it is going
to be set
by the time we actually use this element right?


> For a while, I've been wanting to get rid of PyLdbMessageElement
> completely,
> and just allowing the user to use tuples, e.g.:
>
> msg["foo"] = ([1,2,3], ldb.MOD_MODIFY)
> msg.add("foo", [1,2,3], ldb.MOD_MODIFY)
>
> If you'd be interested in working on that, I'd happily review code.
>

I now recall you really wanted to implent it this way. I have forgotten
about this.
Honestly said, I like MessageElement() example more.
Although, if second one is defined strictly as Tuple of (List,Int) then it
won't make
much of difference for assignment imo.


> For the example mentioned above, I think it would make sense to allow
> name=NULL
> in a very select number of cases. By the time we call one of the ldb APIs
> and
> pass in a ldb_message, the name should be filled in though. We could add a
> check to see if there are any message elements that have name set to NULL
> before we
> pass ldb messages on to the C LDB API.
>
> I will go ahead and expeiment a bit with current approach, but with
MessageElement
without 'name' argument at all.
In my mind, 'name' make sence for MessageElement in context of message imo.

Cheers,
Kamen