[Freeipa-devel] [PATCH] Catch errors.EmptyModlist in b_ldap

[Rob Crittenden]

Jakub Hrozek wrote:
>  On Fri, 2009-02-13 at 13:11 -0500, Rob Crittenden wrote: 
>> Jakub Hrozek wrote:
>>> I think we shouldn't let an EmptyModlist exception propagate all the way
>>> up to ldap.update(). The method is usually used in fronted plugins, like
>>> classes that derive from crud.Mod, where modifying an record to the same
>>> attributes can be done pretty easily, so all the crud.Mod plugins would
>>> have to catch that exception anyway.
>>>
>>> The attached patch modifies ldap.update to return None if no
>>> modification was done.
>> I think this is ok but I want to ponder it a little. How do you plan to 
>> alert the user that nothing changed? In other words, if you have to do a 
>> test against None to see if an error was returned, why not just catch 
>> the exception? 
>  
> I was using the code like this:
> 
>      class object_mod(crud.Mod):
>         def execute():
>            # do stuff
>            return ldap.update(dn, **modkw)
> 
>         def output_for_cli(self, textui, result, key, **options):
>            if result:
>               textui.print_entry(result)
>            else:
>               textui.print_plain("No modification")
> 
> Before, I caught the exception, set the return value to None and used
> the same output_for_cli()..so I figured that if I'm putting this
> try..except around pretty much every ldap.update() I might as well move
> it into the method..while retaining the "nothing changed" information by
> using the return value.
> 
> On the other hand, the documentation[1] says that ldap.update() should
> return None when no such entry exists..so maybe raising the exception is
> the right thing to do. I just think that the exception should never get
> all the way to the user as it's trivially invoked (call two
> modifications with the same parameters)
> 
>> Or should we use a different exception, one that hides more of the details?
>>
>> rob
>  
> OK, what about a PublicError-derived exception..that would be caught in
> the marshalled_dispatch() in a similar way NotFound or DuplicateEntry
> are handled? (Hope this is not a dumb proposal..I must say the
> difference between errors and errors2 isn't all that clear to me..)
>

This sounds ok.

I think Jason has plans to unify these again. IIRC the original 
separation was due to Public/Private errors.

rob