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

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

Reply via email to