On Mon, 2011-09-26 at 17:13 +0200, Jan Cholasta wrote: > On 26.9.2011 14:18, Martin Kosek wrote: > > On Mon, 2011-09-26 at 11:26 +0200, Jan Cholasta wrote: > >> On 23.9.2011 09:00, Martin Kosek wrote: > >>> On Thu, 2011-09-22 at 14:02 +0200, Jan Cholasta wrote: > >>>> On 22.9.2011 13:27, Martin Kosek wrote: > >>>>> On Wed, 2011-09-21 at 15:31 -0400, Rob Crittenden wrote: > >>>>>> Jan Cholasta wrote: > >>>>>>> On 25.8.2011 18:21, Jan Cholasta wrote: > >>>>>>>> What this patch does: > >>>>>>>> > >>>>>>>> * Make sure arguments are validated and default values are filled in > >>>>>>>> before calling a command. > >>>>>>>> * Add new parameter flag "validate_search" to force validation on > >>>>>>>> search > >>>>>>>> arguments. > >>>>>>>> * Fix validation of IP network parameters in the DNS plugin. > >>>>>>>> > >>>>>>>> https://fedorahosted.org/freeipa/ticket/1627 > >>>>>>>> > >>>>>>>> Honza > >>>>>>>> > >>>>>>> > >>>>>>> Redone the patch and split it to 3 parts: > >>>>>> > >>>>>> I haven't tested these yet, these comments are just from reading the > >>>>>> patches. > >>>>>> > >>>>>>> > >>>>>>> * [PATCH 46] Add IP address and IP network parameter types > >>>>>>> Adds two new parameter types, IPAddress and IPNetwork (which replaces > >>>>>>> the validate_search flag, as it was just a hack). > >>>>>> > >>>>>> A param type looks like the way to go. There are other IPaddress > >>>>>> parameters, such in host, should this be expanded to cover that? > >>>>>> > >>>>>> Not sure about replacing List with Str types. It may make no difference > >>>>>> since I'm not sure how a List could be passed for some of these. Can > >>>>>> you > >>>>>> make sure there is no possibility that on the wire someone couldn't > >>>>>> pass > >>>>>> these as a list and actually do something reasonable in the server? I > >>>>>> suspect they can't but this is a rather major datatype change. > >>>>>> > >>>>>>> > >>>>>>> * [PATCH 44] Fix parameter validation > >>>>>>> Changes Command.get_default so that default_from parameters are > >>>>>>> validated before they are used to create the default value. > >>>>>> > >>>>>> Just a heads-up but this will conflict big time with my password patch. > >>>>>> > >>>>>> It throws away the ordering that the parameters had. This could impact > >>>>>> the order in which interactive prompting happens. Did you verify that > >>>>>> it > >>>>>> still works sanely? > >>>>>> > >>>>>>> * [PATCH 47] Remove create_default > >>>>>>> Removes create_default, as it does exactly the same thing as > >>>>>>> default_from, but without the advantage of knowing what parameters are > >>>>>>> used to create the default value. All uses of create_default are > >>>>>>> replaced by default_from with no arguments, because that's all > >>>>>>> create_default is currently used for in IPA. > >>>>>> > >>>>>> I'm not sure why you want to remove this, is it causing problems with > >>>>>> validation? > >>>>>> > >>>>>> In general the patch comments need more details. This patch e-mail has > >>>>>> more information on what each patch does than the patches themselves, > >>>>>> including lacking ticket info. > >>>>>> > >>>>>> rob > >>>>>> > >>>>> > >>>>> Hm, by the way I am not very excited about adopting this sort of changes > >>>>> to framework that close to the 6.2 rebase. Are we sure enough that this > >>>>> won't cause any collateral damage? > >>>>> > >>>>> If possible, I would prefer to integrate it to 3.0 branch so that we > >>>>> have enough time to validate it and fix bugs. > >>>>> > >>>>> Martin > >>>>> > >>>> > >>>> I concur. > >>>> > >>>> Honza > >>>> > >>> > >>> Ok. I created a Trac ticket for 3.0 branch to do this improvement. > >>> > >>> https://fedorahosted.org/freeipa/ticket/1847 > >>> > >>> As for 2.1.2 bugfix release, lets do just the "stupid" safe fix. > >>> > >>> Martin > >>> > >> > >> Stupid safe fix attached (obviously it's ipa-2-1 only). > >> > >> Honza > >> > > > > Works fine. Just one nit - I would suggest adding a comment what this > > hack does. Its not clear just from the code. > > > > Martin > > > > Added comments. > > Honza >
ACK. Pushed to ipa-2-1. The problem should be solved in a more complex way in ticket 1847. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel