[PATCH] SDAP: Remove user from cache for missing user in LDAP

[Lukas Slebodnik]

ehlo,

regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
because function sysdb_get_real_name overrode returned code from LDAP.

LS

[Michal Židek]

On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> ehlo,
>
> regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> because function sysdb_get_real_name overrode returned code from LDAP.
>
> LS

Can not be applied cleanly on current master.

Michal

[Lukas Slebodnik]

On (18/06/15 18:38), Michal Židek wrote:
> On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > ehlo,
> >
> > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > because function sysdb_get_real_name overrode returned code from LDAP.
> >
> > LS
>
> Can not be applied cleanly on current master.

works for me without any problem.

LS

[Michal Židek]

On 06/18/2015 06:40 PM, Lukas Slebodnik wrote:
> On (18/06/15 18:38), Michal Židek wrote:
> > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > because function sysdb_get_real_name overrode returned code from LDAP.
> > >
> > > LS
> >
> > Can not be applied cleanly on current master.
>
> works for me without any problem.
>
> LS

Sorry, my fault. I was trying to apply wrong patch.

Michal

[Michal Židek]

On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> ehlo,
>
> regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> because function sysdb_get_real_name overrode returned code from LDAP.
>
> LS

ACK.

CI link:
http://sssd-ci.duckdns.org/logs/job/17/89/summary.html

Michal

[Jakub Hrozek]

On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > ehlo,
> >
> > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > because function sysdb_get_real_name overrode returned code from LDAP.
> >
> > LS
>
> ACK.
>
> CI link:
> http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
>
> Michal

Thanks.

What about adding a default: to the switch? I know we currently can't
get any other value, but it would be better to error out in that case.

[Michal Židek]

On 06/19/2015 06:03 PM, Jakub Hrozek wrote:
> On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > because function sysdb_get_real_name overrode returned code from LDAP.
> > >
> > > LS
> >
> > ACK.
> >
> > CI link:
> > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> >
> > Michal
>
> Thanks.
>
> What about adding a default: to the switch? I know we currently can't
> get any other value, but it would be better to error out in that case.

I do not think the default label is necessary, the code will get there
only if sdap_ret is EOK or ENOENT (see few lines above the switch
if (ret && ret != ENOENT)).
But I am OK with adding it there if you think it will make the code
nicer.

Michal

[Jakub Hrozek]

On Fri, Jun 19, 2015 at 06:32:56PM +0200, Michal Židek wrote:
> On 06/19/2015 06:03 PM, Jakub Hrozek wrote:
> > On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > > because function sysdb_get_real_name overrode returned code from LDAP.
> > > >
> > > > LS
> > >
> > > ACK.
> > >
> > > CI link:
> > > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> > >
> > > Michal
> >
> > Thanks.
> >
> > What about adding a default: to the switch? I know we currently can't
> > get any other value, but it would be better to error out in that case.
>
> I do not think the default label is necessary, the code will get there
> only if sdap_ret is EOK or ENOENT (see few lines above the switch
> if (ret && ret != ENOENT)).

I know.
> But I am OK with adding it there if you think it will make the code
> nicer.

Not nicer, uglier, but safer :-)

I don't insist on the default, but please at least add a comment to try
to avoid us moving the code around somewhere where another value is possible.

[Michal Židek]

On 06/19/2015 06:39 PM, Jakub Hrozek wrote:
> On Fri, Jun 19, 2015 at 06:32:56PM +0200, Michal Židek wrote:
> > On 06/19/2015 06:03 PM, Jakub Hrozek wrote:
> > > On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > > > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > > > ehlo,
> > > > >
> > > > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > > > because function sysdb_get_real_name overrode returned code from LDAP.
> > > > >
> > > > > LS
> > > >
> > > > ACK.
> > > >
> > > > CI link:
> > > > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> > > >
> > > > Michal
> > >
> > > Thanks.
> > >
> > > What about adding a default: to the switch? I know we currently can't
> > > get any other value, but it would be better to error out in that case.
> >
> > I do not think the default label is necessary, the code will get there
> > only if sdap_ret is EOK or ENOENT (see few lines above the switch
> > if (ret && ret != ENOENT)).
>
> I know.
> > But I am OK with adding it there if you think it will make the code
> > nicer.
>
> Not nicer, uglier, but safer :-)
>
> I don't insist on the default, but please at least add a comment to try
> to avoid us moving the code around somewhere where another value is possible.

Good point. Lukas is already making new version :)

Michal

[Lukas Slebodnik]

On (19/06/15 18:03), Jakub Hrozek wrote:
> On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > because function sysdb_get_real_name overrode returned code from LDAP.
> > >
> > > LS
> >
> > ACK.
> >
> > CI link:
> > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> >
> > Michal
>
> Thanks.
>
> What about adding a default: to the switch? I know we currently can't
> get any other value, but it would be better to error out in that case.

Here you are.

LS

[Jakub Hrozek]

On Fri, Jun 19, 2015 at 06:44:25PM +0200, Lukas Slebodnik wrote:
> On (19/06/15 18:03), Jakub Hrozek wrote:
> > On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > > because function sysdb_get_real_name overrode returned code from LDAP.
> > > >
> > > > LS
> > >
> > > ACK.
> > >
> > > CI link:
> > > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> > >
> > > Michal
> >
> > Thanks.
> >
> > What about adding a default: to the switch? I know we currently can't
> > get any other value, but it would be better to error out in that case.
>
> Here you are.
>
> LS

Thanks, this is what I wanted.

ACK++

[Michal Židek]

On 06/19/2015 07:50 PM, Jakub Hrozek wrote:
> On Fri, Jun 19, 2015 at 06:44:25PM +0200, Lukas Slebodnik wrote:
> > On (19/06/15 18:03), Jakub Hrozek wrote:
> > > On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > > > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > > > ehlo,
> > > > >
> > > > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > > > because function sysdb_get_real_name overrode returned code from LDAP.
> > > > >
> > > > > LS
> > > >
> > > > ACK.
> > > >
> > > > CI link:
> > > > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> > > >
> > > > Michal
> > >
> > > Thanks.
> > >
> > > What about adding a default: to the switch? I know we currently can't
> > > get any other value, but it would be better to error out in that case.
> >
> > Here you are.
> >
> > LS
>
> Thanks, this is what I wanted.
>
> ACK++

Just for the record.

CI for the new version:
http://sssd-ci.duckdns.org/logs/job/17/94/summary.html

Tested and still works. I like the new version more. So
I need to give bigger ack than before:
     _    ____ _  __
    / \  / ___| |/ /
   / _ | |   | ' /
  / ___ \ |___| . \
/_/   _____|_|_\

Michal

[Jakub Hrozek]

On Sun, Jun 21, 2015 at 11:21:50PM +0200, Michal Židek wrote:
> On 06/19/2015 07:50 PM, Jakub Hrozek wrote:
> > On Fri, Jun 19, 2015 at 06:44:25PM +0200, Lukas Slebodnik wrote:
> > > On (19/06/15 18:03), Jakub Hrozek wrote:
> > > > On Fri, Jun 19, 2015 at 05:59:04PM +0200, Michal Židek wrote:
> > > > > On 06/18/2015 06:13 PM, Lukas Slebodnik wrote:
> > > > > > ehlo,
> > > > > >
> > > > > > regression was cuased by patch d0b7e5fcfca7d0db9e3d19be7b51f34d03d3d720
> > > > > > because function sysdb_get_real_name overrode returned code from LDAP.
> > > > > >
> > > > > > LS
> > > > >
> > > > > ACK.
> > > > >
> > > > > CI link:
> > > > > http://sssd-ci.duckdns.org/logs/job/17/89/summary.html
> > > > >
> > > > > Michal
> > > >
> > > > Thanks.
> > > >
> > > > What about adding a default: to the switch? I know we currently can't
> > > > get any other value, but it would be better to error out in that case.
> > >
> > > Here you are.
> > >
> > > LS
> >
> > Thanks, this is what I wanted.
> >
> > ACK++
>
> Just for the record.
>
> CI for the new version:
> http://sssd-ci.duckdns.org/logs/job/17/94/summary.html
>
> Tested and still works. I like the new version more. So
> I need to give bigger ack than before:
>     _    ____ _  __
>    / \  / ___| |/ /
>   / _ | |   | ' /
>  / ___ \ |___| . \
> /_/   _____|_|_\

* master: 9fc96a4a2b07b92585b02dba161ab1eb2dbdad98
* sssd-1-12: d0d6956c3748ba670448edd548b8edd37c5f5c1a