On Mon, 2011-11-28 at 23:24 -0500, Rob Crittenden wrote: > 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
I fixed the comment, it is now much more readable. I see you improved __convert_2_dict function (which is moved to baseldap.py in this patch) in your update plugin framework patch. This has been properly merged. Pushed to master. Martin _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
