On 09/07/2016 09:53 AM, Jakub Hrozek wrote:
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 <fiden...@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.
Thanks, Jakub.
Lukas, I am afraid I am not able to imagine that we have some
wrappers which is not used in code but software engineers use them
for their daily job. It sounds like obstacle for me.
I understand that we have to be care of logs... but this high debug
level isn't mentioned in man page. So if user will use it it will be
accident anyway.
Regards
--
Petr^4 Čech
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
