Repository: incubator-ariatosca Updated Branches: refs/heads/ARIA-262-Inconsistent-node-attributes-behavior bc4cf7f2a -> 4c0306bba (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/4c0306bb Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/4c0306bb Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/4c0306bb Branch: refs/heads/ARIA-262-Inconsistent-node-attributes-behavior Commit: 4c0306bba31359b87087a09482c055a352321f69 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 14:24:22 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 | 17 ++- aria/storage/sql_mapi.py | 3 +- aria/utils/validation.py | 2 +- .../context/test_collection_instrumentation.py | 134 ++++++++++++++----- .../execution_plugin/test_ctx_proxy_server.py | 2 +- 9 files changed, 139 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/4c0306bb/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/4c0306bb/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/4c0306bb/aria/storage/api.py ---------------------------------------------------------------------- diff --git a/aria/storage/api.py b/aria/storage/api.py index 1887ee3..f8bb523 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 hasattr(self._thread_local, '_instrumentation'): + self._thread_local._instrumentation = [] + return self._thread_local._instrumentation + @property def name(self): http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/4c0306bb/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/4c0306bb/aria/storage/core.py ---------------------------------------------------------------------- diff --git a/aria/storage/core.py b/aria/storage/core.py index 5933b87..f8bac51 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,16 +170,14 @@ 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) try: - _instrument() + for mapi in self.registered.values(): + original_instrumentation[mapi] = copy.copy(mapi._instrumentation) + mapi._instrumentation.extend(instrumentation) yield self finally: - _instrument(True) + for mapi in self.registered.values(): + mapi._instrumentation[:] = original_instrumentation[mapi] + http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/4c0306bb/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/4c0306bb/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/4c0306bb/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/4c0306bb/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',