[Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

[David Kupka]

On 09/18/2014 06:34 PM, Martin Basti wrote:
> ...
> 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)
>

Thanks, I modified it bit different way to alse address recommendation 3).

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

Thanks, fixed.

>
> 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)
>

I added one more variable and ask only once.

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

Petr^3 said that it's ok to have it in bindinstance.py.

>

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-8-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 36230 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140923/15362f7c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-8-ipa-41-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 36214 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140923/15362f7c/attachment-0001.bin>