Moving handle_hooks to try and make #708 easier to review (#724)

Author: phobologic

stacker/actions/build.py

@@ -7,7 +7,7 @@
 from .base import STACK_POLL_TIME
 
 from ..providers.base import Template
-from .. import util
+from stacker.hooks import utils
 from ..exceptions import (
     MissingParameterException,
     StackDidNotChange,
@@ -196,7 +196,7 @@ def handle_hooks(stage, hooks, provider, context, dump, outline):
 
     """
     if not outline and not dump and hooks:
-        util.handle_hooks(
+        utils.handle_hooks(
             stage=stage,
             hooks=hooks,
             provider=provider,

stacker/actions/destroy.py

@@ -6,7 +6,7 @@
 from .base import BaseAction, plan, build_walker
 from .base import STACK_POLL_TIME
 from ..exceptions import StackDoesNotExist
-from .. import util
+from stacker.hooks.utils import handle_hooks
 from ..status import (
     CompleteStatus,
     SubmittedStatus,
@@ -82,7 +82,7 @@ def pre_run(self, outline=False, *args, **kwargs):
         """Any steps that need to be taken prior to running the action."""
         pre_destroy = self.context.config.pre_destroy
         if not outline and pre_destroy:
-            util.handle_hooks(
+            handle_hooks(
                 stage="pre_destroy",
                 hooks=pre_destroy,
                 provider=self.provider,
@@ -106,7 +106,7 @@ def post_run(self, outline=False, *args, **kwargs):
         """Any steps that need to be taken after running the action."""
         post_destroy = self.context.config.post_destroy
         if not outline and post_destroy:
-            util.handle_hooks(
+            handle_hooks(
                 stage="post_destroy",
                 hooks=post_destroy,
                 provider=self.provider,

stacker/hooks/utils.py

@@ -2,7 +2,82 @@
 from __future__ import division
 from __future__ import absolute_import
 import os
+import sys
+import collections
+import logging
+
+from stacker.util import load_object_from_string
+
+logger = logging.getLogger(__name__)
 
 
 def full_path(path):
     return os.path.abspath(os.path.expanduser(path))
+
+
+def handle_hooks(stage, hooks, provider, context):
+    """ Used to handle pre/post_build hooks.
+
+    These are pieces of code that we want to run before/after the builder
+    builds the stacks.
+
+    Args:
+        stage (string): The current stage (pre_run, post_run, etc).
+        hooks (list): A list of :class:`stacker.config.Hook` containing the
+            hooks to execute.
+        provider (:class:`stacker.provider.base.BaseProvider`): The provider
+            the current stack is using.
+        context (:class:`stacker.context.Context`): The current stacker
+            context.
+    """
+    if not hooks:
+        logger.debug("No %s hooks defined.", stage)
+        return
+
+    hook_paths = []
+    for i, h in enumerate(hooks):
+        try:
+            hook_paths.append(h.path)
+        except KeyError:
+            raise ValueError("%s hook #%d missing path." % (stage, i))
+
+    logger.info("Executing %s hooks: %s", stage, ", ".join(hook_paths))
+    for hook in hooks:
+        data_key = hook.data_key
+        required = hook.required
+        kwargs = hook.args or {}
+        enabled = hook.enabled
+        if not enabled:
+            logger.debug("hook with method %s is disabled, skipping",
+                         hook.path)
+            continue
+        try:
+            method = load_object_from_string(hook.path)
+        except (AttributeError, ImportError):
+            logger.exception("Unable to load method at %s:", hook.path)
+            if required:
+                raise
+            continue
+        try:
+            result = method(context=context, provider=provider, **kwargs)
+        except Exception:
+            logger.exception("Method %s threw an exception:", hook.path)
+            if required:
+                raise
+            continue
+        if not result:
+            if required:
+                logger.error("Required hook %s failed. Return value: %s",
+                             hook.path, result)
+                sys.exit(1)
+            logger.warning("Non-required hook %s failed. Return value: %s",
+                           hook.path, result)
+        else:
+            if isinstance(result, collections.Mapping):
+                if data_key:
+                    logger.debug("Adding result for hook %s to context in "
+                                 "data_key %s.", hook.path, data_key)
+                    context.set_hook_data(data_key, result)
+                else:
+                    logger.debug("Hook %s returned result data, but no data "
+                                 "key set, so ignoring.", hook.path)

stacker/tests/test_context.py

@@ -5,7 +5,7 @@
 
 from stacker.context import Context, get_fqn
 from stacker.config import load, Config
-from stacker.util import handle_hooks
+from stacker.hooks.utils import handle_hooks
 
 
 class TestContext(unittest.TestCase):

stacker/tests/test_util.py

@@ -19,7 +19,6 @@
     cf_safe_name,
     load_object_from_string,
     camel_to_snake,
-    handle_hooks,
     merge_map,
     yaml_to_ordered_dict,
     get_client_region,
@@ -33,6 +32,8 @@
     SourceProcessor
 )
 
+from stacker.hooks.utils import handle_hooks
+
 from .factories import (
     mock_context,
     mock_provider,

stacker/util.py

@@ -16,7 +16,6 @@
 import tempfile
 import zipfile
 
-import collections
 from collections import OrderedDict
 
 import botocore.client
@@ -337,74 +336,6 @@ def cf_safe_name(name):
     return "".join([uppercase_first_letter(part) for part in parts])
 
 
-def handle_hooks(stage, hooks, provider, context):
-    """ Used to handle pre/post_build hooks.
-
-    These are pieces of code that we want to run before/after the builder
-    builds the stacks.
-
-    Args:
-        stage (string): The current stage (pre_run, post_run, etc).
-        hooks (list): A list of :class:`stacker.config.Hook` containing the
-            hooks to execute.
-        provider (:class:`stacker.provider.base.BaseProvider`): The provider
-            the current stack is using.
-        context (:class:`stacker.context.Context`): The current stacker
-            context.
-    """
-    if not hooks:
-        logger.debug("No %s hooks defined.", stage)
-        return
-
-    hook_paths = []
-    for i, h in enumerate(hooks):
-        try:
-            hook_paths.append(h.path)
-        except KeyError:
-            raise ValueError("%s hook #%d missing path." % (stage, i))
-
-    logger.info("Executing %s hooks: %s", stage, ", ".join(hook_paths))
-    for hook in hooks:
-        data_key = hook.data_key
-        required = hook.required
-        kwargs = hook.args or {}
-        enabled = hook.enabled
-        if not enabled:
-            logger.debug("hook with method %s is disabled, skipping",
-                         hook.path)
-            continue
-        try:
-            method = load_object_from_string(hook.path)
-        except (AttributeError, ImportError):
-            logger.exception("Unable to load method at %s:", hook.path)
-            if required:
-                raise
-            continue
-        try:
-            result = method(context=context, provider=provider, **kwargs)
-        except Exception:
-            logger.exception("Method %s threw an exception:", hook.path)
-            if required:
-                raise
-            continue
-        if not result:
-            if required:
-                logger.error("Required hook %s failed. Return value: %s",
-                             hook.path, result)
-                sys.exit(1)
-            logger.warning("Non-required hook %s failed. Return value: %s",
-                           hook.path, result)
-        else:
-            if isinstance(result, collections.Mapping):
-                if data_key:
-                    logger.debug("Adding result for hook %s to context in "
-                                 "data_key %s.", hook.path, data_key)
-                    context.set_hook_data(data_key, result)
-                else:
-                    logger.debug("Hook %s returned result data, but no data "
-                                 "key set, so ignoring.", hook.path)
-
-
 def get_config_directory():
     """Return the directory the config file is located in.