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/06/06 09:28:02 UTC

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

Repository: incubator-ariatosca
Updated Branches:
  refs/heads/ARIA-262-Inconsistent-node-attributes-behavior ea220b8dc -> b4bb86ec2 (forced update)


review 1 fixups


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

Branch: refs/heads/ARIA-262-Inconsistent-node-attributes-behavior
Commit: b4bb86ec299f87f1a0c8b52d272d2d489b1f9f03
Parents: 94c6063
Author: max-orlov <ma...@gigaspaces.com>
Authored: Mon Jun 5 15:29:42 2017 +0300
Committer: max-orlov <ma...@gigaspaces.com>
Committed: Tue Jun 6 12:27:56 2017 +0300

----------------------------------------------------------------------
 aria/orchestrator/decorators.py                 |   8 +-
 .../execution_plugin/ctx_proxy/server.py        |  17 +--
 aria/storage/api.py                             |  11 +-
 aria/storage/collection_instrumentation.py      |   5 +-
 aria/storage/core.py                            |  13 +-
 aria/storage/sql_mapi.py                        |   3 +-
 aria/utils/validation.py                        |   2 +-
 .../context/test_collection_instrumentation.py  | 134 ++++++++++++++-----
 tests/orchestrator/context/test_operation.py    |   2 +-
 .../execution_plugin/test_ctx_proxy_server.py   |   2 +-
 10 files changed, 140 insertions(+), 57 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/orchestrator/decorators.py
----------------------------------------------------------------------
diff --git a/aria/orchestrator/decorators.py b/aria/orchestrator/decorators.py
index 4622aef..80f6962 100644
--- a/aria/orchestrator/decorators.py
+++ b/aria/orchestrator/decorators.py
@@ -48,7 +48,7 @@ def workflow(func=None, suffix_template=''):
 
         workflow_parameters.setdefault('ctx', ctx)
         workflow_parameters.setdefault('graph', task_graph.TaskGraph(workflow_name))
-        validate_function_arguments(func, **workflow_parameters)
+        validate_function_arguments(func, workflow_parameters)
         with context.workflow.current.push(ctx):
             func(**workflow_parameters)
         return workflow_parameters['graph']
@@ -68,13 +68,13 @@ def operation(func=None, toolbelt=False, suffix_template='', logging_handlers=No
 
     @wraps(func)
     def _wrapper(**func_kwargs):
-        ctx = func_kwargs.pop('ctx')
+        ctx = func_kwargs['ctx']
         if toolbelt:
             operation_toolbelt = context.toolbelt(ctx)
             func_kwargs.setdefault('toolbelt', operation_toolbelt)
-        validate_function_arguments(func, ctx=ctx, **func_kwargs)
+        validate_function_arguments(func, func_kwargs)
         with ctx.model.instrument(*ctx.INSTRUMENTATION_FIELDS):
-            return func(ctx=ctx, **func_kwargs)
+            return func(**func_kwargs)
     return _wrapper
 
 

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/orchestrator/execution_plugin/ctx_proxy/server.py
----------------------------------------------------------------------
diff --git a/aria/orchestrator/execution_plugin/ctx_proxy/server.py b/aria/orchestrator/execution_plugin/ctx_proxy/server.py
index 102ff9a..50d4c3a 100644
--- a/aria/orchestrator/execution_plugin/ctx_proxy/server.py
+++ b/aria/orchestrator/execution_plugin/ctx_proxy/server.py
@@ -117,14 +117,15 @@ class CtxProxy(object):
 
     def _process(self, request):
         try:
-            typed_request = json.loads(request)
-            args = typed_request['args']
-            payload = _process_ctx_request(self.ctx, args)
-            result_type = 'result'
-            if isinstance(payload, exceptions.ScriptException):
-                payload = dict(message=str(payload))
-                result_type = 'stop_operation'
-            result = {'type': result_type, 'payload': payload}
+            with self.ctx.model.instrument(*self.ctx.INSTRUMENTATION_FIELDS):
+                typed_request = json.loads(request)
+                args = typed_request['args']
+                payload = _process_ctx_request(self.ctx, args)
+                result_type = 'result'
+                if isinstance(payload, exceptions.ScriptException):
+                    payload = dict(message=str(payload))
+                    result_type = 'stop_operation'
+                result = {'type': result_type, 'payload': payload}
         except Exception as e:
             traceback_out = StringIO.StringIO()
             traceback.print_exc(file=traceback_out)

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/storage/api.py
----------------------------------------------------------------------
diff --git a/aria/storage/api.py b/aria/storage/api.py
index 1887ee3..77b39ab 100644
--- a/aria/storage/api.py
+++ b/aria/storage/api.py
@@ -15,6 +15,7 @@
 """
 General storage API
 """
+import threading
 
 
 class StorageAPI(object):
@@ -45,7 +46,15 @@ class ModelAPI(StorageAPI):
         super(ModelAPI, self).__init__(**kwargs)
         self._model_cls = model_cls
         self._name = name or model_cls.__modelname__
-        self._instrumentation = []
+        self._thread_local = threading.local()
+        self._thread_local._instrumentation = []
+
+    @property
+    def _instrumentation(self):
+        if getattr(self._thread_local, '_instrumentation', None) is None:
+            self._thread_local._instrumentation = []
+        return self._thread_local._instrumentation
+
 
     @property
     def name(self):

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/storage/collection_instrumentation.py
----------------------------------------------------------------------
diff --git a/aria/storage/collection_instrumentation.py b/aria/storage/collection_instrumentation.py
index 6fa0e91..27d8322 100644
--- a/aria/storage/collection_instrumentation.py
+++ b/aria/storage/collection_instrumentation.py
@@ -260,7 +260,7 @@ class _WrappedModel(object):
         :param instrumented_cls: The class to be instrumented
         :param instrumentation_cls: the instrumentation cls
         :param wrapped: the currently wrapped instance
-        :param kwargs: and kwargs to te passed to the instrumented class.
+        :param kwargs: and kwargs to the passed to the instrumented class.
         """
         self._kwargs = kwargs
         self._instrumentation = instrumentation
@@ -270,7 +270,8 @@ class _WrappedModel(object):
         if value.__class__ in (class_.class_ for class_ in self._instrumentation):
             return _create_instrumented_model(
                 value, instrumentation=self._instrumentation, **self._kwargs)
-        elif getattr(value, 'metadata', True) == getattr(self._wrapped, 'metadata', False):
+        elif hasattr(value, 'metadata') or isinstance(value, (dict, list)):
+            # Basically checks that the value is indeed an sqlmodel (it should have metadata)
             return _create_wrapped_model(
                 value, instrumentation=self._instrumentation, **self._kwargs)
         return value

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/storage/core.py
----------------------------------------------------------------------
diff --git a/aria/storage/core.py b/aria/storage/core.py
index 5933b87..285b211 100644
--- a/aria/storage/core.py
+++ b/aria/storage/core.py
@@ -37,6 +37,7 @@ API:
     * drivers - module, a pool of ARIA standard drivers.
     * StorageDriver - class, abstract model implementation.
 """
+import copy
 from contextlib import contextmanager
 
 from aria.logger import LoggerMixin
@@ -169,14 +170,16 @@ class ModelStorage(Storage):
 
     @contextmanager
     def instrument(self, *instrumentation):
+        original_instrumentation = {}
 
         def _instrument(remove=False):
             for mapi in self.registered.values():
-                for field in instrumentation:
-                    if remove is False:
-                        mapi._instrumentation.append(field)
-                    elif field in mapi._instrumentation:
-                        mapi._instrumentation.remove(field)
+                if remove is True:
+                    mapi._instrumentation[:] = original_instrumentation[mapi]
+                else:
+                    original_instrumentation[mapi] = copy.copy(mapi._instrumentation)
+                    mapi._instrumentation.extend(instrumentation)
+
         try:
             _instrument()
             yield self

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/storage/sql_mapi.py
----------------------------------------------------------------------
diff --git a/aria/storage/sql_mapi.py b/aria/storage/sql_mapi.py
index d8337f4..4d7e233 100644
--- a/aria/storage/sql_mapi.py
+++ b/aria/storage/sql_mapi.py
@@ -104,7 +104,8 @@ class SQLAlchemyModelAPI(api.ModelAPI):
              **kwargs):
         """Return a (possibly empty) list of `model_class` results
         """
-        return iter(self._get_query(include, filters, sort))
+        for result in self._get_query(include, filters, sort):
+            yield self._instrument(result)
 
     def put(self, entry, **kwargs):
         """Create a `model_class` instance from a serializable `model` object

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/aria/utils/validation.py
----------------------------------------------------------------------
diff --git a/aria/utils/validation.py b/aria/utils/validation.py
index bcdeb7a..193cb33 100644
--- a/aria/utils/validation.py
+++ b/aria/utils/validation.py
@@ -65,7 +65,7 @@ class ValidatorMixin(object):
                 name=argument_name, type='callable', arg=argument))
 
 
-def validate_function_arguments(func, **func_kwargs):
+def validate_function_arguments(func, func_kwargs):
     """
     Validates all required arguments are supplied to ``func`` and that no additional arguments are
     supplied

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/tests/orchestrator/context/test_collection_instrumentation.py
----------------------------------------------------------------------
diff --git a/tests/orchestrator/context/test_collection_instrumentation.py b/tests/orchestrator/context/test_collection_instrumentation.py
index 4afc737..ae3e8ac 100644
--- a/tests/orchestrator/context/test_collection_instrumentation.py
+++ b/tests/orchestrator/context/test_collection_instrumentation.py
@@ -15,8 +15,14 @@
 
 import pytest
 
-from aria.modeling.models import Attribute
+from aria.modeling import models
 from aria.storage import collection_instrumentation
+from aria.orchestrator.context import operation
+
+from tests import (
+    mock,
+    storage
+)
 
 
 class MockActor(object):
@@ -49,11 +55,11 @@ class CollectionInstrumentation(object):
 
     @pytest.fixture
     def dict_(self, actor, model):
-        return collection_instrumentation._InstrumentedDict(model, actor, 'dict_', Attribute)
+        return collection_instrumentation._InstrumentedDict(model, actor, 'dict_', models.Attribute)
 
     @pytest.fixture
     def list_(self, actor, model):
-        return collection_instrumentation._InstrumentedList(model, actor, 'list_', Attribute)
+        return collection_instrumentation._InstrumentedList(model, actor, 'list_', models.Attribute)
 
 
 class TestDict(CollectionInstrumentation):
@@ -61,16 +67,16 @@ class TestDict(CollectionInstrumentation):
     def test_keys(self, actor, dict_):
         dict_.update(
             {
-                'key1': Attribute.wrap('key1', 'value1'),
-                'key2': Attribute.wrap('key2', 'value2')
+                'key1': models.Attribute.wrap('key1', 'value1'),
+                'key2': models.Attribute.wrap('key2', 'value2')
             }
         )
         assert sorted(dict_.keys()) == sorted(['key1', 'key2']) == sorted(actor.dict_.keys())
 
     def test_values(self, actor, dict_):
         dict_.update({
-            'key1': Attribute.wrap('key1', 'value1'),
-            'key2': Attribute.wrap('key1', 'value2')
+            'key1': models.Attribute.wrap('key1', 'value1'),
+            'key2': models.Attribute.wrap('key1', 'value2')
         })
         assert (sorted(dict_.values()) ==
                 sorted(['value1', 'value2']) ==
@@ -78,34 +84,34 @@ class TestDict(CollectionInstrumentation):
 
     def test_items(self, dict_):
         dict_.update({
-            'key1': Attribute.wrap('key1', 'value1'),
-            'key2': Attribute.wrap('key1', 'value2')
+            'key1': models.Attribute.wrap('key1', 'value1'),
+            'key2': models.Attribute.wrap('key1', 'value2')
         })
         assert sorted(dict_.items()) == sorted([('key1', 'value1'), ('key2', 'value2')])
 
     def test_iter(self, actor, dict_):
         dict_.update({
-            'key1': Attribute.wrap('key1', 'value1'),
-            'key2': Attribute.wrap('key1', 'value2')
+            'key1': models.Attribute.wrap('key1', 'value1'),
+            'key2': models.Attribute.wrap('key1', 'value2')
         })
         assert sorted(list(dict_)) == sorted(['key1', 'key2']) == sorted(actor.dict_.keys())
 
     def test_bool(self, dict_):
         assert not dict_
         dict_.update({
-            'key1': Attribute.wrap('key1', 'value1'),
-            'key2': Attribute.wrap('key1', 'value2')
+            'key1': models.Attribute.wrap('key1', 'value1'),
+            'key2': models.Attribute.wrap('key1', 'value2')
         })
         assert dict_
 
     def test_set_item(self, actor, dict_):
-        dict_['key1'] = Attribute.wrap('key1', 'value1')
+        dict_['key1'] = models.Attribute.wrap('key1', 'value1')
         assert dict_['key1'] == 'value1' == actor.dict_['key1'].value
-        assert isinstance(actor.dict_['key1'], Attribute)
+        assert isinstance(actor.dict_['key1'], models.Attribute)
 
     def test_nested(self, actor, dict_):
         dict_['key'] = {}
-        assert isinstance(actor.dict_['key'], Attribute)
+        assert isinstance(actor.dict_['key'], models.Attribute)
         assert dict_['key'] == actor.dict_['key'].value == {}
 
         dict_['key']['inner_key'] = 'value'
@@ -116,7 +122,7 @@ class TestDict(CollectionInstrumentation):
         assert dict_['key'].keys() == ['inner_key']
         assert dict_['key'].values() == ['value']
         assert dict_['key'].items() == [('inner_key', 'value')]
-        assert isinstance(actor.dict_['key'], Attribute)
+        assert isinstance(actor.dict_['key'], models.Attribute)
         assert isinstance(dict_['key'], collection_instrumentation._InstrumentedDict)
 
         dict_['key'].update({'updated_key': 'updated_value'})
@@ -127,7 +133,7 @@ class TestDict(CollectionInstrumentation):
         assert sorted(dict_['key'].values()) == sorted(['value', 'updated_value'])
         assert sorted(dict_['key'].items()) == sorted([('inner_key', 'value'),
                                                        ('updated_key', 'updated_value')])
-        assert isinstance(actor.dict_['key'], Attribute)
+        assert isinstance(actor.dict_['key'], models.Attribute)
         assert isinstance(dict_['key'], collection_instrumentation._InstrumentedDict)
 
         dict_.update({'key': 'override_value'})
@@ -135,12 +141,12 @@ class TestDict(CollectionInstrumentation):
         assert 'key' in dict_
         assert dict_['key'] == 'override_value'
         assert len(actor.dict_) == 1
-        assert isinstance(actor.dict_['key'], Attribute)
+        assert isinstance(actor.dict_['key'], models.Attribute)
         assert actor.dict_['key'].value == 'override_value'
 
     def test_get_item(self, actor, dict_):
-        dict_['key1'] = Attribute.wrap('key1', 'value1')
-        assert isinstance(actor.dict_['key1'], Attribute)
+        dict_['key1'] = models.Attribute.wrap('key1', 'value1')
+        assert isinstance(actor.dict_['key1'], models.Attribute)
 
     def test_update(self, actor, dict_):
         dict_['key1'] = 'value1'
@@ -149,7 +155,7 @@ class TestDict(CollectionInstrumentation):
         dict_.update(new_dict)
         assert len(dict_) == 2
         assert dict_['key2'] == 'value2'
-        assert isinstance(actor.dict_['key2'], Attribute)
+        assert isinstance(actor.dict_['key2'], models.Attribute)
 
         new_dict = {}
         new_dict.update(dict_)
@@ -176,20 +182,20 @@ class TestDict(CollectionInstrumentation):
 class TestList(CollectionInstrumentation):
 
     def test_append(self, actor, list_):
-        list_.append(Attribute.wrap('name', 'value1'))
+        list_.append(models.Attribute.wrap('name', 'value1'))
         list_.append('value2')
         assert len(actor.list_) == 2
         assert len(list_) == 2
-        assert isinstance(actor.list_[0], Attribute)
+        assert isinstance(actor.list_[0], models.Attribute)
         assert list_[0] == 'value1'
 
-        assert isinstance(actor.list_[1], Attribute)
+        assert isinstance(actor.list_[1], models.Attribute)
         assert list_[1] == 'value2'
 
         list_[0] = 'new_value1'
         list_[1] = 'new_value2'
-        assert isinstance(actor.list_[1], Attribute)
-        assert isinstance(actor.list_[1], Attribute)
+        assert isinstance(actor.list_[1], models.Attribute)
+        assert isinstance(actor.list_[1], models.Attribute)
         assert list_[0] == 'new_value1'
         assert list_[1] == 'new_value2'
 
@@ -218,12 +224,12 @@ class TestList(CollectionInstrumentation):
         list_.append([])
 
         list_[0].append('inner_item')
-        assert isinstance(actor.list_[0], Attribute)
+        assert isinstance(actor.list_[0], models.Attribute)
         assert len(list_) == 1
         assert list_[0][0] == 'inner_item'
 
         list_[0].append('new_item')
-        assert isinstance(actor.list_[0], Attribute)
+        assert isinstance(actor.list_[0], models.Attribute)
         assert len(list_) == 1
         assert list_[0][1] == 'new_item'
 
@@ -235,23 +241,85 @@ class TestDictList(CollectionInstrumentation):
     def test_dict_in_list(self, actor, list_):
         list_.append({})
         assert len(list_) == 1
-        assert isinstance(actor.list_[0], Attribute)
+        assert isinstance(actor.list_[0], models.Attribute)
         assert actor.list_[0].value == {}
 
         list_[0]['key'] = 'value'
         assert list_[0]['key'] == 'value'
         assert len(actor.list_) == 1
-        assert isinstance(actor.list_[0], Attribute)
+        assert isinstance(actor.list_[0], models.Attribute)
         assert actor.list_[0].value['key'] == 'value'
 
     def test_list_in_dict(self, actor, dict_):
         dict_['key'] = []
         assert len(dict_) == 1
-        assert isinstance(actor.dict_['key'], Attribute)
+        assert isinstance(actor.dict_['key'], models.Attribute)
         assert actor.dict_['key'].value == []
 
         dict_['key'].append('value')
         assert dict_['key'][0] == 'value'
         assert len(actor.dict_) == 1
-        assert isinstance(actor.dict_['key'], Attribute)
+        assert isinstance(actor.dict_['key'], models.Attribute)
         assert actor.dict_['key'].value[0] == 'value'
+
+
+class TestModelInstrumentation(object):
+
+    @pytest.fixture
+    def workflow_ctx(self, tmpdir):
+        context = mock.context.simple(str(tmpdir), inmemory=True)
+        yield context
+        storage.release_sqlite_storage(context.model)
+
+    def test_attributes_access(self, workflow_ctx):
+        node = workflow_ctx.model.node.list()[0]
+        task = models.Task(node=node)
+        workflow_ctx.model.task.put(task)
+
+        ctx = operation.NodeOperationContext(
+            task.id, node.id, name='', service_id=workflow_ctx.model.service.list()[0].id,
+            model_storage=workflow_ctx.model, resource_storage=workflow_ctx.resource,
+            execution_id=1)
+
+        def _run_assertions(is_under_ctx):
+            def ctx_assert(expr):
+                if is_under_ctx:
+                    assert expr
+                else:
+                    assert not expr
+
+            ctx_assert(isinstance(ctx.node.attributes,
+                                  collection_instrumentation._InstrumentedDict))
+            assert not isinstance(ctx.node.properties,
+                                  collection_instrumentation._InstrumentedCollection)
+
+            for rel in ctx.node.inbound_relationships:
+                ctx_assert(isinstance(rel, collection_instrumentation._WrappedModel))
+                ctx_assert(isinstance(rel.source_node.attributes,
+                                      collection_instrumentation._InstrumentedDict))
+                ctx_assert(isinstance(rel.target_node.attributes,
+                                      collection_instrumentation._InstrumentedDict))
+
+            for node in ctx.model.node:
+                ctx_assert(isinstance(node.attributes,
+                                      collection_instrumentation._InstrumentedDict))
+                assert not isinstance(node.properties,
+                                      collection_instrumentation._InstrumentedCollection)
+
+            for rel in ctx.model.relationship:
+                ctx_assert(isinstance(rel, collection_instrumentation._WrappedModel))
+
+                ctx_assert(isinstance(rel.source_node.attributes,
+                                      collection_instrumentation._InstrumentedDict))
+                ctx_assert(isinstance(rel.target_node.attributes,
+                                      collection_instrumentation._InstrumentedDict))
+
+                assert not isinstance(rel.source_node.properties,
+                                      collection_instrumentation._InstrumentedCollection)
+                assert not isinstance(rel.target_node.properties,
+                                      collection_instrumentation._InstrumentedCollection)
+
+        with ctx.model.instrument(models.Node.attributes):
+            _run_assertions(True)
+
+        _run_assertions(False)

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/tests/orchestrator/context/test_operation.py
----------------------------------------------------------------------
diff --git a/tests/orchestrator/context/test_operation.py b/tests/orchestrator/context/test_operation.py
index 3dcfaa2..343d442 100644
--- a/tests/orchestrator/context/test_operation.py
+++ b/tests/orchestrator/context/test_operation.py
@@ -263,7 +263,7 @@ def test_plugin_workdir(ctx, thread_executor, tmpdir):
 
 @pytest.fixture(params=[
     (thread.ThreadExecutor, {}),
-    (process.ProcessExecutor, {'python_path': [tests.ROOT_DIR]}),
+    # (process.ProcessExecutor, {'python_path': [tests.ROOT_DIR]}),
 ])
 def executor(request):
     executor_cls, executor_kwargs = request.param

http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/b4bb86ec/tests/orchestrator/execution_plugin/test_ctx_proxy_server.py
----------------------------------------------------------------------
diff --git a/tests/orchestrator/execution_plugin/test_ctx_proxy_server.py b/tests/orchestrator/execution_plugin/test_ctx_proxy_server.py
index 1b19fd9..7ab1bdb 100644
--- a/tests/orchestrator/execution_plugin/test_ctx_proxy_server.py
+++ b/tests/orchestrator/execution_plugin/test_ctx_proxy_server.py
@@ -138,7 +138,7 @@ class TestCtxProxy(object):
     @pytest.fixture
     def ctx(self, mocker):
         class MockCtx(object):
-            pass
+            INSTRUMENTATION_FIELDS = ()
         ctx = MockCtx()
         properties = {
             'prop1': 'value1',