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