[Freeipa-devel] [PATCH] 0059-0063 Update DNSSEC attributes/record types

Petr Vobornik pvoborni at redhat.com
Fri Jun 20 13:30:03 UTC 2014


On 20.6.2014 14:35, Martin Basti wrote:
> On Thu, 2014-06-19 at 18:37 +0200, Martin Basti wrote:
>> On Fri, 2014-06-13 at 09:55 +0200, Martin Basti wrote:
>>> On Thu, 2014-06-12 at 16:20 +0200, Martin Basti wrote:
>>>> On Thu, 2014-06-12 at 13:17 +0200, Petr Vobornik wrote:
>>>>> On 9.6.2014 17:28, Martin Basti wrote:
>>>>>> Ticket: https://fedorahosted.org/freeipa/ticket/4328
>>>>>>
>>>>>> Petr please make the WebUI patch review (0062) :-)
>>>>>>
>>>>>> Patches attached.
>>>>>>
>>>>>
>>>>> Patch #0059: LGTM
>>>>>
>>>>> Patch #0060:
>>>>>
>>>>> 1. Please add `pattern_errmsg` to `salt` part of nsec3param. Otherwise
>>>>> you get general "Text does not match field pattern" error message in Web UI.
>>>>>
>>>> I will add the message.
>>>>
>>>>> 2. Could be in one if:
>>>>> +            if nsec3params is not None:
>>>>> +                if len(nsec3params) > 1:
>>>>>
>>>>> Maybe I'm missing something. But why does the dns plugin code use
>>>>> following all over the place:
>>>>>
>>>>>           try:
>>>>>               nsec3params = rrattrs['nsec3paramrecord']
>>>>>           except KeyError:
>>>>>               pass
>>>>>           else:
>>>>>               if nsec3params is not None:
>>>>>
>>>>> instead of:
>>>>>
>>>>>       nsec3params = rrattrs.get('nsec3paramrecord')
>>>>>       if nsec3params is not None:
>>>>>
>>>>> btw you use both patterns in the patch.
>>>> I will use shorten form, I wrote it in the same way as the other code in
>>>> the block was written, that's why.
>>>>
>>>>>
>>>>> Patch #0061: ACK
>>>>>
>>>>>
>>>>> Patch #0062:
>>>>>
>>>>> 3. Why are there the idnafsdbrec1 variables?
>>>>>
>>>> It was replace for NSEC records, which are not supported anymore.
>>>> Now I realize, there is wrong description, so the idnafsbrec1 variable
>>>> is not needed.
>>>> I will remove it.
>>>>
>>>>> 4. related to ^^:
>>>>> ./ipatests/test_xmlrpc/test_dns_plugin.py:199:33: E231 missing
>>>>> whitespace after ','
>>>>>
>>>>>
>>>>> Patch #0063: LGTM
>>>>> IDK if they work because I'm experiencing some weird issues with
>>>>> xmlrpc_tests in general.
>>>> I had troubles only with permission tests, but all DNS test worked fine for me.
>>>>
>>>> Thank you for review.
>>>>
>>> Updated patches attached.
>>>
>>
>> Rebased patches attached
>
> Updated patch attached, fixed missing update permission
>

ACK
-- 
Petr Vobornik




More information about the Freeipa-devel mailing list