Python runner actions datastore access changes, action unit testing fixes and lint cleanup

CHANGELOG.rst

@@ -8,7 +8,11 @@ in development
 * Allow user to pass a boolean value for the ``cacert`` st2client constructor argument. This way
   it now mimics the behavior of the ``verify`` argument of the ``requests.request`` method.
   (improvement)
-* Add datastore access to Python actions. (new-feature) #2396 [Kale Blankenship]
+* Add datastore access to Python runner actions via the ``action_service`` which is available
+  to all the Python runner actions after instantiation. (new-feature) #2396 #2511
+  [Kale Blankenship]
+* Update ``st2actions.runners.pythonrunner.Action`` class so the constructor also takes
+  ``action_service`` as the second argument.
 * Allow /v1/webhooks API endpoint request body to either be JSON or url encoded form data.
   Request body type is determined and parsed accordingly based on the value of
   ``Content-Type`` header.

Makefile

@@ -68,9 +68,10 @@ pylint: requirements .pylint
 		. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models $$component/$$component || exit 1; \
 	done
 	# Lint Python pack management actions
-	. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models contrib/packs/actions/pack_mgmt/ || exit 1;
+	. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models contrib/packs/actions/ || exit 1;
 	# Lint other packs
 	. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models contrib/linux || exit 1;
+	. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models contrib/chatops || exit 1;
 	# Lint Python scripts
 	. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models scripts/*.py || exit 1;
 	. $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models tools/*.py || exit 1;
@@ -84,8 +85,9 @@ flake8: requirements .flake8
 	@echo "==================== flake ===================="
 	@echo
 	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 $(COMPONENTS)
-	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 contrib/packs/actions/pack_mgmt/
+	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 contrib/packs/actions/
 	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 contrib/linux
+	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 contrib/chatops/
 	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 scripts/
 	. $(VIRTUALENV_DIR)/bin/activate; flake8 --config ./lint-configs/python/.flake8 tools/
 

contrib/chatops/actions/format_execution_result.py

@@ -7,8 +7,8 @@
 
 
 class FormatResultAction(Action):
-    def __init__(self, config):
-        super(FormatResultAction, self).__init__(config)
+    def __init__(self, config=None, action_service=None):
+        super(FormatResultAction, self).__init__(config=config, action_service=action_service)
         api_url = os.environ.get('ST2_ACTION_API_URL', None)
         token = os.environ.get('ST2_ACTION_AUTH_TOKEN', None)
         self.client = Client(api_url=api_url, token=token)
@@ -51,4 +51,4 @@ def _get_execution(self, execution_id):
         if not execution:
             return None
         excludes = ["trigger", "trigger_type", "trigger_instance", "liveaction"]
-        return execution.to_dict(exclude_attributes= excludes)
+        return execution.to_dict(exclude_attributes=excludes)

contrib/examples/actions/pythonactions/isprime.py

@@ -1,12 +1,11 @@
 import math
 
+from st2actions.runners.pythonrunner import Action
 
-class PrimeChecker(object):
-
-    def __init__(self, config=None):
-        pass
 
+class PrimeCheckerAction(Action):
     def run(self, value=0):
+        self.logger.debug('value=%s' % (value))
         if math.floor(value) != value:
             raise ValueError('%s should be an integer.' % value)
         if value < 2:
@@ -17,6 +16,6 @@ def run(self, value=0):
         return True
 
 if __name__ == '__main__':
-    checker = PrimeChecker()
+    checker = PrimeCheckerAction()
     for i in range(0, 10):
-        print '%s : %s' % (i, checker.run(**{'value': i}))
+        print '%s : %s' % (i, checker.run(value=1))

contrib/examples/tests/test_action_isprime.py

@@ -0,0 +1,15 @@
+from st2tests.base import BaseActionTestCase
+
+from pythonactions.isprime import PrimeCheckerAction
+
+
+class PrimeCheckerActionTestCase(BaseActionTestCase):
+    action_cls = PrimeCheckerAction
+
+    def test_run(self):
+        action = self.get_action_instance()
+        result = action.run(value=1)
+        self.assertFalse(result)
+
+        result = action.run(value=3)
+        self.assertTrue(result)

contrib/packs/actions/check_auto_deploy_repo.py

@@ -15,17 +15,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import sys
-import requests
-import json
-import getopt
-import argparse
-import os
-import yaml
-
-from getpass import getpass
 from st2actions.runners.pythonrunner import Action
 
+
 class CheckAutoDeployRepo(Action):
     def run(self, branch, repo_name):
         """Returns the required data to complete an auto deployment of a pack in repo_name.
@@ -42,12 +34,14 @@ def run(self, branch, repo_name):
         results = {}
 
         try:
-            results['deployment_branch'] = self.config["repositories"][repo_name]["auto_deployment"]["branch"]
-            results['notify_channel'] = self.config["repositories"][repo_name]["auto_deployment"]["notify_channel"]
+            repo_config = self.config["repositories"][repo_name]
+            results['deployment_branch'] = repo_config["auto_deployment"]["branch"]
+            results['notify_channel'] = repo_config["auto_deployment"]["notify_channel"]
         except KeyError:
             raise ValueError("No repositories or auto_deployment config for '%s'" % repo_name)
         else:
             if branch == "refs/heads/%s" % results['deployment_branch']:
                 return results
             else:
-                raise ValueError("Branch %s for %s should not be auto deployed" % (branch, repo_name))
+                raise ValueError("Branch %s for %s should not be auto deployed" %
+                                 (branch, repo_name))

contrib/packs/actions/expand_repo_name.py

@@ -15,17 +15,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import sys
-import requests
-import json
-import getopt
-import argparse
-import os
-import yaml
-
-from getpass import getpass
 from st2actions.runners.pythonrunner import Action
 
+
 class ExpandRepoName(Action):
     def run(self, repo_name):
         """Returns the data required to install packs from repo_name.

contrib/packs/actions/pack_mgmt/delete.py

@@ -26,8 +26,8 @@
 
 
 class UninstallPackAction(Action):
-    def __init__(self, config=None):
-        super(UninstallPackAction, self).__init__(config=config)
+    def __init__(self, config=None, action_service=None):
+        super(UninstallPackAction, self).__init__(config=config, action_service=action_service)
         self._base_virtualenvs_path = os.path.join(cfg.CONF.system.base_path,
                                                    'virtualenvs/')
 

contrib/packs/actions/pack_mgmt/download.py

@@ -61,8 +61,8 @@
 
 
 class DownloadGitRepoAction(Action):
-    def __init__(self, config=None):
-        super(DownloadGitRepoAction, self).__init__(config=config)
+    def __init__(self, config=None, action_service=None):
+        super(DownloadGitRepoAction, self).__init__(config=config, action_service=action_service)
         self._subtree = None
         self._repo_url = None
 

contrib/packs/actions/pack_mgmt/setup_virtualenv.py

@@ -43,8 +43,9 @@ class SetupVirtualEnvironmentAction(Action):
     current dependencies as well as an installation of new dependencies
     """
 
-    def __init__(self, config=None):
-        super(SetupVirtualEnvironmentAction, self).__init__(config=config)
+    def __init__(self, config=None, action_service=None):
+        super(SetupVirtualEnvironmentAction, self).__init__(config=config,
+                                                            action_service=action_service)
         self._base_virtualenvs_path = os.path.join(cfg.CONF.system.base_path,
                                                    'virtualenvs/')
 

contrib/packs/actions/pack_mgmt/unload.py

@@ -31,8 +31,8 @@
 
 
 class UnregisterPackAction(BaseAction):
-    def __init__(self, config=None):
-        super(UnregisterPackAction, self).__init__(config=config)
+    def __init__(self, config=None, action_service=None):
+        super(UnregisterPackAction, self).__init__(config=config, action_service=action_service)
         self.initialize()
 
     def initialize(self):

contrib/packs/tests/test_action_check_auto_deploy_repo.py

@@ -32,30 +32,32 @@
 """
 
 class CheckAutoDeployRepoActionTestCase(BaseActionTestCase):
+    action_cls = CheckAutoDeployRepo
+
     def test_run_config_blank(self):
         config = yaml.safe_load(MOCK_CONFIG_BLANK)
-        action = CheckAutoDeployRepo(config)
+        action = self.get_action_instance(config=config)
 
         self.assertRaises(Exception, action.run,
                           branch="refs/heads/master", repo_name="st2contrib")
 
     def test_run_repositories_blank(self):
         config = yaml.safe_load(MOCK_CONFIG_BLANK_REPOSITORIES)
-        action = CheckAutoDeployRepo(config)
+        action = self.get_action_instance(config=config)
 
         self.assertRaises(Exception, action.run,
                           branch="refs/heads/master", repo_name="st2contrib")
 
     def test_run_st2contrib_no_auto_deloy(self):
         config = yaml.safe_load(MOCK_CONFIG_FULL)
-        action = CheckAutoDeployRepo(config)
+        action = self.get_action_instance(config=config)
 
         self.assertRaises(Exception, action.run,
                           branch="refs/heads/dev", repo_name="st2contrib")
 
     def test_run_st2contrib_auto_deloy(self):
         config = yaml.safe_load(MOCK_CONFIG_FULL)
-        action = CheckAutoDeployRepo(config)
+        action = self.get_action_instance(config=config)
 
         expected = {'deployment_branch': 'master', 'notify_channel': 'community'}
 
@@ -72,7 +74,7 @@ def test_run_st2incubator_no_auto_deloy(self):
 
     def test_run_st2incubator_auto_deloy(self):
         config = yaml.safe_load(MOCK_CONFIG_FULL)
-        action = CheckAutoDeployRepo(config)
+        action = self.get_action_instance(config=config)
 
         expected = {'deployment_branch': 'master', 'notify_channel': 'community'}
 

contrib/packs/tests/test_action_expand_repo_name.py

@@ -30,23 +30,25 @@
 """
 
 class ExpandRepoNameTestCase(BaseActionTestCase):
+    action_cls = ExpandRepoName
+
     def test_run_config_blank(self):
         config = yaml.safe_load(MOCK_CONFIG_BLANK)
-        action = ExpandRepoName(config)
+        action = self.get_action_instance(config=config)
 
         self.assertRaises(Exception, action.run,
                           repo_name="st2contrib")
 
     def test_run_repositories_blank(self):
         config = yaml.safe_load(MOCK_CONFIG_BLANK_REPOSITORIES)
-        action = ExpandRepoName(config)
+        action = self.get_action_instance(config=config)
 
         self.assertRaises(Exception, action.run,
                           repo_name="st2contrib")
 
     def test_run_st2contrib_expands(self):
         config = yaml.safe_load(MOCK_CONFIG_FULL)
-        action = ExpandRepoName(config)
+        action = self.get_action_instance(config=config)
 
         expected = {'repo_url': 'https://github.com/StackStorm/st2contrib.git', 'subtree': True}
 
@@ -55,7 +57,7 @@ def test_run_st2contrib_expands(self):
 
     def test_run_st2incubator_expands(self):
         config = yaml.safe_load(MOCK_CONFIG_FULL)
-        action = ExpandRepoName(config)
+        action = self.get_action_instance(config=config)
 
         expected = {'repo_url': 'https://github.com/StackStorm/st2incubator.git', 'subtree': True}
 

st2actions/st2actions/runners/python_action_wrapper.py

@@ -16,24 +16,58 @@
 import sys
 import json
 import argparse
-import logging as stdlib_logging
 
 from st2common import log as logging
 from st2actions import config
 from st2actions.runners.pythonrunner import Action
+from st2actions.runners.utils import get_logger_for_python_runner_action
+from st2actions.runners.utils import get_action_class_instance
 from st2common.util import loader as action_loader
 from st2common.util.config_parser import ContentPackConfigParser
 from st2common.constants.action import ACTION_OUTPUT_RESULT_DELIMITER
 from st2common.service_setup import db_setup
 from st2common.services.datastore import DatastoreService
 
 __all__ = [
-    'PythonActionWrapper'
+    'PythonActionWrapper',
+    'ActionService'
 ]
 
 LOG = logging.getLogger(__name__)
 
 
+class ActionService(object):
+    """
+    Instance of this class is passed to the action instance and exposes "public"
+    methods which can be called by the action.
+    """
+
+    def __init__(self, action_wrapper):
+        logger = get_logger_for_python_runner_action(action_name=action_wrapper._class_name)
+
+        self._action_wrapper = action_wrapper
+        self._datastore_service = DatastoreService(logger=logger,
+                                                   pack_name=self._action_wrapper._pack,
+                                                   class_name=self._action_wrapper._class_name,
+                                                   api_username='action_service')
+
+    ##################################
+    # Methods for datastore management
+    ##################################
+
+    def list_values(self, local=True, prefix=None):
+        return self._datastore_service.list_values(local, prefix)
+
+    def get_value(self, name, local=True):
+        return self._datastore_service.get_value(name, local)
+
+    def set_value(self, name, value, ttl=None, local=True):
+        return self._datastore_service.set_value(name, value, ttl, local)
+
+    def delete_value(self, name, local=True):
+        return self._datastore_service.delete_value(name, local)
+
+
 class PythonActionWrapper(object):
     def __init__(self, pack, file_path, parameters=None, parent_args=None):
         """
@@ -92,39 +126,17 @@ def _get_action_instance(self):
         if config:
             LOG.info('Using config "%s" for action "%s"' % (config.file_path,
                                                             self._file_path))
-
-            action_instance = action_cls(config=config.config)
+            config = config.config
         else:
             LOG.info('No config found for action "%s"' % (self._file_path))
-            action_instance = action_cls(config={})
-
-        # Setup action_instance proeprties
-        action_instance.logger = self._set_up_logger(action_cls.__name__)
-        action_instance.datastore = DatastoreService(logger=action_instance.logger,
-                                                     pack_name=self._pack,
-                                                     class_name=action_cls.__name__,
-                                                     api_username="action_service")
+            config = None
 
+        action_service = ActionService(action_wrapper=self)
+        action_instance = get_action_class_instance(action_cls=action_cls,
+                                                    config=config,
+                                                    action_service=action_service)
         return action_instance
 
-    def _set_up_logger(self, action_name):
-        """
-        Set up a logger which logs all the messages with level DEBUG
-        and above to stderr.
-        """
-        logger_name = 'actions.python.%s' % (action_name)
-        logger = logging.getLogger(logger_name)
-
-        console = stdlib_logging.StreamHandler()
-        console.setLevel(stdlib_logging.DEBUG)
-
-        formatter = stdlib_logging.Formatter('%(name)-12s: %(levelname)-8s %(message)s')
-        console.setFormatter(formatter)
-        logger.addHandler(console)
-        logger.setLevel(stdlib_logging.DEBUG)
-
-        return logger
-
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(description='Python action runner process wrapper')