You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ariatosca.apache.org by mx...@apache.org on 2017/02/22 19:35:50 UTC

incubator-ariatosca git commit: review 2 [Forced Update!]

Repository: incubator-ariatosca
Updated Branches:
  refs/heads/ARIA-106-Create-sqla-logging-handler 73dab0013 -> 35a9c22a6 (forced update)


review 2


Project: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/commit/35a9c22a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/35a9c22a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/35a9c22a

Branch: refs/heads/ARIA-106-Create-sqla-logging-handler
Commit: 35a9c22a6dfc1219ca76247f32e3723e32aafaba
Parents: c37f075
Author: max-orlov <ma...@gigaspaces.com>
Authored: Wed Feb 22 19:20:45 2017 +0200
Committer: max-orlov <ma...@gigaspaces.com>
Committed: Wed Feb 22 21:35:35 2017 +0200

----------------------------------------------------------------------
 aria/__init__.py                             |  2 +
 aria/logger.py                               | 21 ++++---
 aria/orchestrator/context/common.py          |  7 ++-
 aria/orchestrator/decorators.py              |  4 +-
 aria/orchestrator/workflows/executor/base.py | 17 +++--
 aria/utils/imports.py                        | 35 +++++------
 tests/.pylintrc                              |  2 +-
 tests/orchestrator/context/test_operation.py | 76 +++++++++++++----------
 8 files changed, 86 insertions(+), 78 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/aria/__init__.py
----------------------------------------------------------------------
diff --git a/aria/__init__.py b/aria/__init__.py
index 7fd0db9..6b10501 100644
--- a/aria/__init__.py
+++ b/aria/__init__.py
@@ -32,6 +32,8 @@ from . import (
 )
 
 if sys.version_info < (2, 7):
+    # pkgutil in python2.6 has a bug where it fails to import from protected modules, which causes
+    # the entire process to fail. In order to overcome this issue we use our custom iter_modules
     from .utils.imports import iter_modules
 else:
     from pkgutil import iter_modules

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/aria/logger.py
----------------------------------------------------------------------
diff --git a/aria/logger.py b/aria/logger.py
index 1fcdf03..f7a23af 100644
--- a/aria/logger.py
+++ b/aria/logger.py
@@ -17,11 +17,12 @@
 Logging related mixins and functions
 """
 
-
 import logging
 from logging import handlers as logging_handlers
 from datetime import datetime
 
+import sqlalchemy
+
 _base_logger = logging.getLogger('aria')
 
 
@@ -99,8 +100,13 @@ def create_console_log_handler(level=logging.DEBUG, formatter=None):
     return console
 
 
-def create_sqla_log_handler(session, engine, level=logging.DEBUG):
-    return _SQLAlchemyHandler(session=session, engine=engine, level=level)
+def create_sqla_log_handler(session, engine, log_cls, level=logging.DEBUG):
+
+    # This is needed since the engine and session are entirely new we need to reflect the db
+    # schema of the logging model into the engine and session.
+    log_cls.__table__.create(bind=engine, checkfirst=True)
+
+    return _SQLAlchemyHandler(session=session, engine=engine, log_cls=log_cls, level=level)
 
 
 class _DefaultConsoleFormat(logging.Formatter):
@@ -144,16 +150,11 @@ def create_file_log_handler(
 
 class _SQLAlchemyHandler(logging.Handler):
 
-    def __init__(self, session, engine, **kwargs):
+    def __init__(self, session, engine, log_cls, **kwargs):
         logging.Handler.__init__(self, **kwargs)
         self._session = session
         self._engine = engine
-
-        # Cyclic dependency
-        from aria.storage.modeling.model import Log
-        self._cls = Log
-        # This is needed since the engine and session are entirely new
-        self._cls.__table__.create(bind=self._engine, checkfirst=True)
+        self._cls = log_cls
 
     def emit(self, record):
         created_at = datetime.strptime(logging.Formatter('%(asctime)s').formatTime(record),

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/aria/orchestrator/context/common.py
----------------------------------------------------------------------
diff --git a/aria/orchestrator/context/common.py b/aria/orchestrator/context/common.py
index 4b1730a..b34cd5d 100644
--- a/aria/orchestrator/context/common.py
+++ b/aria/orchestrator/context/common.py
@@ -23,7 +23,10 @@ from uuid import uuid4
 import jinja2
 
 from aria import logger as aria_logger
-from aria.storage import exceptions
+from aria.storage import (
+    exceptions,
+    modeling
+)
 
 
 class BaseContext(object):
@@ -71,7 +74,7 @@ class BaseContext(object):
         if self._model._initiator:
             api_kwargs.update(self._model._initiator(**self._model._initiator_kwargs))
         api_kwargs.update(**self._model._api_kwargs)
-        return aria_logger.create_sqla_log_handler(**api_kwargs)
+        return aria_logger.create_sqla_log_handler(log_cls=modeling.model.Log, **api_kwargs)
 
     def __repr__(self):
         return (

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/aria/orchestrator/decorators.py
----------------------------------------------------------------------
diff --git a/aria/orchestrator/decorators.py b/aria/orchestrator/decorators.py
index 1a8ead0..3ced61c 100644
--- a/aria/orchestrator/decorators.py
+++ b/aria/orchestrator/decorators.py
@@ -71,9 +71,7 @@ def operation(func=None, toolbelt=False, suffix_template='', logging_handlers=No
             operation_toolbelt = context.toolbelt(func_kwargs['ctx'])
             func_kwargs.setdefault('toolbelt', operation_toolbelt)
         validate_function_arguments(func, func_kwargs)
-
-        with func_kwargs['ctx'].logging_handlers(logging_handlers):
-            return func(**func_kwargs)
+        return func(**func_kwargs)
     return _wrapper
 
 

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/aria/orchestrator/workflows/executor/base.py
----------------------------------------------------------------------
diff --git a/aria/orchestrator/workflows/executor/base.py b/aria/orchestrator/workflows/executor/base.py
index cd8e7ee..4ae046d 100644
--- a/aria/orchestrator/workflows/executor/base.py
+++ b/aria/orchestrator/workflows/executor/base.py
@@ -39,15 +39,14 @@ class BaseExecutor(logger.LoggerMixin):
         """
         pass
 
-    def _task_started(self, task):
-        self._signal(events.start_task_signal, task)
-
-    def _task_failed(self, task, exception):
-        self._signal(events.on_failure_task_signal, task, exception=exception)
+    @staticmethod
+    def _task_started(task):
+        events.start_task_signal.send(task)
 
-    def _task_succeeded(self, task):
-        self._signal(events.on_success_task_signal, task)
+    @staticmethod
+    def _task_failed(task, exception):
+        events.on_failure_task_signal.send(task, exception=exception)
 
     @staticmethod
-    def _signal(signal, task, **kwargs):
-        signal.send(task, **kwargs)
+    def _task_succeeded(task):
+        events.on_success_task_signal.send(task)

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/aria/utils/imports.py
----------------------------------------------------------------------
diff --git a/aria/utils/imports.py b/aria/utils/imports.py
index 1782d16..64a48cf 100644
--- a/aria/utils/imports.py
+++ b/aria/utils/imports.py
@@ -79,23 +79,18 @@ def load_attribute(attribute_path):
         raise
 
 
-class _SafeModuleImporter(object):
-    def __init__(self):
-        self._yielded = {}
-
-    def iter_modules(self):
-        # apparently pkgutil had some issues in python 2.6. Accessing any root level directories
-        # failed. and it got the entire process of importing fail. Since we only need any
-        # aria_extension related loading, in the meantime we could try to import only those
-        # (and assume they are not located at the root level.
-        # [In python 2.7 it does actually ignore any OSError].
-        for importer in pkgutil.iter_importers():
-            try:
-                for module_name, ispkg in pkgutil.iter_importer_modules(importer):
-                    if module_name not in self._yielded:
-                        self._yielded[module_name] = True
-                        yield importer, module_name, ispkg
-            except OSError:
-                pass
-
-iter_modules = _SafeModuleImporter().iter_modules
+def iter_modules():
+    # apparently pkgutil had some issues in python 2.6. Accessing any root level directories
+    # failed. and it got the entire process of importing fail. Since we only need any
+    # aria_extension related loading, in the meantime we could try to import only those
+    # (and assume they are not located at the root level.
+    # [In python 2.7 it does actually ignore any OSError].
+    yielded = {}
+    for importer in pkgutil.iter_importers():
+        try:
+            for module_name, ispkg in pkgutil.iter_importer_modules(importer):
+                if module_name not in yielded:
+                    yielded[module_name] = True
+                    yield importer, module_name, ispkg
+        except OSError:
+            pass

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/tests/.pylintrc
----------------------------------------------------------------------
diff --git a/tests/.pylintrc b/tests/.pylintrc
index 5de0691..06409e9 100644
--- a/tests/.pylintrc
+++ b/tests/.pylintrc
@@ -77,7 +77,7 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=import-star-module-level,old-octal-literal,oct-method,print-statement,unpacking-in-except,parameter-unpacking,backtick,old-raise-syntax,old-ne-operator,long-suffix,dict-view-method,dict-iter-method,metaclass-assignment,next-method-called,raising-string,indexing-exception,raw_input-builtin,long-builtin,file-builtin,execfile-builtin,coerce-builtin,cmp-builtin,buffer-builtin,basestring-builtin,apply-builtin,filter-builtin-not-iterating,using-cmp-argument,useless-suppression,range-builtin-not-iterating,suppressed-message,no-absolute-import,old-division,cmp-method,reload-builtin,zip-builtin-not-iterating,intern-builtin,unichr-builtin,reduce-builtin,standarderror-builtin,unicode-builtin,xrange-builtin,coerce-method,delslice-method,getslice-method,setslice-method,input-builtin,round-builtin,hex-method,nonzero-method,map-builtin-not-iterating,redefined-builtin,no-self-use,missing-docstring,attribute-defined-outside-init,redefined-outer-name,import-error,redefined-variable-type,broad
 -except,protected-access,global-statement,too-many-locals,abstract-method
+disable=import-star-module-level,old-octal-literal,oct-method,print-statement,unpacking-in-except,parameter-unpacking,backtick,old-raise-syntax,old-ne-operator,long-suffix,dict-view-method,dict-iter-method,metaclass-assignment,next-method-called,raising-string,indexing-exception,raw_input-builtin,long-builtin,file-builtin,execfile-builtin,coerce-builtin,cmp-builtin,buffer-builtin,basestring-builtin,apply-builtin,filter-builtin-not-iterating,using-cmp-argument,useless-suppression,range-builtin-not-iterating,suppressed-message,no-absolute-import,old-division,cmp-method,reload-builtin,zip-builtin-not-iterating,intern-builtin,unichr-builtin,reduce-builtin,standarderror-builtin,unicode-builtin,xrange-builtin,coerce-method,delslice-method,getslice-method,setslice-method,input-builtin,round-builtin,hex-method,nonzero-method,map-builtin-not-iterating,redefined-builtin,no-self-use,missing-docstring,attribute-defined-outside-init,redefined-outer-name,import-error,redefined-variable-type,broad
 -except,protected-access,global-statement,too-many-locals,abstract-method,no-member
 
 [REPORTS]
 

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/35a9c22a/tests/orchestrator/context/test_operation.py
----------------------------------------------------------------------
diff --git a/tests/orchestrator/context/test_operation.py b/tests/orchestrator/context/test_operation.py
index 50e0af4..2b26bb1 100644
--- a/tests/orchestrator/context/test_operation.py
+++ b/tests/orchestrator/context/test_operation.py
@@ -15,8 +15,6 @@
 
 import os
 import time
-import logging
-import tempfile
 
 import pytest
 
@@ -50,18 +48,6 @@ def ctx(tmpdir):
     storage.release_sqlite_storage(context.model)
 
 
-tmp_file = os.path.join(tempfile.gettempdir(), 'op_testing_const_file_name')
-
-
-@pytest.fixture(autouse=True)
-def tmpfile_cleanup():
-    try:
-        yield
-    finally:
-        if os.path.isfile(tmp_file):
-            os.remove(tmp_file)
-
-
 @pytest.fixture
 def thread_executor():
     result = thread.ThreadExecutor()
@@ -227,19 +213,20 @@ def test_plugin_workdir(ctx, thread_executor, tmpdir):
 
 
 @pytest.fixture(params=[
-    (thread.ThreadExecutor()),
-    (process.ProcessExecutor(python_path=tests.ROOT_DIR))
+    (thread.ThreadExecutor, dict()),
+    (process.ProcessExecutor, dict(python_path=tests.ROOT_DIR))
 ])
 def executor(request):
-    ex = request.param
+    ex_cls, kwargs = request.param
+    ex = ex_cls(**kwargs)
     try:
         yield ex
     finally:
         ex.close()
 
 
-def test_operation_logging(ctx, executor):
-    operation_name = 'aria.interfaces.lifecycle.create'
+def test_node_operation_logging(ctx, executor):
+    operation_name = mock.operations.NODE_OPERATIONS_INSTALL[0]
 
     node = ctx.model.node.get_by_name(mock.models.DEPENDENCY_NODE_INSTANCE_NAME)
     interface = mock.models.get_interface(
@@ -265,9 +252,44 @@ def test_operation_logging(ctx, executor):
         )
 
     execute(workflow_func=basic_workflow, workflow_context=ctx, executor=executor)
+    _assert_loggins(ctx.model.log.list(), inputs)
+
+
+def test_relationship_operation_logging(ctx, executor):
+    operation_name = mock.operations.RELATIONSHIP_OPERATIONS_INSTALL[0].rsplit('_', 1)[0]
 
-    logs = ctx.model.log.list()
+    relationship = ctx.model.relationship.list()[0]
+    relationship.interfaces = [mock.models.get_interface(
+        operation_name,
+        operation_kwargs=dict(implementation=op_path(logged_operation, module_path=__name__)),
+        edge='source'
+    )]
+    ctx.model.relationship.update(relationship)
+
+    inputs = {
+        'op_start': 'op_start',
+        'op_end': 'op_end',
+    }
 
+    @workflow
+    def basic_workflow(graph, **_):
+        graph.add_tasks(
+            api.task.OperationTask.relationship(
+                name=operation_name,
+                instance=relationship,
+                inputs=inputs,
+                edge='source',
+            )
+        )
+
+    execute(workflow_func=basic_workflow, workflow_context=ctx, executor=executor)
+    _assert_loggins(ctx.model.log.list(), inputs)
+
+
+def _assert_loggins(logs, inputs):
+
+    # The logs should contain the following: Workflow Start, Operation Start, custom operation
+    # log string (op_start), custom operation log string (op_end), Operation End, Workflow End.
     assert len(logs) == 6
 
     op_start_log = [l for l in logs if inputs['op_start'] in l.msg and l.level.lower() == 'info']
@@ -280,20 +302,8 @@ def test_operation_logging(ctx, executor):
 
     assert op_start_log.created_at < op_end_log.created_at
 
-    with open(tmp_file, 'r') as f:
-        logs = [l.strip() for l in f.readlines()]
-
-    assert inputs['op_start'] in logs
-    assert inputs['op_end'] in logs
 
-
-class MockLogHandler(logging.Handler):
-    def emit(self, record):
-        with open(tmp_file, 'a+') as f:
-            f.write(record.msg + '\n')
-
-
-@operation(logging_handlers=[MockLogHandler()])
+@operation
 def logged_operation(ctx, **_):
     ctx.logger.info(ctx.task.inputs['op_start'])
     # enables to check the relation between the created_at field properly