[Freeipa-devel] [PATCHES] Improve framework parameter validation
Martin Kosek
mkosek at redhat.com
Tue Mar 27 14:42:09 UTC 2012
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
More information about the Freeipa-devel
mailing list