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