On 09/12/2016 10:01 AM, Lukas Slebodnik wrote:
On (11/09/16 23:49), Jakub Hrozek wrote:
On Thu, Sep 08, 2016 at 12:56:08PM +0200, Lukas Slebodnik wrote:
Let me explain why wrappers are not good idea in production.
There was introduced new wrapper(#1991) for ldb_search
SSS_LDB_SEARCH. It should guarantee that ENONET will be
returned and not EOK + res->count == 0.

I found just a single usage of this wrapper since introducing
but many usage of ldb_search (I stopped counting after 10).
And there will be the same problem with newly introduced wrappers.
It's crystal clear that review does not help. Otherwise we would use
SSS_LDB_SEARCH everywhere.

That is a reason why I am fine with using wrappers just for a for development
but not for productions. Or try to propose some automatic way
how to simply log ldifs for *ALL* required ldb functions.
IMHO, it would be the best to implement it in libldb itself.

You have a point here (and I regret adding the ENOENT retval in general,
but the difference is that ldb_search wrapper changes /functionality/, this
just adds logging. So the only thing we would miss if we forget to use
the wrapper is the extra debugging. And in that case we would have to
build a new package and commit the messages to master, but that's no
different from missing debug messages in general.

Inconsistencies are bad. it does not matter wheter it's about ENOENT
or logging.

There's been quite a few cases where I wanted to see what exactly is
being added for example with duplicate member: attributes or a member
attribute that points nowhere or 'binary' attributes. These patches
would solve it nicely.
And for such few cases you can prepare custom build of sssd.

I am fine with first 3 patches but rest of patches (replacement
ldb_* with wrappers) have NACK or
a) you can find other way how to consistently log messages from ldb_ functions

BTW you will still be able to prepare test builds for debugging because
wrappers will be part of upstream (but will be ununsed by default.)

Hello Lukas,

I will be glad if we push first 3 patches now.

I am not happy we have no consensus for the other 4.
We could discuss if there is other way how to consistently
log messages from ldb_ functions on SSSD meeting.

Regards

--
Petr^4 Čech
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to