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

--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to