On 25/09/14 14:56, Petr Vobornik wrote:

Ticket: https://fedorahosted.org/freeipa/ticket/4149


5. You've removed 'idnssoamname' and 'force' from Web UI but
dnszone-add precallback still uses these params. What is the intended
purpose?
User should use modify dialog in webUI for zones.
Precallback fills default value for idnsmname from LDAP.
with --force there will be no validation of user specified soa mname

Issue with web ui is that it can't call dnszone-mod with --force option. Should be fixed separately.


Unrelated to this patch set:
b. Web UI doesn't have means to call dnszone-mod with --force option
I'm not sure what you mean, it didn't do that before my patches.

See #5


Updated patches attached


Review of new version:

All new issues are nitpicks. If somebody else thinks they are OK and if pspacek's functional tests pass then ACK.

Patch: 114: ACK
Patch: 115: ACK

Patch: 120

1) Why is there:
  `default=None,  # value will be added in precallback from ldap` ?

Static 'default' is by default `None`
You're right. I'll leave there only comment.


2) Wonder if RequirementError would be a better fit:
+                raise errors.ValidationError(
+                    name='name_server',
+                    error=_(u"is required"))
I tried it and ipa currently returns RequirementError, I will change it

Patch 121:
3)
+    if ns_hostname:
+        ns_hostname = normalize_zone(ns_hostname)
+        ns_hostname = unicode(ns_hostname)

     try:
         api.Command.dnszone_add(unicode(name),
-                                idnssoamname=unicode(ns_main),
+                                idnssoamname=ns_hostname,

If `ns_hostname` is '' then it will not be converted to unicode. I'm not sure if it can cause an issue.

ns_hostname values can be only None or DNSName instance, DNSName instance is always non_zero.

Patch 123: ACK
Patch 124: ACK
Patch 125: ACK


--
Martin Basti

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

Reply via email to