...
1)
+        if options.unattended:
+            for ip in ip_addresses:
+                if search_reverse_zones and find_reverse_zone(str(ip)):
+                    # reverse zone is already in LDAP
+                    continue
+                for rz in ret_reverse_zones:
+                    if verify_reverse_zone(rz, ip):
+                        # reverse zone was entered by user
+                        break
+                else:
+                    rz = get_reverse_zone_default(str(ip))
+                    ret_reverse_zones.append(rz)
+        elif options.reverse_zones or create_reverse():
+            for ip in ip_addresses:
+                if search_reverse_zones and find_reverse_zone(str(ip)):
+                    # reverse zone is already in LDAP
+                    continue
+                for rz in ret_reverse_zones:
+                    if verify_reverse_zone(rz, ip):
+                        # reverse zone was entered by user
+                        break
+                else:
+                    rz = get_reverse_zone_default(str(ip))
+                    rz = read_reverse_zone(rz, str(ip))
+                    ret_reverse_zones.append(rz)
+        else:
+            options.no_reverse = True
+            ret_reverse_zones = []

You can make it shorter without duplications:

# this ifs can be in one line
if not options.unatended:
    if not options.reverse_zones
        if not create_reverse():
            options.no_reverse=True
            return []

for ip in ip_addresses:
    if search_reverse_zones and find_reverse_zone(str(ip)):
        # reverse zone is already in LDAP
        continue
    for rz in ret_reverse_zones:
        if verify_reverse_zone(rz, ip):
            # reverse zone was entered by user
            break
        else:
            rz = get_reverse_zone_default(str(ip))
            if not options.unattended:
                rz = read_reverse_zone(rz, str(ip))
            ret_reverse_zones.append(rz)


2)
Typo?     There is no IP address matching reverze_zone %s."
---------------------------------------------^^


3)
Would be nice to ask user to create new zones only if new zones are required. (If all required zones exist in LDAP, you ask user anyway)

4)
Ask framework gurus, if installutils module is better place for function above



--
Martin Basti

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

Reply via email to