[Freeipa-devel] [PATCH] 241 Fix precallback validators in DNS plugin

[Martin Kosek]

On Thu, 2012-03-22 at 17:19 +0100, Petr Viktorin wrote:
> On 03/22/2012 04:53 PM, Martin Kosek wrote:
> > On Thu, 2012-03-22 at 16:30 +0100, Petr Viktorin wrote:
> >> On 03/21/2012 01:51 PM, Martin Kosek wrote:
> >>> DNS plugin contains several RR type record validators run in
> >>> pre_callback which cannot be used as standard param validator
> >>> as it needs more data and resources that standard validators
> >>> provide. However, the precallback validators are not run for
> >>> DNS records created by new structured options and thus an invalid
> >>> value may slip in.
> >>>
> >>> This patch moves the execution of these precallback validators
> >>> _after_ the processing of structured DNS options. It also cleans
> >>> them up a little and makes them more robust.
> >>>
> >>> https://fedorahosted.org/freeipa/ticket/2550
> >>>
> >>
> >> Functionally, the patch works fine.
> >> Since you're cleaning up, I have some nitpicks, but ACK if you disagree.
> >>
> >>
> >> Consider if instead of:
> >>
> >> rtype_cb = '_%s_pre_callback' % rtype
> >> if hasattr(self.obj, rtype_cb):
> >>       getattr(self.obj, rtype_cb)(ldap, dn, entry_attrs, *keys, **options)
> >>
> >> this wouldn't be more readable:
> >>
> >> rtype_cb = getattr(self.obj, '_%s_pre_callback' % rtype, None)
> >> if rtype_cb:
> >>       rtype_cb(ldap, dn, entry_attrs, *keys, **options)
> >>
> >> and since the block appears twice now, make it a method.
> >>
> >> Also, since is_ns_rec_resolvable raises an exception rather than
> >> returning a bool, it should probably be renamed to something like
> >> check_ns_rec_resolvable.
> >>
> >
> > These are good ideas, please check the attached patch.
> >
> > Martin
> 
> ACK
> 

Pushed to master, ipa-2-2.

Martin