openstack/python-cinderclient
Code Issues Proposed changes

Fix highest version supported by client and server

Browse Source 
Current code mistakenly thinks that 3.40 is older than 3.27, because
it's treating 3.40 as 3.4, thus returning the wrong maximum supported
version by both server and client.

It is also returning a 3.40 version as 3.4 because it's returning it as
a float.

This patch fixes both issues by not using float conversion but using the
APIVersion object to do the comparison and by changing returned type to
a string so it can be used to instantiate a client.

Change-Id: Ica4d718b3de31c31da047f07c5154b242e122596
Closes-Bug: #1705093
This commit is contained in:
Gorka Eguileor 2017-07-19 14:45:13 +02:00
parent 83230498eb
commit 52cc5c6cb3
2 changed files with 12 additions and 10 deletions
cinderclient
client.py
tests/unit
test_client.py

6
cinderclient/client.py

@ -96,10 +96,10 @@ def get_server_version(url):
def get_highest_client_server_version(url):
"""Returns highest supported version by client and server as a string."""
min_server, max_server = get_server_version(url)
max_server_version = api_versions.APIVersion.get_string(max_server)
return min(float(max_server_version), float(api_versions.MAX_VERSION))
max_client = api_versions.APIVersion(api_versions.MAX_VERSION)
return min(max_server, max_client).get_string()
def get_volume_api_from_url(url):

16
cinderclient/tests/unit/test_client.py

@ -14,6 +14,7 @@
import json
import logging
import ddt
import fixtures
from keystoneauth1 import adapter
from keystoneauth1 import exceptions as keystone_exception
@ -309,6 +310,7 @@ class ClientTestSensitiveInfo(utils.TestCase):
self.assertNotIn(auth_password, output[1], output)
@ddt.ddt
class GetAPIVersionTestCase(utils.TestCase):
@mock.patch('cinderclient.client.requests.get')
@ -334,7 +336,8 @@ class GetAPIVersionTestCase(utils.TestCase):
self.assertEqual(max_version, api_versions.APIVersion('3.16'))
@mock.patch('cinderclient.client.requests.get')
def test_get_highest_client_server_version(self, mock_request):
@ddt.data('3.12', '3.40')
def test_get_highest_client_server_version(self, version, mock_request):
mock_response = utils.TestResponse({
"status_code": 200,
@ -345,9 +348,8 @@ class GetAPIVersionTestCase(utils.TestCase):
url = "http://192.168.122.127:8776/v3/e5526285ebd741b1819393f772f11fc3"
highest = cinderclient.client.get_highest_client_server_version(url)
current_client_MAX_VERSION = float(api_versions.MAX_VERSION)
if current_client_MAX_VERSION > 3.16:
self.assertEqual(3.16, highest)
else:
self.assertEqual(current_client_MAX_VERSION, highest)
with mock.patch.object(api_versions, 'MAX_VERSION', version):
highest = (
cinderclient.client.get_highest_client_server_version(url))
expected = version if version == '3.12' else '3.16'
self.assertEqual(expected, highest)