[Freeipa-devel] [PATCHES] 0581-0582 ipalib.config: Only convert numeric values to float

[Fraser Tweedale]

On Fri, Jun 13, 2014 at 02:12:41PM +0200, Petr Viktorin wrote:
> First patch: minor fix in env loading
> 
> Second patch:
> 
> When api.env is loaded, strings that "look like" floats get auto-converted
> to floats. This is wrong, as the conversion can lose precision, which
> matters for the new api_version.
> 
> Here's a patch that disables this conversion, and fixes places that the
> disabling might break.
> 
> -- 
> Petr³

Changes look fine, apply fine, and ipalib tests pass.  ACK to both.

Fraser

> From 47b6db3e91a16b6598c655edf17d1533b5e3dcc8 Mon Sep 17 00:00:00 2001
> From: Petr Viktorin <pviktori at redhat.com>
> Date: Fri, 13 Jun 2014 12:40:32 +0200
> Subject: [PATCH] ipalib.config: Only convert basedn to DN
> 
> The current code would convert values to DN if the key was
> a substring of 'basedn', e.g. 'base' or 'sed'.
> 
> Only convert if we're actually dealing with 'basedn'.
> ---
>  ipalib/config.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ipalib/config.py b/ipalib/config.py
> index f86c0a5ea3885d2bf89712f91b0c0705dceedd32..709e067416046b2c5174554043be87fa27042bf9 100644
> --- a/ipalib/config.py
> +++ b/ipalib/config.py
> @@ -257,7 +257,7 @@ def __setitem__(self, key, value):
>                  value = m[value]
>              elif value.isdigit():
>                  value = int(value)
> -            elif key in ('basedn'):
> +            elif key == 'basedn':
>                  value = DN(value)
>              else:
>                  try:
> -- 
> 1.9.0
> 

> From bb3d1e766140b0ed1cbf37562080070cf46cab1e Mon Sep 17 00:00:00 2001
> From: Petr Viktorin <pviktori at redhat.com>
> Date: Fri, 13 Jun 2014 12:47:48 +0200
> Subject: [PATCH] ipalib.config: Don't autoconvert values to float
> 
> When api.env is loaded, strings that "look like" floats got
> auto-converted to floats.
> This is wrong, as the conversion to float can lose precision.
> Case in point: the api_version (e.g. '2.88') should never be
> interpreted as float.
> 
> Do not automatically convert to float.
> 
> We have two numeric options: startup_timeout and wait_for_dns.
> wait_for_dns is already converted to int when used in the code.
> Convert startup_timeout to float explicitly when used, so
> configuration that specified it with a decimal point continues
> to work.
> ---
>  ipalib/config.py                       | 5 -----
>  ipapython/ipautil.py                   | 2 ++
>  ipapython/platform/fedora16/service.py | 2 +-
>  ipatests/test_ipalib/test_config.py    | 5 ++---
>  4 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/ipalib/config.py b/ipalib/config.py
> index 709e067416046b2c5174554043be87fa27042bf9..b12cfd32184edf964353fda9304dbb5149eb525f 100644
> --- a/ipalib/config.py
> +++ b/ipalib/config.py
> @@ -259,11 +259,6 @@ def __setitem__(self, key, value):
>                  value = int(value)
>              elif key == 'basedn':
>                  value = DN(value)
> -            else:
> -                try:
> -                    value = float(value)
> -                except (TypeError, ValueError):
> -                    pass
>          assert type(value) in (unicode, int, float, bool, NoneType, DN)
>          object.__setattr__(self, key, value)
>          self.__d[key] = value
> diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
> index 844dbb68792c483552149be7ae442a1e40eb9626..d95983b208f47ff42dfc254248e2f4f03d372bff 100644
> --- a/ipapython/ipautil.py
> +++ b/ipapython/ipautil.py
> @@ -1135,6 +1135,7 @@ def wait_for_open_ports(host, ports, timeout=0):
>      in seconds may be specified to limit the wait. If the timeout is
>      exceeded, socket.timeout exception is raised.
>      """
> +    timeout = float(timeout)
>      if not isinstance(ports, (tuple, list)):
>          ports = [ports]
>  
> @@ -1156,6 +1157,7 @@ def wait_for_open_socket(socket_name, timeout=0):
>      Wait until the specified socket on the local host is open. Timeout
>      in seconds may be specified to limit the wait.
>      """
> +    timeout = float(timeout)
>      op_timeout = time.time() + timeout
>  
>      while True:
> diff --git a/ipapython/platform/fedora16/service.py b/ipapython/platform/fedora16/service.py
> index 41c241ae5c31df56544b5b2bebd71c5ef109dd6e..86403d82583ed1e70044ce788d7ead7a5f3544a1 100644
> --- a/ipapython/platform/fedora16/service.py
> +++ b/ipapython/platform/fedora16/service.py
> @@ -152,7 +152,7 @@ def __wait_until_running(self):
>                  'The httpd proxy is not installed, wait on local port')
>              use_proxy = False
>          root_logger.debug('Waiting until the CA is running')
> -        timeout = api.env.startup_timeout
> +        timeout = float(api.env.startup_timeout)
>          op_timeout = time.time() + timeout
>          while time.time() < op_timeout:
>              try:
> diff --git a/ipatests/test_ipalib/test_config.py b/ipatests/test_ipalib/test_config.py
> index f896b893601c3ce85213cac39338095d7ac946f7..e04dd953074342f09b0ca4ca6dbea37eb37aaf48 100644
> --- a/ipatests/test_ipalib/test_config.py
> +++ b/ipatests/test_ipalib/test_config.py
> @@ -43,8 +43,7 @@
>      ('trailing_whitespace', u' value  ', u'value'),
>      ('an_int', 42, 42),
>      ('int_repr', ' 42 ', 42),
> -    ('a_float', 3.14, 3.14),
> -    ('float_repr', ' 3.14 ', 3.14),
> +    ('not_a_float', '3.14', u'3.14'),
>      ('true', True, True),
>      ('true_repr', ' True ', True),
>      ('false', False, False),
> @@ -406,7 +405,7 @@ def test_merge_from_file(self):
>          assert o.yes is True
>          assert o.no is False
>          assert o.number == 42
> -        assert o.floating == 3.14
> +        assert o.floating == '3.14'
>  
>      def new(self, in_tree=False):
>          """
> -- 
> 1.9.0
> 

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