From 5ed06a9b780a4058fb0dcae253e16e55dd6c3912 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 20 Nov 2015 22:00:59 -0500 Subject: Revert "Fix wrong type of exception raised" This reverts commit c5bb4d2ab83820f5ee2ac3fc6e8d9210e98b26ef. Every part of this patch was wrong, except that it did (by accident) start returning 500 errors instead of 400 errors in the case where a template in the global environment was not found, which is actually the operator's fault and not the user's. That's not really a serious enough problem to justify changing it on a stable branch though, so this is a straight revert. Luckily the patch hasn't made it into a release yet. Change-Id: Ia3154c7c41bb69cfa2ee36eb6d48fcd339900d60 Closes-Bug: #1518458 Related-Bug: #1447194 --- heat/common/exception.py | 8 -------- heat/engine/environment.py | 9 ++++---- heat/engine/resource.py | 2 +- .../resources/openstack/heat/resource_group.py | 2 +- heat/engine/resources/template_resource.py | 6 +++--- heat/engine/service.py | 16 +++++++-------- heat/tests/test_engine_service.py | 2 +- heat/tests/test_provider_template.py | 8 ++++---- heat/tests/test_resource.py | 6 +++--- heat/tests/test_resource_group.py | 5 ++--- heat/tests/test_stack_resource.py | 24 ++++++++-------------- 11 files changed, 37 insertions(+), 51 deletions(-) diff --git a/heat/common/exception.py b/heat/common/exception.py index ba1d85db2..7d4ff08a6 100644 --- a/heat/common/exception.py +++ b/heat/common/exception.py @@ -319,18 +319,10 @@ class ResourceNotFound(HeatException): "in Stack %(stack_name)s.") -class TemplateNotFound(HeatException): - msg_fmt = _("%(message)s") - - class ResourceTypeNotFound(HeatException): msg_fmt = _("The Resource Type (%(type_name)s) could not be found.") -class InvalidResourceType(HeatException): - msg_fmt = _("%(message)s") - - class ResourceNotAvailable(HeatException): msg_fmt = _("The Resource (%(resource_name)s) is not available.") diff --git a/heat/engine/environment.py b/heat/engine/environment.py index 0018b97ba..bd81c8677 100644 --- a/heat/engine/environment.py +++ b/heat/engine/environment.py @@ -400,19 +400,20 @@ class ResourceRegistry(object): def get_class(self, resource_type, resource_name=None): if resource_type == "": msg = _('Resource "%s" has no type') % resource_name - raise exception.InvalidResourceType(message=msg) + raise exception.StackValidationFailed(message=msg) elif resource_type is None: msg = _('Non-empty resource type is required ' 'for resource "%s"') % resource_name - raise exception.InvalidResourceType(message=msg) + raise exception.StackValidationFailed(message=msg) elif not isinstance(resource_type, six.string_types): msg = _('Resource "%s" type is not a string') % resource_name - raise exception.InvalidResourceType(message=msg) + raise exception.StackValidationFailed(message=msg) info = self.get_resource_info(resource_type, resource_name=resource_name) if info is None: - raise exception.ResourceTypeNotFound(type_name=resource_type) + msg = _("Unknown resource Type : %s") % resource_type + raise exception.StackValidationFailed(message=msg) return info.get_class() def as_dict(self): diff --git a/heat/engine/resource.py b/heat/engine/resource.py index ca969510d..d8079a48a 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -143,7 +143,7 @@ class Resource(object): try: ResourceClass = registry.get_class(definition.resource_type, resource_name=name) - except exception.TemplateNotFound: + except exception.NotFound: ResourceClass = template_resource.TemplateResource assert issubclass(ResourceClass, Resource) diff --git a/heat/engine/resources/openstack/heat/resource_group.py b/heat/engine/resources/openstack/heat/resource_group.py index ca958f4c9..ddb6dd91f 100644 --- a/heat/engine/resources/openstack/heat/resource_group.py +++ b/heat/engine/resources/openstack/heat/resource_group.py @@ -195,7 +195,7 @@ class ResourceGroup(stack_resource.StackResource): # make sure we can resolve the nested resource type try: self.stack.env.get_class(res_def.resource_type) - except exception.TemplateNotFound: + except exception.NotFound: # its a template resource pass diff --git a/heat/engine/resources/template_resource.py b/heat/engine/resources/template_resource.py index 84f68d25f..266bec101 100644 --- a/heat/engine/resources/template_resource.py +++ b/heat/engine/resources/template_resource.py @@ -88,7 +88,7 @@ class TemplateResource(stack_resource.StackResource): args = {'name': template_name, 'exc': six.text_type(r_exc)} msg = _('Could not fetch remote template ' '"%(name)s": %(exc)s') % args - raise exception.TemplateNotFound(message=msg) + raise exception.NotFound(msg_fmt=msg) @staticmethod def get_schemas(tmpl, param_defaults): @@ -101,7 +101,7 @@ class TemplateResource(stack_resource.StackResource): self._parsed_nested = None try: tmpl = template.Template(self.child_template()) - except (exception.TemplateNotFound, ValueError) as download_error: + except (exception.NotFound, ValueError) as download_error: self.validation_exception = download_error tmpl = template.Template( {"HeatTemplateFormatVersion": "2012-12-12"}) @@ -187,7 +187,7 @@ class TemplateResource(stack_resource.StackResource): try: t_data = self.get_template_file(self.template_name, self.allowed_schemes) - except exception.TemplateNotFound as err: + except exception.NotFound as err: if self.action == self.UPDATE: raise reported_excp = err diff --git a/heat/engine/service.py b/heat/engine/service.py index a2cc0e0c3..069d6d3ef 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -989,10 +989,10 @@ class EngineService(service.Service): """ try: resource_class = resources.global_env().get_class(type_name) - except (exception.InvalidResourceType, - exception.ResourceTypeNotFound, - exception.TemplateNotFound) as ex: - raise ex + except exception.StackValidationFailed: + raise exception.ResourceTypeNotFound(type_name=type_name) + except exception.NotFound as ex: + raise exception.StackValidationFailed(message=ex.message) def properties_schema(): for name, schema_dict in resource_class.properties_schema.items(): @@ -1021,10 +1021,10 @@ class EngineService(service.Service): try: return resources.global_env().get_class( type_name).resource_to_template(type_name) - except (exception.InvalidResourceType, - exception.ResourceTypeNotFound, - exception.TemplateNotFound) as ex: - raise ex + except exception.StackValidationFailed: + raise exception.ResourceTypeNotFound(type_name=type_name) + except exception.NotFound as ex: + raise exception.StackValidationFailed(message=ex.message) @context.request_context def list_events(self, cnxt, stack_identity, filters=None, limit=None, diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index aa9411384..b35a7f793 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -2409,7 +2409,7 @@ class StackServiceTest(common.HeatTestCase): mock_iterable = mock.MagicMock(return_value=iter([info])) with mock.patch('heat.engine.environment.ResourceRegistry.iterable_by', new=mock_iterable): - ex = self.assertRaises(exception.TemplateNotFound, + ex = self.assertRaises(exception.StackValidationFailed, function, self.ctx, type_name='ResourceWithWrongRefOnFile') diff --git a/heat/tests/test_provider_template.py b/heat/tests/test_provider_template.py index 67044a0a5..068468dcc 100644 --- a/heat/tests/test_provider_template.py +++ b/heat/tests/test_provider_template.py @@ -562,7 +562,7 @@ class ProviderTemplateTest(common.HeatTestCase): env_str = {'resource_registry': {'resources': {'fred': { "OS::ResourceType": "some_magic.yaml"}}}} env = environment.Environment(env_str) - ex = self.assertRaises(exception.TemplateNotFound, env.get_class, + ex = self.assertRaises(exception.NotFound, env.get_class, 'OS::ResourceType', 'fred') self.assertIn('Could not fetch remote template "some_magic.yaml"', six.text_type(ex)) @@ -887,14 +887,14 @@ class TemplateDataTest(common.HeatTestCase): self.res.action = self.res.UPDATE self.res.nested = mock.MagicMock() self.res.get_template_file = mock.Mock( - side_effect=exception.TemplateNotFound(message='not found')) - self.assertRaises(exception.TemplateNotFound, self.res.template_data) + side_effect=exception.NotFound()) + self.assertRaises(exception.NotFound, self.res.template_data) def test_template_data_in_create_without_template_file(self): self.res.action = self.res.CREATE self.res.nested = mock.MagicMock() self.res.get_template_file = mock.Mock( - side_effect=exception.TemplateNotFound(message='not found')) + side_effect=exception.NotFound()) self.assertEqual('{}', self.res.template_data()) diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 719c6cb84..2dda46eab 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -71,7 +71,7 @@ class ResourceTest(common.HeatTestCase): self.assertEqual(generic_rsrc.GenericResource, cls) def test_get_class_noexist(self): - self.assertRaises(exception.ResourceTypeNotFound, + self.assertRaises(exception.StackValidationFailed, resources.global_env().get_class, 'NoExistResourceType') @@ -105,13 +105,13 @@ class ResourceTest(common.HeatTestCase): def test_resource_new_err(self): snippet = rsrc_defn.ResourceDefinition('aresource', 'NoExistResourceType') - self.assertRaises(exception.ResourceTypeNotFound, + self.assertRaises(exception.StackValidationFailed, resource.Resource, 'aresource', snippet, self.stack) def test_resource_non_type(self): resource_name = 'aresource' snippet = rsrc_defn.ResourceDefinition(resource_name, '') - ex = self.assertRaises(exception.InvalidResourceType, + ex = self.assertRaises(exception.StackValidationFailed, resource.Resource, resource_name, snippet, self.stack) self.assertIn(_('Resource "%s" has no type') % resource_name, diff --git a/heat/tests/test_resource_group.py b/heat/tests/test_resource_group.py index 6640a97b0..e7c23da64 100644 --- a/heat/tests/test_resource_group.py +++ b/heat/tests/test_resource_group.py @@ -308,10 +308,9 @@ class ResourceGroupTest(common.HeatTestCase): stack = utils.parse_stack(tmp) snip = stack.t.resource_definitions(stack)['group1'] resg = resource_group.ResourceGroup('test', snip, stack) - exc = self.assertRaises(exception.ResourceTypeNotFound, + exc = self.assertRaises(exception.StackValidationFailed, resg.validate) - exp_msg = 'The Resource Type (idontexist) could not be found.' - self.assertIn(exp_msg, six.text_type(exc)) + self.assertIn('Unknown resource Type', six.text_type(exc)) def test_reference_attr(self): stack = utils.parse_stack(template2) diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index 6f4fd7255..e957dd17f 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -379,29 +379,23 @@ class StackResourceTest(common.HeatTestCase): rsrc.validate) self.assertIn(raise_exc_msg, six.text_type(exc)) - def _test_validate_unknown_resource_type(self, stack_name, tmpl, - resource_name, - stack_resource=True): - raise_exc_msg = ('The Resource Type (idontexist) could not be found.') + def _test_validate_unknown_resource_type(self, stack_name, + tmpl, resource_name): + raise_exc_msg = ('Unknown resource Type : idontexist') stack = parser.Stack(utils.dummy_context(), stack_name, tmpl) rsrc = stack[resource_name] - if stack_resource: - exc = self.assertRaises(exception.StackValidationFailed, - rsrc.validate) - else: - exc = self.assertRaises(exception.ResourceTypeNotFound, - rsrc.validate) + + exc = self.assertRaises(exception.StackValidationFailed, + rsrc.validate) self.assertIn(raise_exc_msg, six.text_type(exc)) def test_validate_resource_group(self): - # resource group validate without nested template is a normal - # resource validation + # test validate without nested template stack_name = 'validate_resource_group_template' t = template_format.parse(resource_group_template) tmpl = templatem.Template(t) self._test_validate_unknown_resource_type(stack_name, tmpl, - 'my_resource_group', - stack_resource=False) + 'my_resource_group') # validate with nested template res_prop = t['resources']['my_resource_group']['properties'] @@ -412,7 +406,7 @@ class StackResourceTest(common.HeatTestCase): 'my_resource_group') def test_validate_heat_autoscaling_group(self): - # Autoscaling validation is a nested stack validation + # test validate without nested template stack_name = 'validate_heat_autoscaling_group_template' t = template_format.parse(heat_autoscaling_group_template) tmpl = templatem.Template(t) -- cgit v1.2.1