From 56fce183789df4fbacea42e0125b6acac913bdce Mon Sep 17 00:00:00 2001 From: Yves-Gwenael Bourhis Date: Tue, 27 Jan 2015 13:32:39 +0100 Subject: [PATCH] Stop memoizing on request arguments in nova api The request parameter is a django.http.HttpRequest instance object which contains metadata about the request. This object is a new instance of django.http.HttpRequest in every view, so memoizing with it memoizes during the rendering of a single view only, thus preventing multiple api calls only during the rendering of one view. Some nova api memoized methods should memoize more persistantly with objects which do not change after the rendering of each view, like the list of extensions suported. However, it is still a good idea to limit memoization of some api calls during the rendering of a single view, if the query is subject to give different results (i.e., the flavor_list can change when an administrator adds a flavor so we need to retreive new data for each view in case a flavor gets added by a cloud admin). Implements blueprint: improve-horizon-caching Co-Authored-By: Daniel Castellanos Change-Id: I9f87df72cbf149e7b2d1763d7976cb845b1e98f9 --- horizon/test/tests/utils.py | 46 +++++++++++++ horizon/utils/memoized.py | 68 +++++++++++++++++++ openstack_dashboard/api/nova.py | 56 +++++++++------ .../test/api_tests/nova_tests.py | 9 +-- 4 files changed, 151 insertions(+), 28 deletions(-) diff --git a/horizon/test/tests/utils.py b/horizon/test/tests/utils.py index 7077bfcddc..4647c76403 100644 --- a/horizon/test/tests/utils.py +++ b/horizon/test/tests/utils.py @@ -373,6 +373,52 @@ class MemoizedTests(test.TestCase): 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) + class GetPageSizeTests(test.TestCase): def test_bad_session_value(self): diff --git a/horizon/utils/memoized.py b/horizon/utils/memoized.py index 5babe2068b..607acebf71 100644 --- a/horizon/utils/memoized.py +++ b/horizon/utils/memoized.py @@ -104,3 +104,71 @@ 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 diff --git a/openstack_dashboard/api/nova.py b/openstack_dashboard/api/nova.py index 82c2df67a9..5959d9041d 100644 --- a/openstack_dashboard/api/nova.py +++ b/openstack_dashboard/api/nova.py @@ -39,6 +39,7 @@ from horizon import conf from horizon import exceptions as horizon_exceptions from horizon.utils import functions as utils from horizon.utils.memoized import memoized # noqa +from horizon.utils.memoized import memoized_with_request # noqa from openstack_dashboard.api import base from openstack_dashboard.api import network_base @@ -55,6 +56,8 @@ VERSIONS.load_supported_version(2, {"client": nova_client, "version": 2}) INSTANCE_ACTIVE_STATE = 'ACTIVE' VOLUME_STATE_AVAILABLE = "available" DEFAULT_QUOTA_NAME = 'default' +INSECURE = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False) +CACERT = getattr(settings, 'OPENSTACK_SSL_CACERT', None) class VNCConsole(base.APIDictWrapper): @@ -451,20 +454,31 @@ class FloatingIpManager(network_base.FloatingIpManager): return True -@memoized -def novaclient(request): - insecure = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False) - cacert = getattr(settings, 'OPENSTACK_SSL_CACERT', None) +def get_auth_params_from_request(request): + """Extracts the properties from the request object needed by the novaclient + call below. These will be used to memoize the calls to novaclient + """ + return ( + request.user.username, + request.user.token.id, + request.user.tenant_id, + base.url_for(request, 'compute') + ) + + +@memoized_with_request(get_auth_params_from_request) +def novaclient(request_auth_params): + username, token_id, project_id, auth_url = request_auth_params c = nova_client.Client(VERSIONS.get_active_version()['version'], - request.user.username, - request.user.token.id, - project_id=request.user.tenant_id, - auth_url=base.url_for(request, 'compute'), - insecure=insecure, - cacert=cacert, + username, + token_id, + project_id=project_id, + auth_url=auth_url, + insecure=INSECURE, + cacert=CACERT, http_log_debug=settings.DEBUG) - c.client.auth_token = request.user.token.id - c.client.management_url = base.url_for(request, 'compute') + c.client.auth_token = token_id + c.client.management_url = auth_url return c @@ -575,10 +589,10 @@ def flavor_list_paged(request, is_public=True, get_extras=False, marker=None, return (flavors, has_more_data, has_prev_data) -@memoized -def flavor_access_list(request, flavor=None): +@memoized_with_request(novaclient) +def flavor_access_list(nova_api, flavor=None): """Get the list of access instance sizes (flavors).""" - return novaclient(request).flavor_access.list(flavor=flavor) + return nova_api.flavor_access.list(flavor=flavor) def add_tenant_to_flavor(request, flavor, tenant): @@ -1022,27 +1036,25 @@ def interface_detach(request, server, port_id): return novaclient(request).servers.interface_detach(server, port_id) -@memoized -def list_extensions(request): +@memoized_with_request(novaclient) +def list_extensions(nova_api): """List all nova extensions, except the ones in the blacklist.""" - blacklist = set(getattr(settings, 'OPENSTACK_NOVA_EXTENSIONS_BLACKLIST', [])) return [ extension for extension in - nova_list_extensions.ListExtManager(novaclient(request)).show_all() + nova_list_extensions.ListExtManager(nova_api).show_all() if extension.name not in blacklist ] -@memoized -def extension_supported(extension_name, request): +@memoized_with_request(list_extensions, 1) +def extension_supported(extension_name, extensions): """Determine if nova supports a given extension name. Example values for the extension_name include AdminActions, ConsoleOutput, etc. """ - extensions = list_extensions(request) for extension in extensions: if extension.name == extension_name: return True diff --git a/openstack_dashboard/test/api_tests/nova_tests.py b/openstack_dashboard/test/api_tests/nova_tests.py index f1a430e61e..4292d19823 100644 --- a/openstack_dashboard/test/api_tests/nova_tests.py +++ b/openstack_dashboard/test/api_tests/nova_tests.py @@ -521,20 +521,17 @@ class ComputeApiTests(test.APITestCase): self.assertIsNone(api_val) + @test.create_stubs({api.nova: ('flavor_access_list',)}) def test_flavor_access_list(self): flavor_access = self.flavor_access.list() flavor = [f for f in self.flavors.list() if f.id == flavor_access[0].flavor_id][0] - novaclient = self.stub_novaclient() - novaclient.flavors = self.mox.CreateMockAnything() - novaclient.flavor_access = self.mox.CreateMockAnything() - novaclient.flavor_access.list(flavor=flavor).AndReturn(flavor_access) + api.nova.flavor_access_list(self.request, flavor)\ + .AndReturn(flavor_access) self.mox.ReplayAll() - api_flavor_access = api.nova.flavor_access_list(self.request, flavor) - self.assertEqual(len(flavor_access), len(api_flavor_access)) for access in api_flavor_access: self.assertIsInstance(access, nova_flavor_access.FlavorAccess)