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 _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel