From 41ae901c3bfe7c9ea86dd38787a8b3e89aecc8a3 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Mon, 1 May 2017 17:26:05 -0400 Subject: [PATCH] Get rid of resource_validate flag in Stack Why pass a parameter to a method to influence what it does when you can set some state on the object that the programmer always has to keep in their head to figure out what any given call will do? Change-Id: I020238de0a351dd2eaf51c6f970ae36ad0f931be --- heat/engine/resource.py | 2 -- heat/engine/service.py | 7 +++---- heat/engine/stack.py | 24 +++++++----------------- heat/tests/test_engine_service.py | 3 +-- heat/tests/test_resource.py | 2 -- heat/tests/test_stack.py | 10 ++++------ 6 files changed, 15 insertions(+), 33 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 7a83bdec3a..bdce34d9f5 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -220,8 +220,6 @@ class Resource(status.ResourceStatus): self.context = stack.context self.name = name self.t = definition - # Only translate in cases where resource_validate is True - # ex. for template-validate self.reparse(client_resolve=False) self.update_policy = self.t.update_policy(self.update_policy_schema, self.context) diff --git a/heat/engine/service.py b/heat/engine/service.py index d1fe36d71e..bc2e7d6457 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1231,11 +1231,11 @@ class EngineService(service.ServiceBase): stack_name = 'dummy' stack = parser.Stack(cnxt, stack_name, tmpl, strict_validate=False, - resource_validate=False, service_check_defer=service_check_defer) try: stack.validate(ignorable_errors=ignorable_errors, - validate_by_deps=False) + validate_by_deps=False, + validate_res_tmpl_only=True) except exception.StackValidationFailed as ex: return {'Error': six.text_type(ex)} @@ -2428,8 +2428,7 @@ class EngineService(service.ServiceBase): continue stk = parser.Stack.load(cnxt, stack=s, - service_check_defer=True, - resource_validate=False) + service_check_defer=True) LOG.info('Engine %(engine)s went down when stack ' '%(stack_id)s was in action %(action)s', {'engine': engine_id, 'action': stk.action, diff --git a/heat/engine/stack.py b/heat/engine/stack.py index f89cfc9f43..2d379b438c 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -124,7 +124,7 @@ class Stack(collections.Mapping): use_stored_context=False, username=None, nested_depth=0, strict_validate=True, convergence=False, current_traversal=None, tags=None, prev_raw_template_id=None, - current_deps=None, cache_data=None, resource_validate=True, + current_deps=None, cache_data=None, service_check_defer=False, deleted_time=None): """Initialise the Stack. @@ -192,13 +192,6 @@ class Stack(collections.Mapping): # for not-yet-created resources (which return None) self.strict_validate = strict_validate - # resource_validate can be used to disable resource plugin subclass - # validate methods, which is useful when you want to validate - # template integrity but some parameters may not be provided - # at all, thus we can't yet reference property values such as is - # commonly done in plugin validate() methods - self.resource_validate = resource_validate - # service_check_defer can be used to defer the validation of service # availability for a given resource, which helps to create the resource # dependency tree completely when respective service is not available, @@ -509,8 +502,7 @@ class Stack(collections.Mapping): @classmethod def load(cls, context, stack_id=None, stack=None, show_deleted=True, use_stored_context=False, force_reload=False, cache_data=None, - service_check_defer=False, resource_validate=True, - load_template=True): + service_check_defer=False, load_template=True): """Retrieve a Stack from the database.""" if stack is None: stack = stack_object.Stack.get_by_id( @@ -528,7 +520,6 @@ class Stack(collections.Mapping): use_stored_context=use_stored_context, cache_data=cache_data, service_check_defer=service_check_defer, - resource_validate=resource_validate, load_template=load_template) @classmethod @@ -563,8 +554,7 @@ class Stack(collections.Mapping): @classmethod def _from_db(cls, context, stack, use_stored_context=False, cache_data=None, - service_check_defer=False, resource_validate=True, - load_template=True): + service_check_defer=False, load_template=True): if load_template: template = tmpl.Template.load( context, stack.raw_template_id, stack.raw_template) @@ -589,8 +579,7 @@ class Stack(collections.Mapping): current_deps=stack.current_deps, cache_data=cache_data, nested_depth=stack.nested_depth, deleted_time=stack.deleted_at, - service_check_defer=service_check_defer, - resource_validate=resource_validate) + service_check_defer=service_check_defer) def get_kwargs_for_cloning(self, keep_status=False, only_db=False): """Get common kwargs for calling Stack() for cloning. @@ -797,7 +786,8 @@ class Stack(collections.Mapping): return handler and handler(resource_name) @profiler.trace('Stack.validate', hide_args=False) - def validate(self, ignorable_errors=None, validate_by_deps=True): + def validate(self, ignorable_errors=None, validate_by_deps=True, + validate_res_tmpl_only=False): """Validates the stack.""" # TODO(sdake) Should return line number of invalid reference @@ -852,7 +842,7 @@ class Stack(collections.Mapping): if res.name not in unique_defn_names: continue try: - if self.resource_validate: + if not validate_res_tmpl_only: if res.external_id is not None: res.validate_external() continue diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index a484e7921b..a160b45387 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -1338,8 +1338,7 @@ class StackServiceTest(common.HeatTestCase): ]) mock_stack_load.assert_called_once_with(self.ctx, stack=db_stack, - service_check_defer=True, - resource_validate=False) + service_check_defer=True) self.assertTrue(lock2.release.called) reason = ('Engine went down during stack %s' % fake_stack.action) mock_thread.start_with_acquired_lock.assert_called_once_with( diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 38b5c2828e..a2f4463abc 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -193,9 +193,7 @@ class ResourceTest(common.HeatTestCase): self.assertEqual(0, mock_load.call_count) # set stack._resources = None to reload the resources - # and set resource_validate = False stack._resources = None - stack.resource_validate = False mock_db_get.return_value = mock.Mock() self.assertEqual(1, len(stack.resources)) self.assertEqual(2, mock_translate.call_count) diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py index 67b3daecc8..2e8a36ffee 100644 --- a/heat/tests/test_stack.py +++ b/heat/tests/test_stack.py @@ -445,8 +445,7 @@ class StackTest(common.HeatTestCase): current_deps=None, cache_data=None, nested_depth=0, deleted_time=None, - service_check_defer=False, - resource_validate=True) + service_check_defer=False) self.m.ReplayAll() stack.Stack.load(self.ctx, stack_id=self.stack.id) @@ -2029,8 +2028,7 @@ class StackTest(common.HeatTestCase): self.assertIn("The Parameter (aparam) was not provided", six.text_type(ex)) - self.stack.resource_validate = False - self.assertIsNone(self.stack.validate()) + self.assertIsNone(self.stack.validate(validate_res_tmpl_only=True)) def test_nodisable_validate_tmpl_err(self): tmpl = template_format.parse(""" @@ -2058,9 +2056,9 @@ class StackTest(common.HeatTestCase): "The specified reference \"noexist\" (in AResource) is incorrect", six.text_type(ex)) - self.stack.resource_validate = False ex = self.assertRaises(exception.InvalidTemplateReference, - self.stack.validate) + self.stack.validate, + validate_res_tmpl_only=True) self.assertIn( "The specified reference \"noexist\" (in AResource) is incorrect", six.text_type(ex))