On Wed, Sep 07, 2016 at 08:45:18AM +0200, Lukas Slebodnik wrote:
On (05/09/16 16:07), Jakub Hrozek wrote:
>On Mon, Sep 05, 2016 at 03:32:48PM +0200, Lukas Slebodnik wrote:
>> On (05/09/16 15:24), Jakub Hrozek wrote:
>> >On Mon, Sep 05, 2016 at 02:31:31PM +0200, Fabiano FidĂȘncio wrote:
>> >> On Mon, Sep 5, 2016 at 11:59 AM, Fabiano FidĂȘncio
<fidencio(a)redhat.com> wrote:
>> >> > Petr,
>> >> >
>> >> > I went through your patches and in general they look good to me.
>> >> > However, I haven't done any tests yet with your patches (and
I'll do
>> >> > it after lunch).
>> >>
>> >> I've done some tests and I've been able to see the ldif changes
in the
>> >> domain log. So, I assume it's working.
>> >> For sure it's a good improvement! Would be worth to link some
>> >> documentation about ldiff as it may be confusing for someone who is
>> >> not used to it.
>> >>
>> >> I'll wait for a new version of the patches and go through them
again.
>> >>
>> >> I really would like to have someone's else opinion on this series.
>> >
>> >I quickly scrolled through the patches and the primary thing I don't
>> >understand is why are the wrappers used only in sysdb? I think we should
>> >just use them everywhere..
>> I do not like wrappers.
>> We should not log ldif by default.
>
>That's why there is a separate log level, you need to turn these on
>separately (yes, logging LDIFs by default would be too much..)
>
Even though it is a separate debug level users still might
enable them by a chance.
How, except reading the code? This new debug level is not documented
anywhere and even starts off at a different base so neither debug_level=10
nor debug_level=0xFFF0 will trigger this.
IMH0 it will be confusing for them.
There are many users which are confused when try to analyze
sudo logs. They can see some "LDAP like" filters which
are used for internal searching. Users think we use wrong attribute
due to sudoRule -> sudoRole.
really? Someone who will discover a magic constant on SSSD will then be
confused by more logging?
btw what if this extra debugging was controlled by an environment
variable, do you think then it would be safer?
Or if we prepend the LDIF with something like "SSSD cache modification
message" ?
These wrappers should not be used in sssd upstream.
They can be prepared for debugging purposes in development process.
...which means we will have to rebase the patches, build the correct
version, pass it on to the person and care about correct versioning...
Sorry, I just don't agree with any of the arguments, but I'm curious
to hear more.