On Thu, 2012-03-15 at 14:57 +0100, Jan Cholasta wrote: > On 15.3.2012 14:20, Petr Viktorin wrote: > > On 03/15/2012 12:05 PM, Jan Cholasta wrote: > >> On 15.3.2012 11:36, Jan Cholasta wrote: > >>> (this is a continuation of > >>> <http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html>) > >>> > >>> > >>> > >>> > >>> Hi, > >>> > >>> the attached patches fix <https://fedorahosted.org/freeipa/ticket/1847> > >>> and <https://fedorahosted.org/freeipa/ticket/2245>: > >>> > >>> [PATCH] Fix the procedure for getting default values of command > >>> parameters. > >>> > >>> The parameters used in default_from of other parameters are now properly > >>> validated before the default_from is called. > >>> > >>> [PATCH] Change parameters to use only default_from for dynamic default > >>> values. > >>> > >>> Replace all occurences of create_default with equivalent default_from > >>> and remove create_default from the framework. This is needed for proper > >>> parameter validation, as there is no way to tell which parameters to > >>> validate prior to calling create_default, because create_default does > >>> not provide information about which parameters are used for generating > >>> the default value. > >>> > >>> Honza > >>> > >> > >> Forgot to remove one FIXME bit in dns.py. Update patch attached. > >> > >> Honza > > > > > > > diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py > > > index a10960a..61c645d 100644 > > > --- a/ipalib/plugins/dns.py > > > +++ b/ipalib/plugins/dns.py > > > @@ -1528,7 +1528,7 @@ class dnszone(LDAPObject): > > > label=_('SOA serial'), > > > doc=_('SOA record serial number'), > > > minvalue=1, > > > - create_default=_create_zone_serial, > > > + default_from=_create_zone_serial, > > > autofill=True, > > > ), > > > Int('idnssoarefresh', > > > diff --git a/ipalib/plugins/passwd.py b/ipalib/plugins/passwd.py > > > index b26f7e9..9bee314 100644 > > > --- a/ipalib/plugins/passwd.py > > > +++ b/ipalib/plugins/passwd.py > > > @@ -69,7 +69,7 @@ class passwd(Command): > > > label=_('User name'), > > > primary_key=True, > > > autofill=True, > > > - create_default=lambda **kw: util.get_current_principal(), > > > + default_from=lambda: util.get_current_principal(), > > > normalizer=lambda value: normalize_principal(value), > > > ), > > > Password('password', > > > > > > This is just a minor nitpick, but I'd like to know if there's a reason > > behind it: why are you sometimes using lambda and sometimes not? > > I use lambda as a protective measure against accidents caused by adding > optional arguments to the functions used. _create_zone_serial is an > exception to that rule, because it is private to the dns plugin. > > > > > The patch works well here, but I think I'm not the one to ack it. > > > > Honza >
The patch looks OK, I found just minor issues. 1) We may want to add some check for wildcards (**kw) in default_from, I guess it would mess with your dependency solver. Some nice error would warn developers that they are doing something bad. 2) Patch 47.4 needs minor rebasing Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
