From b87922862f031483c781562cfb1bef5433bdb7e6 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 21 Sep 2012 17:12:46 +0200 Subject: [PATCH] Get rid of gratuitous params in RPC API For historical reasons, we were passing all of the parameters from the query string received by the AWS API to the engine as "params" in many calls. This is not required, since all of the relevant information has already been extracted. Change-Id: Iedec5b442ccb0b358afc8a4e06f60672890aba05 Signed-off-by: Zane Bitter --- heat/api/v1/stacks.py | 27 ++++++-------------- heat/engine/manager.py | 14 ++++------- heat/engine/rpcapi.py | 32 +++++++++++++----------- heat/tests/test_api_cfn_v1.py | 41 ++++++++++--------------------- heat/tests/test_engine_manager.py | 15 ++++++----- heat/tests/test_rpcapi.py | 11 ++++----- 6 files changed, 54 insertions(+), 86 deletions(-) diff --git a/heat/api/v1/stacks.py b/heat/api/v1/stacks.py index b97055f4..a74f2879 100644 --- a/heat/api/v1/stacks.py +++ b/heat/api/v1/stacks.py @@ -126,9 +126,7 @@ class StackController(object): try: # Note show_stack returns details for all stacks when called with # no stack_name, we only use a subset of the result here though - stack_list = self.engine_rpcapi.show_stack(con, - stack_identity=None, - params=parms) + stack_list = self.engine_rpcapi.show_stack(con, None) except rpc_common.RemoteError as ex: return exception.map_remote_error(ex) @@ -201,9 +199,7 @@ class StackController(object): else: identity = None - stack_list = self.engine_rpcapi.show_stack(con, - stack_identity=identity, - params=parms) + stack_list = self.engine_rpcapi.show_stack(con, identity) except rpc_common.RemoteError as ex: return exception.map_remote_error(ex) @@ -337,9 +333,7 @@ class StackController(object): logger.info('get_template') try: identity = self._get_identity(con, req.params['StackName']) - templ = self.engine_rpcapi.get_template(con, - stack_identity=identity, - params=parms) + templ = self.engine_rpcapi.get_template(con, identity) except rpc_common.RemoteError as ex: return exception.map_remote_error(ex) @@ -377,16 +371,14 @@ class StackController(object): return exception.HeatMissingParameterError(detail=msg) try: - stack = json.loads(templ) + template = json.loads(templ) except ValueError: msg = _("The Template must be a JSON document.") return exception.HeatInvalidParameterValueError(detail=msg) logger.info('validate_template') try: - return self.engine_rpcapi.validate_template(con, - template=stack, - params=parms) + return self.engine_rpcapi.validate_template(con, template) except rpc_common.RemoteError as ex: return exception.map_remote_error(ex) @@ -400,10 +392,7 @@ class StackController(object): try: identity = self._get_identity(con, req.params['StackName']) - res = self.engine_rpcapi.delete_stack(con, - stack_identity=identity, - params=parms, - cast=False) + res = self.engine_rpcapi.delete_stack(con, identity, cast=False) except rpc_common.RemoteError as ex: return exception.map_remote_error(ex) @@ -445,9 +434,7 @@ class StackController(object): stack_name = req.params.get('StackName', None) try: identity = stack_name and self._get_identity(con, stack_name) - event_res = self.engine_rpcapi.list_events(con, - stack_identity=identity, - params=parms) + event_res = self.engine_rpcapi.list_events(con, identity) except rpc_common.RemoteError as ex: return exception.map_remote_error(ex) diff --git a/heat/engine/manager.py b/heat/engine/manager.py index 6bc31423..f4589bf8 100644 --- a/heat/engine/manager.py +++ b/heat/engine/manager.py @@ -92,12 +92,11 @@ class EngineManager(manager.Manager): return s - def show_stack(self, context, stack_identity, params): + def show_stack(self, context, stack_identity): """ The show_stack method returns the attributes of one stack. arg1 -> RPC context. arg2 -> Name of the stack you want to see, or None to see all - arg3 -> Dict of http request parameters passed in from API side. """ if stack_identity is not None: stacks = [self._get_stack(context, stack_identity)] @@ -189,7 +188,7 @@ class EngineManager(manager.Manager): arg1 -> RPC context. arg3 -> Template of stack you want to create. - arg4 -> Params passed from API. + arg4 -> Stack Input Params """ logger.info('validate_template') if template is None: @@ -221,24 +220,22 @@ class EngineManager(manager.Manager): } return {'ValidateTemplateResult': result} - def get_template(self, context, stack_identity, params): + def get_template(self, context, stack_identity): """ Get the template. arg1 -> RPC context. arg2 -> Name of the stack you want to see. - arg3 -> Dict of http request parameters passed in from API side. """ s = self._get_stack(context, stack_identity) if s: return s.raw_template.template return None - def delete_stack(self, context, stack_identity, params): + def delete_stack(self, context, stack_identity): """ The delete_stack method deletes a given stack. arg1 -> RPC context. arg2 -> Name of the stack you want to delete. - arg3 -> Params passed from API. """ st = self._get_stack(context, stack_identity) @@ -248,12 +245,11 @@ class EngineManager(manager.Manager): greenpool.spawn_n(stack.delete) return None - def list_events(self, context, stack_identity, params): + def list_events(self, context, stack_identity): """ The list_events method lists all events associated with a given stack. arg1 -> RPC context. arg2 -> Name of the stack you want to get events for. - arg3 -> Params passed from API. """ if stack_identity is not None: st = self._get_stack(context, stack_identity) diff --git a/heat/engine/rpcapi.py b/heat/engine/rpcapi.py index 15b038f8..7d0185f1 100644 --- a/heat/engine/rpcapi.py +++ b/heat/engine/rpcapi.py @@ -70,7 +70,7 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): stack_name=stack_name), topic=_engine_topic(self.topic, ctxt, None)) - def show_stack(self, ctxt, stack_identity, params): + def show_stack(self, ctxt, stack_identity): """ The show_stack method returns the attributes of one stack. @@ -80,7 +80,7 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): :param params: Dict of http request parameters passed in from API side. """ return self.call(ctxt, self.make_msg('show_stack', - stack_identity=stack_identity, params=params), + stack_identity=stack_identity), topic=_engine_topic(self.topic, ctxt, None)) def create_stack(self, ctxt, stack_name, template, params, args): @@ -98,7 +98,8 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): """ return self.call(ctxt, self.make_msg('create_stack', stack_name=stack_name, - template=template, params=params, args=args), + template=template, + params=params, args=args), topic=_engine_topic(self.topic, ctxt, None)) def update_stack(self, ctxt, stack_identity, template, params, args): @@ -115,8 +116,9 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): :param args: Request parameters/args passed from API """ return self.call(ctxt, self.make_msg('update_stack', - stack_identity=stack_identity, - template=template, params=params, args=args), + stack_identity=stack_identity, + template=template, + params=params, args=args), topic=_engine_topic(self.topic, ctxt, None)) def validate_template(self, ctxt, template, params): @@ -129,10 +131,10 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): :param params: Params passed from API. """ return self.call(ctxt, self.make_msg('validate_template', - template=template, params=params), + template=template, params=params), topic=_engine_topic(self.topic, ctxt, None)) - def get_template(self, ctxt, stack_identity, params): + def get_template(self, ctxt, stack_identity): """ Get the template. @@ -141,10 +143,10 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): :param params: Dict of http request parameters passed in from API side. """ return self.call(ctxt, self.make_msg('get_template', - stack_identity=stack_identity, params=params), + stack_identity=stack_identity), topic=_engine_topic(self.topic, ctxt, None)) - def delete_stack(self, ctxt, stack_identity, params, cast=True): + def delete_stack(self, ctxt, stack_identity, cast=True): """ The delete_stack method deletes a given stack. @@ -154,10 +156,10 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): """ rpc_method = self.cast if cast else self.call return rpc_method(ctxt, self.make_msg('delete_stack', - stack_identity=stack_identity, params=params), + stack_identity=stack_identity), topic=_engine_topic(self.topic, ctxt, None)) - def list_events(self, ctxt, stack_identity, params): + def list_events(self, ctxt, stack_identity): """ The list_events method lists all events associated with a given stack. @@ -166,13 +168,13 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): :param params: Params passed from API. """ return self.call(ctxt, self.make_msg('list_events', - stack_identity=stack_identity, params=params), + stack_identity=stack_identity), topic=_engine_topic(self.topic, ctxt, None)) def describe_stack_resource(self, ctxt, stack_identity, resource_name): return self.call(ctxt, self.make_msg('describe_stack_resource', - stack_identity=stack_identity, - resource_name=resource_name), + stack_identity=stack_identity, + resource_name=resource_name), topic=_engine_topic(self.topic, ctxt, None)) def describe_stack_resources(self, ctxt, stack_identity, @@ -185,7 +187,7 @@ class EngineAPI(heat.openstack.common.rpc.proxy.RpcProxy): def list_stack_resources(self, ctxt, stack_identity): return self.call(ctxt, self.make_msg('list_stack_resources', - stack_identity=stack_identity), + stack_identity=stack_identity), topic=_engine_topic(self.topic, ctxt, None)) def metadata_list_stacks(self, ctxt): diff --git a/heat/tests/test_api_cfn_v1.py b/heat/tests/test_api_cfn_v1.py index 9dab36f2..0f9a127e 100644 --- a/heat/tests/test_api_cfn_v1.py +++ b/heat/tests/test_api_cfn_v1.py @@ -96,8 +96,7 @@ class StackControllerTest(unittest.TestCase): u'stack_status': u'CREATE_COMPLETE'}]} self.m.StubOutWithMock(rpc, 'call') rpc.call(dummy_req.context, self.topic, {'method': 'show_stack', - 'args': {'stack_identity': None, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': None}, 'version': self.api_version}, None ).AndReturn(engine_resp) @@ -123,8 +122,7 @@ class StackControllerTest(unittest.TestCase): # heat exception type self.m.StubOutWithMock(rpc, 'call') rpc.call(dummy_req.context, self.topic, {'method': 'show_stack', - 'args': {'stack_identity': None, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': None}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("AttributeError")) @@ -143,8 +141,7 @@ class StackControllerTest(unittest.TestCase): # heat exception type self.m.StubOutWithMock(rpc, 'call') rpc.call(dummy_req.context, self.topic, {'method': 'show_stack', - 'args': {'stack_identity': None, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': None}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("Exception")) @@ -196,8 +193,7 @@ class StackControllerTest(unittest.TestCase): 'args': {'stack_name': stack_name}, 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'show_stack', - 'args': {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': identity}, 'version': self.api_version}, None).AndReturn(engine_resp) self.m.ReplayAll() @@ -279,8 +275,7 @@ class StackControllerTest(unittest.TestCase): self.m.StubOutWithMock(rpc, 'call') rpc.call(dummy_req.context, self.topic, {'method': 'show_stack', - 'args': {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': identity}, 'version': self.api_version}, None).AndReturn(engine_resp) self.m.ReplayAll() @@ -334,8 +329,7 @@ class StackControllerTest(unittest.TestCase): 'args': {'stack_name': stack_name}, 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'show_stack', - 'args': {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': identity}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("AttributeError")) @@ -595,8 +589,7 @@ class StackControllerTest(unittest.TestCase): 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'get_template', 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + {'stack_identity': identity}, 'version': self.api_version}, None).AndReturn(engine_resp) self.m.ReplayAll() @@ -623,8 +616,7 @@ class StackControllerTest(unittest.TestCase): 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'get_template', 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + {'stack_identity': identity}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("AttributeError")) @@ -671,8 +663,7 @@ class StackControllerTest(unittest.TestCase): 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'get_template', 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + {'stack_identity': identity}, 'version': self.api_version}, None).AndReturn(engine_resp) self.m.ReplayAll() @@ -716,9 +707,7 @@ class StackControllerTest(unittest.TestCase): 'version': self.api_version}, None).AndReturn(identity) # Engine returns None when delete successful rpc.call(dummy_req.context, self.topic, {'method': 'delete_stack', - 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': identity}, 'version': self.api_version}, None).AndReturn(None) self.m.ReplayAll() @@ -744,9 +733,7 @@ class StackControllerTest(unittest.TestCase): # Insert an engine RPC error and ensure we map correctly to the # heat exception type rpc.call(dummy_req.context, self.topic, {'method': 'delete_stack', - 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + 'args': {'stack_identity': identity}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("AttributeError")) @@ -805,8 +792,7 @@ class StackControllerTest(unittest.TestCase): 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'list_events', 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + {'stack_identity': identity}, 'version': self.api_version}, None).AndReturn(engine_resp) self.m.ReplayAll() @@ -843,8 +829,7 @@ class StackControllerTest(unittest.TestCase): 'version': self.api_version}, None).AndReturn(identity) rpc.call(dummy_req.context, self.topic, {'method': 'list_events', 'args': - {'stack_identity': identity, - 'params': dict(dummy_req.params)}, + {'stack_identity': identity}, 'version': self.api_version}, None ).AndRaise(rpc_common.RemoteError("Exception")) diff --git a/heat/tests/test_engine_manager.py b/heat/tests/test_engine_manager.py index 66d50acc..4ffaa895 100644 --- a/heat/tests/test_engine_manager.py +++ b/heat/tests/test_engine_manager.py @@ -215,8 +215,7 @@ class stackManagerCreateUpdateDeleteTest(unittest.TestCase): self.m.ReplayAll() - self.assertEqual(self.man.delete_stack(self.ctx, - stack.identifier(), {}), + self.assertEqual(self.man.delete_stack(self.ctx, stack.identifier()), None) self.m.VerifyAll() @@ -228,7 +227,7 @@ class stackManagerCreateUpdateDeleteTest(unittest.TestCase): self.assertRaises(AttributeError, self.man.delete_stack, - self.ctx, stack.identifier(), {}) + self.ctx, stack.identifier()) self.m.VerifyAll() def test_stack_update(self): @@ -379,7 +378,7 @@ class stackManagerTest(unittest.TestCase): self.ctx, self.stack_name, self.stack.t, {}, {}) def test_stack_event_list(self): - el = self.man.list_events(self.ctx, self.stack_identity, {}) + el = self.man.list_events(self.ctx, self.stack_identity) self.assertTrue('events' in el) events = el['events'] @@ -422,7 +421,7 @@ class stackManagerTest(unittest.TestCase): self.assertTrue('event_time' in ev) def test_stack_describe_all(self): - sl = self.man.show_stack(self.ctx, None, {}) + sl = self.man.show_stack(self.ctx, None) self.assertEqual(len(sl['stacks']), 1) for s in sl['stacks']: @@ -434,7 +433,7 @@ class stackManagerTest(unittest.TestCase): self.tenant = 'stack_describe_all_empty_tenant' self.setUp() - sl = self.man.show_stack(self.ctx, None, {}) + sl = self.man.show_stack(self.ctx, None) self.assertEqual(len(sl['stacks']), 0) @@ -443,10 +442,10 @@ class stackManagerTest(unittest.TestCase): nonexist['stack_name'] = 'wibble' self.assertRaises(AttributeError, self.man.show_stack, - self.ctx, nonexist, {}) + self.ctx, nonexist) def test_stack_describe(self): - sl = self.man.show_stack(self.ctx, self.stack_identity, {}) + sl = self.man.show_stack(self.ctx, self.stack_identity) self.assertEqual(len(sl['stacks']), 1) diff --git a/heat/tests/test_rpcapi.py b/heat/tests/test_rpcapi.py index b3d40de0..6fc8cce6 100644 --- a/heat/tests/test_rpcapi.py +++ b/heat/tests/test_rpcapi.py @@ -87,8 +87,7 @@ class EngineRpcAPITestCase(unittest.TestCase): self.assertEqual(arg, expected_arg) def test_show_stack(self): - self._test_engine_api('show_stack', 'call', stack_identity='wordpress', - params={'Action': 'ListStacks'}) + self._test_engine_api('show_stack', 'call', stack_identity='wordpress') def test_create_stack(self): self._test_engine_api('create_stack', 'call', stack_name='wordpress', @@ -109,19 +108,19 @@ class EngineRpcAPITestCase(unittest.TestCase): def test_get_template(self): self._test_engine_api('get_template', 'call', - stack_identity='wordpress', params={}) + stack_identity='wordpress') def test_delete_stack_cast(self): self._test_engine_api('delete_stack', 'cast', - stack_identity='wordpress', params={}) + stack_identity='wordpress') def test_delete_stack_call(self): self._test_engine_api('delete_stack', 'call', - stack_identity='wordpress', params={}) + stack_identity='wordpress') def test_list_events(self): self._test_engine_api('list_events', 'call', - stack_identity='wordpress', params={}) + stack_identity='wordpress') def test_describe_stack_resource(self): self._test_engine_api('describe_stack_resource', 'call', -- 2.45.2