openstack/horizon
Code Issues Proposed changes

Replace @memoized_with_* with @memoized

Browse Source 
Since @memoized now uses weakref to delete old cache entries, and no
longer keeps the object passed as parameters alive indefinitely, we
no longer need to treat the Request objects specially. In fact, if
we stop doing it, it should improve memory use, because now all
request-specific caches will be removed as soon as the request
object dies.

I also removed the memoized_with_argconv function, and replaced it
with an explicit function for converting teh parameters in the one
place where it was used.

Change-Id: I710b96a170e429c6ffdf22ad3e552ee6e1c6b7e3
This commit is contained in:
Radomir Dopieralski 2018-09-25 11:40:02 +02:00
parent cfe03529c3
commit 3acb28270a
5 changed files with 27 additions and 208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
horizon/test/unit/utils/test_memoized.py
View File

@ -35,82 +35,3 @@ class MemoizedTests(test.TestCase):
for x in range(0, 5):
cache_calls(1)
self.assertEqual(1, len(values_list))
def test_memoized_with_request_call(self):
chorus = [
"I",
"Love",
"Rock 'n' Roll",
"put another coin",
"in the Jukebox Baby."
]
leader = 'Joan Jett'
group = 'Blackhearts'
for position, chorus_line in enumerate(chorus):
changed_args = False
def some_func(some_param):
if not changed_args:
self.assertEqual(some_param, chorus_line)
else:
self.assertNotEqual(some_param, chorus_line)
self.assertEqual(some_param, group)
return leader
@memoized.memoized_with_request(some_func, position)
def some_other_func(*args):
return args
# check chorus_copy[position] is replaced by some_func's
# output
output1 = some_other_func(*chorus)
self.assertEqual(output1[position], leader)
# Change args used to call the function
chorus_copy = list(chorus)
chorus_copy[position] = group
changed_args = True
# check that some_func is called with a different parameter, and
# that check chorus_copy[position] is replaced by some_func's
# output and some_other_func still called with the same parameters
output2 = some_other_func(*chorus_copy)
self.assertEqual(output2[position], leader)
# check that some_other_func returned a memoized list.
self.assertIs(output1, output2)
def test_memoized_with_argcnv(self):
value_list = []
def converter(*args, **kwargs):
new_args = tuple(reversed(args))
new_kwargs = dict((k, v + 1) for k, v in kwargs.items())
return new_args, new_kwargs
@memoized.memoized_with_argconv(converter)
def target_func(*args, **kwargs):
value_list.append(1)
return args, kwargs
for i in range(3):
ret_args, ret_kwargs = target_func(1, 2, 3)
self.assertEqual((3, 2, 1), ret_args)
self.assertEqual({}, ret_kwargs)
self.assertEqual(1, len(value_list))
value_list = []
for i in range(3):
ret_args, ret_kwargs = target_func(a=1, b=2, c=3)
self.assertEqual(tuple(), ret_args)
self.assertEqual({'a': 2, 'b': 3, 'c': 4}, ret_kwargs)
self.assertEqual(1, len(value_list))
value_list = []
for i in range(3):
ret_args, ret_kwargs = target_func(1, 2, a=3, b=4)
self.assertEqual((2, 1), ret_args)
self.assertEqual({'a': 4, 'b': 5}, ret_kwargs)
self.assertEqual(1, len(value_list))

96
horizon/utils/memoized.py
View File

@ -110,99 +110,3 @@ def memoized(func):
# it doesn't keep the instances in memory forever. We might want to separate
# them in the future, however.
memoized_method = memoized
def memoized_with_request(request_func, request_index=0):
"""Decorator for caching functions which receive a request argument
memoized functions with a request argument are memoized only during the
rendering of a single view because the request argument is a new request
instance on each view.
If you want a function to be memoized for multiple views use this
decorator.
It replaces the request argument in the call to the decorated function
with the result of calling request_func on that request object.
request_function is a function which will receive the request argument.
request_index indicates which argument of the decorated function is the
request object to pass into request_func, which will also be replaced
by the result of request_func being called.
your memoized function will instead receive request_func(request)
passed as argument at the request_index.
The intent of that function is to extract the information needed from the
request, and thus the memoizing will operate just on that part of the
request that is relevant to the function being memoized.
short example::
@memoized
def _get_api_client(username, token_id, project_id, auth_url)
return api_client.Client(username, token_id, project_id, auth_url)
def get_api_client(request):
return _api_client(request.user.username,
request.user.token.id,
request.user.tenant_id)
@memoized_with_request(get_api_client)
def some_api_function(api_client, *args, **kwargs):
# is like returning get_api_client(
# request).some_method(*args, **kwargs)
# but with memoization.
return api_client.some_method(*args, **kwargs)
@memoized_with_request(get_api_client, 1)
def some_other_funt(param, api_client, other_param):
# The decorated function will be called this way:
# some_other_funt(param, request, other_param)
# but will be called behind the scenes this way:
# some_other_funt(param, get_api_client(request), other_param)
return api_client.some_method(param, other_param)
See openstack_dashboard.api.nova for a complete example.
"""
def wrapper(func):
memoized_func = memoized(func)
@functools.wraps(func)
def wrapped(*args, **kwargs):
args = list(args)
request = args.pop(request_index)
args.insert(request_index, request_func(request))
return memoized_func(*args, **kwargs)
return wrapped
return wrapper
def memoized_with_argconv(convert_func):
"""Decorator for caching functions which receive unhashable arguments
This decorator is a generalized version of memoized_with_request.
There are cases where argument(s) other than 'request' are also unhashable.
For such cases, such arguments also need to be converted into hashable
variables.
'convert_func' is responsible for replacing unhashable arguments
into corresponding hashable variables.
'convert_func' receives original arguments as its arguments and
it must return a full arguments including converted arguments.
See openstack_dashboard.api.nova as an example.
"""
def wrapper(func):
memoized_func = memoized(func)
@functools.wraps(func)
def wrapped(*args, **kwargs):
args, kwargs = convert_func(*args, **kwargs)
return memoized_func(*args, **kwargs)
return wrapped
return wrapper

19
openstack_dashboard/api/cinder.py
View File

@ -34,7 +34,6 @@ from cinderclient.v2.contrib import list_extensions as cinder_list_extensions
from horizon import exceptions
from horizon.utils import functions as utils
from horizon.utils.memoized import memoized
from horizon.utils.memoized import memoized_with_request
from openstack_dashboard.api import base
from openstack_dashboard.api import microversions
@ -209,15 +208,16 @@ def get_auth_params_from_request(request):
)
@memoized_with_request(get_auth_params_from_request)
def cinderclient(request_auth_params, version=None):
@memoized
def cinderclient(request, version=None):
if version is None:
api_version = VERSIONS.get_active_version()
version = api_version['version']
insecure = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False)
cacert = getattr(settings, 'OPENSTACK_SSL_CACERT', None)
username, token_id, tenant_id, cinder_urls, auth_url = request_auth_params
(username, token_id, tenant_id, cinder_urls,
auth_url) = get_auth_params_from_request(request)
version = base.Version(version)
if version == 2:
service_names = ('volumev2', 'volume')
@ -1015,15 +1015,16 @@ def availability_zone_list(request, detailed=False):
@profiler.trace
@memoized_with_request(cinderclient)
def list_extensions(cinder_api):
@memoized
def list_extensions(request):
cinder_api = cinderclient(request)
return tuple(cinder_list_extensions.ListExtManager(cinder_api).show_all())
@memoized_with_request(list_extensions)
def extension_supported(extensions, extension_name):
@memoized
def extension_supported(request, extension_name):
"""This method will determine if Cinder supports a given extension name."""
for extension in extensions:
for extension in list_extensions(request):
if extension.name == extension_name:
return True
return False

12
openstack_dashboard/api/neutron.py
View File

@ -35,7 +35,6 @@ import six
from horizon import exceptions
from horizon import messages
from horizon.utils.memoized import memoized
from horizon.utils.memoized import memoized_with_request
from openstack_dashboard.api import base
from openstack_dashboard.api import nova
from openstack_dashboard.contrib.developer.profiler import api as profiler
@ -806,9 +805,9 @@ def get_auth_params_from_request(request):
)
@memoized_with_request(get_auth_params_from_request)
def neutronclient(request_auth_params):
token_id, neutron_url, auth_url = request_auth_params
@memoized
def neutronclient(request):
token_id, neutron_url, auth_url = get_auth_params_from_request(request)
insecure = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False)
cacert = getattr(settings, 'OPENSTACK_SSL_CACERT', None)
c = neutron_client.Client(token=token_id,
@ -1785,12 +1784,13 @@ def _server_get_addresses(request, server, ports, floating_ips, network_names):
@profiler.trace
@memoized_with_request(neutronclient)
def list_extensions(neutron_api):
@memoized
def list_extensions(request):
"""List neutron extensions.
:param request: django request object
"""
neutron_api = neutronclient(request)
try:
extensions_list = neutron_api.list_extensions()
except exceptions.ServiceCatalogException:

29
openstack_dashboard/api/nova.py
View File

@ -266,15 +266,14 @@ def get_auth_params_from_request(request):
)
def _argconv_for_novaclient(request, version=None):
req_param = get_auth_params_from_request(request)
def novaclient(request, version=None):
if isinstance(version, api_versions.APIVersion):
version = version.get_string()
return (req_param, version), {}
return cached_novaclient(request, version)
@memoized.memoized_with_argconv(_argconv_for_novaclient)
def novaclient(request_auth_params, version=None):
@memoized.memoized
def cached_novaclient(request, version=None):
(
username,
token_id,
@ -282,7 +281,7 @@ def novaclient(request_auth_params, version=None):
project_domain_id,
nova_url,
auth_url
) = request_auth_params
) = get_auth_params_from_request(request)
if version is None:
version = VERSIONS.get_active_version()['version']
c = nova_client.Client(version,
@ -1066,11 +1065,12 @@ def interface_detach(request, server, port_id):
@profiler.trace
@memoized.memoized_with_request(novaclient)
def list_extensions(nova_api):
@memoized.memoized
def list_extensions(request):
"""List all nova extensions, except the ones in the blacklist."""
blacklist = set(getattr(settings,
'OPENSTACK_NOVA_EXTENSIONS_BLACKLIST', []))
nova_api = novaclient(request)
return tuple(
extension for extension in
nova_list_extensions.ListExtManager(nova_api).show_all()
@ -1078,22 +1078,15 @@ def list_extensions(nova_api):
)
# NOTE(amotoki): In Python 3, a tuple of the Extension classes
# is not hashable. The return value must be a known premitive.
# This converts the return value to a tuple of a string.
def _list_extensions_wrap(request):
return tuple(e.name for e in list_extensions(request))
@profiler.trace
@memoized.memoized_with_request(_list_extensions_wrap, 1)
def extension_supported(extension_name, supported_ext_names):
@memoized.memoized
def extension_supported(extension_name, request):
"""Determine if nova supports a given extension name.
Example values for the extension_name include AdminActions, ConsoleOutput,
etc.
"""
for ext in supported_ext_names:
for ext in list_extensions(request):
if ext == extension_name:
return True
return False