[Freeipa-devel] [PATCH] 157 Add --delattr option to complement --setattr/--addattr

[Rob Crittenden]

Martin Kosek wrote:
> On Sat, 2011-11-05 at 13:43 +0100, Martin Kosek wrote:
>> On Fri, 2011-11-04 at 09:23 -0400, Rob Crittenden wrote:
>>> Martin Kosek wrote:
>>>> Add a --delattr option to round out multi-valued attribute manipulation.
>>>> The new option is be available for all LDAPUpdate based commands.
>>>>
>>>> --delattr is evaluated last, it can remove any value present either
>>>> in --addattr/--setattr options or stored in LDAP.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/1929
>>>
>>> Should --delattr raise an error if the value doesn't exist?
>>>
>>> I think it probably should.
>>>
>>> rob
>>
>> You are right, it would be more expected behavior. I fixed that. In the
>> process of implementing the change I found that current --*attr
>> processing is not good, so I refactored it completely to one function
>> available for all baseldap.py commands.
>>
>> In the process I found out that we don't have any common class for all
>> baseldap.py commands and the result is BaseLDAPCommand which can now
>> handle attribute processing and provide it to other LDAP commands.
>>
>> And I also fixed one group unit test. Now all unit tests were OK.
>>
>> Martin
>
> I rebased the patch (API.txt format was changed) and tested that it
> still works ok.
>
> Martin

ACK but some of the comments need to be cleaned up before pushing. It 
will also require a minor rebase in frontend.py.

process_attr_options() should probably read:

Process all --setattr, --addattr, and --delattr options and add the 
resulting value to the list of attributes. --setattr is processed first,
then --addattr and finally --delattr.

When --setattr is not used then the original LDAP object is looked up 
(of course, not when dn is None) and the changes are applied to old 
object values.

...
AttrValueNotFound exception may be raised when an attribute value was 
not found either by --setattr and --addattr nor in existing LDAP object.

rob