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

[Martin Kosek]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-241-2-fix-precallback-validators-in-dns-plugin.patch
Type: text/x-patch
Size: 8176 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120322/d57c0a20/attachment.bin>