Fix version handling compatible with python-semanticversion 2.8

It seems the behavior of 'partial' argument of Version class __init__
has been changed between semantic-version 2.6.0 and 2.8.1
(though I could not identify the root cause).
'partial' argument is marked as deprecated in semantic-vesion 2.7.0,
so it is a good chance not to depend on 'partial' argument in horizon.

This commit uses Version.coerce() [1] instead to convert non-semver
version into a valid semver version.
We also need to keep the original version information as it is passed
when initializing python-*client (cinderclient and keystoneclient).

In addition, the previous implementation based on semantic-version 2.6.0
returns True for "Version("3.55") == 3". It depends on the behavior
of "partial" argument of semantic_version.Version. It was not
documented and it looks tricky to depend on this behavior.
"major" and "minor" properties are now introduced and api/cinder.py
is updated accordingly. I believe this approach is clearer and stable.

Unit test coverage on dict behavior is improved.
Variable names in the unit tests are adjusted to more meaningful ones.

[1] https://python-semanticversion.readthedocs.io/en/latest/#coercing-an-arbitrary-version-string

Change-Id: If0deee9d0289ff91d58d942b9612f7736356ae18
This commit is contained in:
Akihiro Motoki 2019-09-06 17:26:49 +09:00
parent cd8fae0631
commit 5fd5b4c893
3 changed files with 70 additions and 18 deletions

View File

@ -32,23 +32,45 @@ __all__ = ('APIResourceWrapper', 'APIDictWrapper',
@functools.total_ordering @functools.total_ordering
class Version(object): class Version(object):
"""A class to handle API version.
The current OpenStack APIs use the versioning of "<major>.<minor>",
so this class supports this style only.
"""
# NOTE(amotoki): The implementation depends on the semantic_version library
# but we don't care the patch version in this class.
def __init__(self, version): def __init__(self, version):
self.version = semantic_version.Version(str(version), partial=True) # NOTE(amotoki):
# All comparisons should use self.sem_ver as we would like to
# compare versions in the semantic versioning way.
# self.orig_ver should be used only in __str__ and __repr__
# to keep the original version information.
self.orig_ver = str(version)
self.sem_ver = semantic_version.Version.coerce(str(version))
@property
def major(self):
return self.sem_ver.major
@property
def minor(self):
return self.sem_ver.minor
def __eq__(self, other): def __eq__(self, other):
return self.version == Version(other).version return self.sem_ver == Version(other).sem_ver
def __lt__(self, other): def __lt__(self, other):
return self.version < Version(other).version return self.sem_ver < Version(other).sem_ver
def __repr__(self): def __repr__(self):
return "Version('%s')" % self.version return "Version('%s')" % self.orig_ver
def __str__(self): def __str__(self):
return str(self.version) return str(self.orig_ver)
def __hash__(self): def __hash__(self):
return hash(str(self.version)) return hash(str(self.sem_ver))
class APIVersionManager(object): class APIVersionManager(object):

View File

@ -233,9 +233,9 @@ def cinderclient(request, version=None):
(username, token_id, tenant_id, cinder_urls, (username, token_id, tenant_id, cinder_urls,
auth_url) = get_auth_params_from_request(request) auth_url) = get_auth_params_from_request(request)
version = base.Version(version) version = base.Version(version)
if version == 2: if version.major == 2:
service_names = ('volumev2', 'volume') service_names = ('volumev2', 'volume')
elif version == 3: elif version.major == 3:
service_names = ('volumev3', 'volume') service_names = ('volumev3', 'volume')
else: else:
service_names = ('volume',) service_names = ('volume',)

View File

@ -83,25 +83,55 @@ class APIVersionTests(test.TestCase):
self.assertEqual(api_base.Version('1.0'), version) self.assertEqual(api_base.Version('1.0'), version)
def test_greater(self): def test_greater(self):
version1 = api_base.Version('1.0') version10 = api_base.Version('1.0')
version12 = api_base.Version('1.2') version12 = api_base.Version('1.2')
version120 = api_base.Version('1.20') version120 = api_base.Version('1.20')
self.assertGreater(version12, version1) self.assertGreater(version12, version10)
self.assertGreater(version120, version12) self.assertGreater(version120, version12)
self.assertEqual(version12, 1) # sic! self.assertGreater(version12, 1)
self.assertGreater(1.2, version1) self.assertGreater(1.2, version10)
self.assertGreater(version120, 1.2) self.assertGreater(version120, 1.2)
self.assertGreater('1.20', version12) self.assertGreater('1.20', version12)
def test_dict(self): def test_dict(self):
version1 = api_base.Version('1.0') test_dict = {api_base.Version('1.0'): 1}
version1b = api_base.Version('1.0')
self.assertIn(version1, {version1b: 1}) self.assertIn(api_base.Version('1'), test_dict)
self.assertIn(api_base.Version('1.0'), test_dict)
self.assertIn(api_base.Version('1.0.0'), test_dict)
self.assertIn(api_base.Version(1), test_dict)
self.assertIn(api_base.Version(1.0), test_dict)
self.assertNotIn(api_base.Version('1.2'), test_dict)
self.assertNotIn(api_base.Version('1.20'), test_dict)
test_dict = {api_base.Version('1.2'): 1}
self.assertIn(api_base.Version('1.2'), test_dict)
self.assertIn(api_base.Version('1.2.0'), test_dict)
self.assertIn(api_base.Version(1.2), test_dict)
self.assertNotIn(api_base.Version('1'), test_dict)
self.assertNotIn(api_base.Version('1.0'), test_dict)
self.assertNotIn(api_base.Version('1.0.0'), test_dict)
self.assertNotIn(api_base.Version(1), test_dict)
self.assertNotIn(api_base.Version(1.0), test_dict)
self.assertNotIn(api_base.Version('1.20'), test_dict)
def test_text(self): def test_text(self):
version1 = api_base.Version('1.0') version10 = api_base.Version('1.0')
self.assertEqual("1.0", str(version1)) self.assertEqual("1.0", str(version10))
self.assertEqual("Version('1.0')", repr(version1)) self.assertEqual("Version('1.0')", repr(version10))
def test_major_minor(self):
version1 = api_base.Version('1')
version10 = api_base.Version('1.0')
version120 = api_base.Version('1.20')
self.assertEqual(1, version1.major)
self.assertEqual(0, version1.minor)
self.assertEqual(1, version10.major)
self.assertEqual(0, version10.minor)
self.assertEqual(1, version120.major)
self.assertEqual(20, version120.minor)
# Wrapper classes that only define _attrs don't need extra testing. # Wrapper classes that only define _attrs don't need extra testing.