openstack
/
nova
Code Issues Proposed changes

Added method is_supported to check API microversions 

Right now to check if some part of code supports
requested microversion developer usually:

 1. get a microversion from request.
 2. create instance of class APIVersionRequest
    with requested microversion.
 3. compare these two objects.

This check takes at least 2 lines and looks ugly.
Sometimes developers create unnecessary class fields
to store object APIVersionRequest with requested microversion.

To make nova code more readable and simple method
'is_supported' was created.

Microversion check with this method takes only one line
and code looks more beautiful.

Change-Id: I9078cfa1afad8b6b09583dac87fa757a0cc32daa
This commit is contained in:
Sergey Nikitin 2015-10-29 14:45:54 +03:00
parent 95019f9786
commit 2002120c45
8 changed files with 70 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
nova/api/openstack/api_version_request.py
View File

@ -76,6 +76,24 @@ def max_api_version():
return APIVersionRequest(_MAX_API_VERSION)
def is_supported(req, min_version=_MIN_API_VERSION,
max_version=_MAX_API_VERSION):
"""Check if API request version satisfies version restrictions.
:param req: request object
:param min_version: minimal version of API needed for correct
request processing
:param max_version: maximum version of API needed for correct
request processing
:returns True if request satisfies minimal and maximum API version
requirements. False in other case.
"""
return (APIVersionRequest(max_version) >= req.api_version_request >=
APIVersionRequest(min_version))
class APIVersionRequest(object):
"""This class represents an API Version Request with convenience
methods for manipulation and comparison of version

11
nova/api/openstack/compute/extended_server_attributes.py
View File

@ -27,14 +27,13 @@ class ExtendedServerAttributesController(wsgi.Controller):
def __init__(self, *args, **kwargs):
super(ExtendedServerAttributesController, self).__init__(*args,
**kwargs)
self.api_version_2_3 = api_version_request.APIVersionRequest('2.3')
def _extend_server(self, context, server, instance, requested_version):
def _extend_server(self, context, server, instance, req):
key = "OS-EXT-SRV-ATTR:hypervisor_hostname"
server[key] = instance.node
properties = ['host', 'name']
if requested_version >= self.api_version_2_3:
if api_version_request.is_supported(req, min_version='2.3'):
properties += ['reservation_id', 'launch_index',
'hostname', 'kernel_id', 'ramdisk_id',
'root_device_name', 'user_data']
@ -53,8 +52,7 @@ class ExtendedServerAttributesController(wsgi.Controller):
db_instance = req.get_db_instance(server['id'])
# server['id'] is guaranteed to be in the cache due to
# the core API adding it in its 'show' method.
self._extend_server(context, server, db_instance,
req.api_version_request)
self._extend_server(context, server, db_instance, req)
@wsgi.extends
def detail(self, req, resp_obj):
@ -65,8 +63,7 @@ class ExtendedServerAttributesController(wsgi.Controller):
db_instance = req.get_db_instance(server['id'])
# server['id'] is guaranteed to be in the cache due to
# the core API adding it in its 'detail' method.
self._extend_server(context, server, db_instance,
req.api_version_request)
self._extend_server(context, server, db_instance, req)
class ExtendedServerAttributes(extensions.V21APIExtensionBase):

11
nova/api/openstack/compute/extended_volumes.py
View File

@ -25,14 +25,13 @@ soft_authorize = extensions.os_compute_soft_authorizer(ALIAS)
class ExtendedVolumesController(wsgi.Controller):
def __init__(self, *args, **kwargs):
super(ExtendedVolumesController, self).__init__(*args, **kwargs)
self.api_version_2_3 = api_version_request.APIVersionRequest('2.3')
def _extend_server(self, context, server, requested_version, bdms):
def _extend_server(self, context, server, req, bdms):
volumes_attached = []
for bdm in bdms:
if bdm.get('volume_id'):
volume_attached = {'id': bdm['volume_id']}
if requested_version >= self.api_version_2_3:
if api_version_request.is_supported(req, min_version='2.3'):
volume_attached['delete_on_termination'] = (
bdm['delete_on_termination'])
volumes_attached.append(volume_attached)
@ -42,18 +41,16 @@ class ExtendedVolumesController(wsgi.Controller):
@wsgi.extends
def show(self, req, resp_obj, id):
context = req.environ['nova.context']
version = req.api_version_request
if soft_authorize(context):
server = resp_obj.obj['server']
bdms = objects.BlockDeviceMappingList.bdms_by_instance_uuid(
context, [server['id']])
instance_bdms = self._get_instance_bdms(bdms, server)
self._extend_server(context, server, version, instance_bdms)
self._extend_server(context, server, req, instance_bdms)
@wsgi.extends
def detail(self, req, resp_obj):
context = req.environ['nova.context']
version = req.api_version_request
if soft_authorize(context):
servers = list(resp_obj.obj['servers'])
instance_uuids = [server['id'] for server in servers]
@ -61,7 +58,7 @@ class ExtendedVolumesController(wsgi.Controller):
context, instance_uuids)
for server in servers:
instance_bdms = self._get_instance_bdms(bdms, server)
self._extend_server(context, server, version, instance_bdms)
self._extend_server(context, server, req, instance_bdms)
def _get_instance_bdms(self, bdms, server):
# server['id'] is guaranteed to be in the cache due to

3
nova/api/openstack/compute/flavor_access.py
View File

@ -107,8 +107,7 @@ class FlavorActionController(wsgi.Controller):
flavor = common.get_flavor(context, id)
try:
req_ver = req.api_version_request
if req_ver >= api_version_request.APIVersionRequest("2.7"):
if api_version_request.is_supported(req, min_version='2.7'):
if flavor.is_public:
exp = _("Can not add access to a public flavor.")
raise webob.exc.HTTPConflict(explanation=exp)

3
nova/api/openstack/compute/servers.py
View File

@ -1109,8 +1109,7 @@ class ServersController(wsgi.Controller):
"""Return server search options allowed by non-admin."""
opt_list = ('reservation_id', 'name', 'status', 'image', 'flavor',
'ip', 'changes-since', 'all_tenants')
req_ver = req.api_version_request
if req_ver > api_version_request.APIVersionRequest("2.4"):
if api_version_request.is_supported(req, min_version='2.5'):
opt_list += ('ip6',)
return opt_list

7
nova/api/openstack/compute/services.py
View File

@ -181,8 +181,7 @@ class ServiceController(wsgi.Controller):
"""Return a list of all running services. Filter by host & service
name
"""
req_ver = req.api_version_request
if req_ver >= api_version_request.APIVersionRequest("2.11"):
if api_version_request.is_supported(req, min_version='2.11'):
_services = self._get_services_list(req, ['forced_down'])
else:
_services = self._get_services_list(req)
@ -194,9 +193,7 @@ class ServiceController(wsgi.Controller):
@validation.schema(services.service_update_v211, '2.11')
def update(self, req, id, body):
"""Perform service update"""
req_ver = req.api_version_request
if req_ver >= api_version_request.APIVersionRequest("2.11"):
if api_version_request.is_supported(req, min_version='2.11'):
actions = self.actions.copy()
actions["force-down"] = self._forced_down
else:

3
nova/api/openstack/compute/virtual_interfaces.py
View File

@ -32,8 +32,7 @@ def _translate_vif_summary_view(req, vif):
d = {}
d['id'] = vif.uuid
d['mac_address'] = vif.address
if (req.api_version_request >=
api_version_request.APIVersionRequest("2.12")):
if api_version_request.is_supported(req, min_version='2.12'):
d['net_id'] = vif.net_uuid
# NOTE(gmann): This is for v2.1 compatible mode where response should be
# same as v2 one.

39
nova/tests/unit/api/openstack/test_api_version_request.py
View File

@ -15,6 +15,7 @@
from nova.api.openstack import api_version_request
from nova import exception
from nova import test
from nova.tests.unit.api.openstack import fakes
class APIVersionRequestTests(test.NoDBTestCase):
@ -123,3 +124,41 @@ class APIVersionRequestTests(test.NoDBTestCase):
self.assertRaises(ValueError,
api_version_request.APIVersionRequest().get_string)
def test_is_supported_min_version(self):
req = fakes.HTTPRequest.blank('/fake', version='2.5')
self.assertTrue(api_version_request.is_supported(
req, min_version='2.4'))
self.assertTrue(api_version_request.is_supported(
req, min_version='2.5'))
self.assertFalse(api_version_request.is_supported(
req, min_version='2.6'))
def test_is_supported_max_version(self):
req = fakes.HTTPRequest.blank('/fake', version='2.5')
self.assertFalse(api_version_request.is_supported(
req, max_version='2.4'))
self.assertTrue(api_version_request.is_supported(
req, max_version='2.5'))
self.assertTrue(api_version_request.is_supported(
req, max_version='2.6'))
def test_is_supported_min_and_max_version(self):
req = fakes.HTTPRequest.blank('/fake', version='2.5')
self.assertFalse(api_version_request.is_supported(
req, min_version='2.3', max_version='2.4'))
self.assertTrue(api_version_request.is_supported(
req, min_version='2.3', max_version='2.5'))
self.assertTrue(api_version_request.is_supported(
req, min_version='2.3', max_version='2.7'))
self.assertTrue(api_version_request.is_supported(
req, min_version='2.5', max_version='2.7'))
self.assertFalse(api_version_request.is_supported(
req, min_version='2.6', max_version='2.7'))
self.assertTrue(api_version_request.is_supported(
req, min_version='2.5', max_version='2.5'))
self.assertFalse(api_version_request.is_supported(
req, min_version='2.10', max_version='2.1'))