On Tue, 2012-03-27 at 16:42 +0200, Martin Kosek wrote: > On Tue, 2012-03-27 at 16:30 +0200, Jan Cholasta wrote: > > On 27.3.2012 16:00, Martin Kosek wrote: > > > 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. > > > > Added the check. > > > > > > > > 2) Patch 47.4 needs minor rebasing > > > > Done. > > > > > > > > Martin > > > > > > > Updated patches attached. > > > > Honza > > > > I think you squashed the change with an incorrect commit. The check > should be rather included in patch 44. > > Martin
I just noticed it breaks one unit test: ====================================================================== ERROR: Test the `ipalib.parameters.DefaultFrom.__init__` method. ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in runTest self.test(*self.arg) File "/home/mkosek/freeipa/tests/test_ipalib/test_parameters.py", line 52, in test_init o = self.cls(callback, *keys) File "/home/mkosek/freeipa/ipalib/parameters.py", line 201, in __init__ raise ValueError("callback: variable-length argument list not allowed") ValueError: callback: variable-length argument list not allowed ---------------------------------------------------------------------- I also think it would be useful to have one test specifically for this new check. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel