openstack/python-novaclient
Code Issues Proposed changes

Extend microversion stuff to support resource classes

Browse Source 
Current implementation of novaclient.api_versions.wraps allow to use
versioned methods for shell functions and resource managers. As resource
managers can have versioned methods, resource objects can have it too, but
it was not implemented yet. This patch fixes this omission.

Changes:
 - Add api_version property to base resource class. It is mapped to
   api_version property of manager class;
 - Move `novaclient.utils.generate_function_name` to
   `novaclient.api_versions._generate_function_name`, since this method is
   specific to microversion stuff and should not used outside api_versions
   module;
 - Rewrite _generate_function_name to handle class(owner) name. Previously,
   it was improssible to have two classes in one module with versioned
   methods with equal names.
 - Remove call of generate_function_name from novaclient.shell. Shell module
   should not take care about function identifiers. get_substitutions accepts
   object with __id__ property now.
 - Mark _add_substitution as private method, since it should not be used
   outside api_versions module
 - Split all versioned methods of Server resource from novaclient.v2.servers
   module.

Change-Id: Icfce16bfa6f919d7f8451d592f4a8e276b1f1709
This commit is contained in:
Andrey Kurilin 2016-04-21 12:00:04 +03:00
parent 19ff19e1b8
commit f9bdba2dd7
7 changed files with 133 additions and 81 deletions

37
novaclient/api_versions.py

@ -16,13 +16,13 @@ import logging
import os
import pkgutil
import re
import traceback
from oslo_utils import strutils
import novaclient
from novaclient import exceptions
from novaclient.i18n import _, _LW
from novaclient import utils
LOG = logging.getLogger(__name__)
if not LOG.handlers:
@ -340,12 +340,32 @@ def check_headers(response, api_version):
_warn_missing_microversion_header(HEADER_NAME)
def add_substitution(versioned_method):
def _add_substitution(versioned_method):
_SUBSTITUTIONS.setdefault(versioned_method.name, [])
_SUBSTITUTIONS[versioned_method.name].append(versioned_method)
def _get_function_name(func):
# NOTE(andreykurilin): Based on the facts:
# - Python 2 does not have __qualname__ property as Python 3 has;
# - we cannot use im_class here, since we need to obtain name of
# function in `wraps` decorator during class initialization
# ("im_class" property does not exist at that moment)
# we need to write own logic to obtain the full function name which
# include module name, owner name(optional) and just function name.
filename, _lineno, _name, line = traceback.extract_stack()[-4]
module, _file_extension = os.path.splitext(filename)
module = module.replace("/", ".")
if module.endswith(func.__module__):
return "%s.[%s].%s" % (func.__module__, line, func.__name__)
else:
return "%s.%s" % (func.__module__, func.__name__)
def get_substitutions(func_name, api_version=None):
if hasattr(func_name, "__id__"):
func_name = func_name.__id__
substitutions = _SUBSTITUTIONS.get(func_name, [])
if api_version and not api_version.is_null():
return [m for m in substitutions
@ -362,10 +382,11 @@ def wraps(start_version, end_version=None):
def decor(func):
func.versioned = True
name = utils.get_function_name(func)
name = _get_function_name(func)
versioned_method = VersionedMethod(name, start_version,
end_version, func)
add_substitution(versioned_method)
_add_substitution(versioned_method)
@functools.wraps(func)
def substitution(obj, *args, **kwargs):
@ -374,7 +395,6 @@ def wraps(start_version, end_version=None):
if not methods:
raise exceptions.VersionNotFoundForAPIMethod(
obj.api_version.get_string(), name)
return methods[-1].func(obj, *args, **kwargs)
# Let's share "arguments" with original method and substitution to
@ -383,6 +403,13 @@ def wraps(start_version, end_version=None):
func.arguments = []
substitution.arguments = func.arguments
# NOTE(andreykurilin): The way to obtain function's name in Python 2
# bases on traceback(see _get_function_name for details). Since the
# right versioned method method is used in several places, one object
# can have different names. Let's generate name of function one time
# and use __id__ property in all other places.
substitution.__id__ = name
return substitution
return decor

4
novaclient/base.py

@ -147,6 +147,10 @@ class Resource(RequestIdMixin):
info = ", ".join("%s=%s" % (k, getattr(self, k)) for k in reprkeys)
return "<%s %s>" % (self.__class__.__name__, info)
@property
def api_version(self):
return self.manager.api_version
@property
def human_id(self):
"""Human-readable ID which can be used for bash completion.

3
novaclient/shell.py

@ -633,8 +633,7 @@ class OpenStackComputeShell(object):
desc = callback.__doc__ or ''
if hasattr(callback, "versioned"):
additional_msg = ""
subs = api_versions.get_substitutions(
utils.get_function_name(callback))
subs = api_versions.get_substitutions(callback)
if do_help:
additional_msg = msg % {
'start': subs[0].start_version.get_string(),

55
novaclient/tests/unit/test_api_versions.py

@ -233,11 +233,9 @@ class WrapsTestCase(utils.TestCase):
m.name = args[0]
return m
@mock.patch("novaclient.utils.get_function_name")
@mock.patch("novaclient.api_versions._get_function_name")
@mock.patch("novaclient.api_versions.VersionedMethod")
def test_end_version_is_none(self, mock_versioned_method, mock_name):
func_name = "foo"
mock_name.return_value = func_name
mock_versioned_method.side_effect = self._side_effect_of_vers_method
@api_versions.wraps("2.2")
@ -247,15 +245,13 @@ class WrapsTestCase(utils.TestCase):
foo(self._get_obj_with_vers("2.4"))
mock_versioned_method.assert_called_once_with(
func_name, api_versions.APIVersion("2.2"),
mock_name.return_value, api_versions.APIVersion("2.2"),
api_versions.APIVersion("2.latest"), mock.ANY)
@mock.patch("novaclient.utils.get_function_name")
@mock.patch("novaclient.api_versions._get_function_name")
@mock.patch("novaclient.api_versions.VersionedMethod")
def test_start_and_end_version_are_presented(self, mock_versioned_method,
mock_name):
func_name = "foo"
mock_name.return_value = func_name
mock_versioned_method.side_effect = self._side_effect_of_vers_method
@api_versions.wraps("2.2", "2.6")
@ -265,14 +261,12 @@ class WrapsTestCase(utils.TestCase):
foo(self._get_obj_with_vers("2.4"))
mock_versioned_method.assert_called_once_with(
func_name, api_versions.APIVersion("2.2"),
mock_name.return_value, api_versions.APIVersion("2.2"),
api_versions.APIVersion("2.6"), mock.ANY)
@mock.patch("novaclient.utils.get_function_name")
@mock.patch("novaclient.api_versions._get_function_name")
@mock.patch("novaclient.api_versions.VersionedMethod")
def test_api_version_doesnt_match(self, mock_versioned_method, mock_name):
func_name = "foo"
mock_name.return_value = func_name
mock_versioned_method.side_effect = self._side_effect_of_vers_method
@api_versions.wraps("2.2", "2.6")
@ -283,7 +277,7 @@ class WrapsTestCase(utils.TestCase):
foo, self._get_obj_with_vers("2.1"))
mock_versioned_method.assert_called_once_with(
func_name, api_versions.APIVersion("2.2"),
mock_name.return_value, api_versions.APIVersion("2.2"),
api_versions.APIVersion("2.6"), mock.ANY)
def test_define_method_is_actually_called(self):
@ -301,7 +295,8 @@ class WrapsTestCase(utils.TestCase):
checker.assert_called_once_with(*((obj,) + some_args), **some_kwargs)
def test_arguments_property_is_copied(self):
@mock.patch("novaclient.api_versions._get_function_name")
def test_arguments_property_is_copied(self, mock_name):
@nutils.arg("argument_1")
@api_versions.wraps("2.666", "2.777")
@nutils.arg("argument_2")
@ -309,14 +304,44 @@ class WrapsTestCase(utils.TestCase):
pass
versioned_method = api_versions.get_substitutions(
nutils.get_function_name(some_func),
api_versions.APIVersion("2.700"))[0]
mock_name.return_value, api_versions.APIVersion("2.700"))[0]
self.assertEqual(some_func.arguments,
versioned_method.func.arguments)
self.assertIn((("argument_1",), {}), versioned_method.func.arguments)
self.assertIn((("argument_2",), {}), versioned_method.func.arguments)
def test_several_methods_with_same_name_in_one_module(self):
class A(object):
api_version = api_versions.APIVersion("777.777")
@api_versions.wraps("777.777")
def f(self):
return 1
class B(object):
api_version = api_versions.APIVersion("777.777")
@api_versions.wraps("777.777")
def f(self):
return 2
self.assertEqual(1, A().f())
self.assertEqual(2, B().f())
def test_generate_function_name(self):
expected_name = "novaclient.tests.unit.test_api_versions.fake_func"
self.assertNotIn(expected_name, api_versions._SUBSTITUTIONS)
@api_versions.wraps("7777777.7777777")
def fake_func():
pass
self.assertIn(expected_name, api_versions._SUBSTITUTIONS)
self.assertEqual(expected_name, fake_func.__id__)
class DiscoverVersionTestCase(utils.TestCase):
def setUp(self):

5
novaclient/tests/unit/v2/test_servers.py

@ -1144,11 +1144,6 @@ class ServersV225Test(ServersV219Test):
{'os-migrateLive': {'host': 'hostname',
'block_migration': 'auto'}})
def test_live_migrate_server_with_disk_over_commit(self):
s = self.cs.servers.get(1234)
self.assertRaises(ValueError, s.live_migrate, 'hostname',
'auto', 'True')
class ServersV226Test(ServersV225Test):
def setUp(self):

10
novaclient/utils.py

@ -465,13 +465,3 @@ def record_time(times, enabled, *args):
yield
end = time.time()
times.append((' '.join(args), start, end))
def get_function_name(func):
if six.PY2:
if hasattr(func, "im_class"):
return "%s.%s" % (func.im_class, func.__name__)
else:
return "%s.%s" % (func.__module__, func.__name__)
else:
return "%s.%s" % (func.__module__, func.__qualname__)

100
novaclient/v2/servers.py

@ -50,18 +50,25 @@ class Server(base.Resource):
"""
return self.manager.delete(self)
@api_versions.wraps("2.0", "2.18")
def update(self, name=None):
"""
Update the name and the description for this server.
:param name: Update the server's name.
:returns: :class:`Server`
"""
return self.manager.update(self, name=name)
@api_versions.wraps("2.19")
def update(self, name=None, description=None):
"""
Update the name and the description for this server.
:param name: Update the server's name.
:param description: Update the server's description(
allowed for 2.19-latest).
:param description: Update the server's description.
:returns: :class:`Server`
"""
if (description is not None and
self.manager.api_version < api_versions.APIVersion("2.19")):
raise exceptions.UnsupportedAttribute("description", "2.19")
update_kwargs = {"name": name}
if description is not None:
update_kwargs["description"] = description
@ -403,43 +410,41 @@ class Server(base.Resource):
except Exception:
return {}
@api_versions.wraps("2.0", "2.24")
def live_migrate(self, host=None,
block_migration=None,
block_migration=False,
disk_over_commit=None):
"""
Migrates a running instance to a new machine.
:param host: destination host name.
:param block_migration: if True, do block_migration, the default
value None will be mapped to False for 2.0 -
2.24, 'auto' for higher than 2.25
value is False and None will be mapped to False
:param disk_over_commit: if True, allow disk over commit, the default
value None will be mapped to False. It will
not be supported since 2.25
value is None which is mapped to False
:returns: An instance of novaclient.base.TupleWithMeta
"""
if block_migration is None:
block_migration = False
if disk_over_commit is None:
disk_over_commit = False
return self.manager.live_migrate(self, host,
block_migration,
disk_over_commit)
if (self.manager.api_version < api_versions.APIVersion("2.25")):
# NOTE(eliqiao): We do this to keep old version api has same
# default value if user don't pass these parameters when using
# SDK
if block_migration is None:
block_migration = False
if disk_over_commit is None:
disk_over_commit = False
@api_versions.wraps("2.25")
def live_migrate(self, host=None, block_migration=None):
"""
Migrates a running instance to a new machine.
return self.manager.live_migrate(self, host,
block_migration,
disk_over_commit)
else:
if block_migration is None:
block_migration = 'auto'
if disk_over_commit is not None:
raise ValueError("Setting 'disk_over_commit' argument is "
"prohibited after microversion 2.25.")
return self.manager.live_migrate(self, host,
block_migration)
:param host: destination host name.
:param block_migration: if True, do block_migration, the default
value is None which is mapped to 'auto'.
:returns: An instance of novaclient.base.TupleWithMeta
"""
if block_migration is None:
block_migration = "auto"
return self.manager.live_migrate(self, host, block_migration)
def reset_state(self, state='error'):
"""
@ -480,29 +485,31 @@ class Server(base.Resource):
"""
return self.manager.list_security_group(self)
def evacuate(self, host=None, on_shared_storage=None, password=None):
@api_versions.wraps("2.0", "2.13")
def evacuate(self, host=None, on_shared_storage=True, password=None):
"""
Evacuate an instance from failed host to specified host.
:param host: Name of the target host
:param on_shared_storage: Specifies whether instance files located
on shared storage. After microversion 2.14, this
parameter must have its default value of None.
on shared storage.
:param password: string to set as admin password on the evacuated
server.
:returns: An instance of novaclient.base.TupleWithMeta
"""
if api_versions.APIVersion("2.14") <= self.manager.api_version:
if on_shared_storage is not None:
raise ValueError("Setting 'on_shared_storage' argument is "
"prohibited after microversion 2.14.")
return self.manager.evacuate(self, host, password)
else:
# microversions 2.0 - 2.13
if on_shared_storage is None:
on_shared_storage = True
return self.manager.evacuate(self, host, on_shared_storage,
password)
return self.manager.evacuate(self, host, on_shared_storage, password)
@api_versions.wraps("2.14")
def evacuate(self, host=None, password=None):
"""
Evacuate an instance from failed host to specified host.
:param host: Name of the target host
:param password: string to set as admin password on the evacuated
server.
:returns: An instance of novaclient.base.TupleWithMeta
"""
return self.manager.evacuate(self, host, password)
def interface_list(self):
"""
@ -526,30 +533,35 @@ class Server(base.Resource):
"""Trigger crash dump in an instance"""
return self.manager.trigger_crash_dump(self)
@api_versions.wraps('2.26')
def tag_list(self):
"""
Get list of tags from an instance.
"""
return self.manager.tag_list(self)
@api_versions.wraps('2.26')
def delete_tag(self, tag):
"""
Remove single tag from an instance.
"""
return self.manager.delete_tag(self, tag)
@api_versions.wraps('2.26')
def delete_all_tags(self):
"""
Remove all tags from an instance.
"""
return self.manager.delete_all_tags(self)
@api_versions.wraps('2.26')
def set_tags(self, tags):
"""
Set list of tags to an instance.
"""
return self.manager.set_tags(self, tags)
@api_versions.wraps('2.26')
def add_tag(self, tag):
"""
Add single tag to an instance.