URL: https://github.com/freeipa/freeipa/pull/254
Author: tiran
 Title: #254: Replace LooseVersion with pkg_resource.parse_version
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/254/head:pr254
git checkout pr254
From a2a7d2c0d6b8b74d224e9c17b28efbc9d791f8a9 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Mon, 21 Nov 2016 10:24:17 +0100
Subject: [PATCH] Replace LooseVersion

pylint is having a hard time with distutils.version in tox's virtual
envs. virtualenv uses some tricks to provide a virtual distutils
package, pylint can't cope with.

https://github.com/PyCQA/pylint/issues/73 suggests to use pkg_resources
instead. pkg_resources' version parser has some more benefits, e.g. PEP
440 conformity. But pkg_resources.parse_version() is a heavy weight solution
with reduced functionality, e.g. no access to major version.

For API_VERSION and plugin version we can use a much simpler and faster
approach.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaclient/remote_plugins/compat.py         | 13 ++++++------
 ipalib/capabilities.py                     |  6 +++---
 ipalib/frontend.py                         | 27 +++++++++++++------------
 ipalib/plugable.py                         |  9 +++++----
 ipapython/ipautil.py                       | 32 ++++++++++++++++++++++++++++++
 ipaserver/install/krbinstance.py           |  6 +++---
 ipaserver/install/server/replicainstall.py | 10 +++++-----
 7 files changed, 68 insertions(+), 35 deletions(-)

diff --git a/ipaclient/remote_plugins/compat.py b/ipaclient/remote_plugins/compat.py
index 984eecd..c1ae635 100644
--- a/ipaclient/remote_plugins/compat.py
+++ b/ipaclient/remote_plugins/compat.py
@@ -2,7 +2,6 @@
 # Copyright (C) 2016  FreeIPA Contributors see COPYING for license
 #
 
-from distutils.version import LooseVersion
 import importlib
 import os
 import re
@@ -12,6 +11,7 @@
 
 from ipaclient.frontend import ClientCommand, ClientMethod
 from ipalib.frontend import Object
+from ipapython.ipautil import APIVersion
 
 if six.PY3:
     unicode = str
@@ -58,7 +58,7 @@ def get_package(server_info, client):
         server_info['version'] = server_version
         server_info.update_validity()
 
-    server_version = LooseVersion(server_version)
+    server_version = APIVersion(server_version)
 
     package_names = {}
     base_name = __name__.rpartition('.')[0]
@@ -66,15 +66,14 @@ def get_package(server_info, client):
     for name in os.listdir(base_dir):
         package_dir = os.path.join(base_dir, name)
         if name.startswith('2_') and os.path.isdir(package_dir):
-            package_version = name.replace('_', '.')
+            package_version = APIVersion(name.replace('_', '.'))
             package_names[package_version] = '{}.{}'.format(base_name, name)
 
     package_version = None
-    for version in sorted(package_names, key=LooseVersion):
-        if (package_version is None or
-                LooseVersion(package_version) < LooseVersion(version)):
+    for version in sorted(package_names):
+        if package_version is None or package_version < version:
             package_version = version
-        if LooseVersion(version) >= server_version:
+        if version >= server_version:
             break
 
     package_name = package_names[package_version]
diff --git a/ipalib/capabilities.py b/ipalib/capabilities.py
index 7ddaea2..55b84aa 100644
--- a/ipalib/capabilities.py
+++ b/ipalib/capabilities.py
@@ -25,7 +25,7 @@
 versions they were introduced in.
 """
 
-from distutils import version
+from ipapython.ipautil import APIVersion
 
 VERSION_WITHOUT_CAPABILITIES = u'2.51'
 
@@ -64,6 +64,6 @@ def client_has_capability(client_version, capability):
     :param client_version: The API version string reported by the client
     """
 
-    version_tuple = version.LooseVersion(client_version)
+    version = APIVersion(client_version)
 
-    return version_tuple >= version.LooseVersion(capabilities[capability])
+    return version >= APIVersion(capabilities[capability])
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index c94d174..c67aa7a 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -20,12 +20,10 @@
 """
 Base classes for all front-end plugins.
 """
-
-from distutils import version
-
 import six
 
 from ipapython.version import API_VERSION
+from ipapython.ipautil import APIVersion
 from ipapython.ipa_log_manager import root_logger
 from ipalib.base import NameSpace
 from ipalib.plugable import Plugin, APINameSpace
@@ -770,16 +768,19 @@ def verify_client_version(self, client_version):
         If the client minor version is less than or equal to the server
         then let the request proceed.
         """
-        server_ver = version.LooseVersion(API_VERSION)
-        ver = version.LooseVersion(client_version)
-        if len(ver.version) < 2:
-            raise VersionError(cver=ver.version, sver=server_ver.version, server= self.env.xmlrpc_uri)
-        client_major = ver.version[0]
-
-        server_major = server_ver.version[0]
-
-        if server_major != client_major:
-            raise VersionError(cver=client_version, sver=API_VERSION, server=self.env.xmlrpc_uri)
+        server_apiver = APIVersion(self.api_version)
+        try:
+            client_apiver = APIVersion(client_version)
+        except ValueError:
+            raise VersionError(cver=client_version,
+                               sver=self.api_version,
+                               server=self.env.xmlrpc_uri)
+
+        if (client_apiver.major != server_apiver.major or
+                    client_apiver > server_apiver):
+            raise VersionError(cver=client_version,
+                               sver=self.api_version,
+                           server=self.env.xmlrpc_uri)
 
     def run(self, *args, **options):
         """
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 76fb9fd..3fdedf0 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -24,8 +24,6 @@
 you are unfamiliar with this Python feature, see
 http://docs.python.org/ref/sequence-types.html
 """
-
-from distutils.version import LooseVersion
 import operator
 import sys
 import threading
@@ -725,8 +723,11 @@ def finalize(self):
                 except KeyError:
                     pass
                 else:
-                    version = LooseVersion(plugin.version)
-                    default_version = LooseVersion(default_version)
+                    # Technicall plugin.version is not an API version. The
+                    # APIVersion class can handle plugin versions. It's more
+                    # lean than pkg_resource.parse_version().
+                    version = ipautil.APIVersion(plugin.version)
+                    default_version = ipautil.APIVersion(default_version)
                     if version < default_version:
                         continue
             self.__default_map[plugin.name] = plugin.version
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index f7d7537..7d34294 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1533,3 +1533,35 @@ def escape_seq(seq, *args):
     """
 
     return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
+
+
+class APIVersion(tuple):
+    """API version parser and handler
+
+    The class is used to parse ipapython.version.API_VERSION and plugin
+    versions.
+    """
+    __slots__ = ()
+
+    def __new__(cls, version):
+        major, dot, minor = version.partition(u'.')
+        major = int(major)
+        minor = int(minor) if dot else 0
+        return tuple.__new__(cls, (major, minor))
+
+    def __str__(self):
+        return '{}.{}'.format(*self)
+
+    def __repr__(self):
+        return "<APIVersion('{}.{}')>".format(*self)
+
+    def __getnewargs__(self):
+        return str(self)
+
+    @property
+    def major(self):
+        return self[0]
+
+    @property
+    def minor(self):
+        return self[1]
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index b5cfd79..02ecee2 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -24,6 +24,7 @@
 import os
 import pwd
 import socket
+
 import dns.name
 
 from ipaserver.install import service
@@ -38,7 +39,6 @@
 from ipaserver.install import ldapupdate
 
 from ipaserver.install import certs
-from distutils import version
 from ipaplatform.constants import constants
 from ipaplatform.tasks import tasks
 from ipaplatform.paths import paths
@@ -291,8 +291,8 @@ def __configure_instance(self):
                              raiseonerr=False, capture_output=True)
         if result.returncode == 0:
             verstr = result.output.split()[-1]
-            ver = version.LooseVersion(verstr)
-            min = version.LooseVersion(MIN_KRB5KDC_WITH_WORKERS)
+            ver = tasks.parse_ipa_version(verstr)
+            min = tasks.parse_ipa_version(MIN_KRB5KDC_WITH_WORKERS)
             if ver >= min:
                 workers = True
         # Write down config file
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index a7b333c..ee1147d 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -4,7 +4,6 @@
 
 from __future__ import print_function
 
-from distutils.version import LooseVersion
 import dns.exception as dnsexception
 import dns.name as dnsname
 import dns.resolver as dnsresolver
@@ -15,6 +14,7 @@
 import tempfile
 import traceback
 
+from pkg_resources import parse_version
 import six
 
 from ipapython import ipaldap, ipautil, sysrestore
@@ -510,12 +510,12 @@ def check_remote_version(api):
     finally:
         client.disconnect()
 
-    remote_version = env['version']
-    version = api.env.version
-    if LooseVersion(remote_version) > LooseVersion(version):
+    remote_version = parse_version(env['version'])
+    api_version = parse_version(api.env.version)
+    if remote_version > api_version:
         raise RuntimeError(
             "Cannot install replica of a server of higher version ({}) than"
-            "the local version ({})".format(remote_version, version))
+            "the local version ({})".format(remote_version, api_version))
 
 
 def common_check(no_ntp):
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to