[GitHub] incubator-ariatosca issue #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on the issue: https://github.com/apache/incubator-ariatosca/pull/188 Hey guys, I suggest going over the complex use cases in the [Clearwater example](https://github.com/apache/incubator-ariatosca/blob/ARIA-321-clearwater/examples/clearwater/scripts/host/configure.sh). Basically all the changes in the PR were inspired by those real-world needs of `ctx`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247622 --- Diff: tests/orchestrator/execution_plugin/test_ctx_proxy_server.py --- @@ -39,26 +39,26 @@ def test_dict_prop_access_get_key(self, server): assert response == 'value1' def test_dict_prop_access_get_key_nested(self, server): -response = self.request(server, 'node', 'properties', 'prop2.nested_prop1') +response = self.request(server, 'node', 'properties', 'prop2', 'nested_prop1') assert response == 'nested_value1' def test_dict_prop_access_get_with_list_index(self, server): -response = self.request(server, 'node', 'properties', 'prop3[2].value') +response = self.request(server, 'node', 'properties', 'prop3', 2, 'value') assert response == 'value_2' --- End diff -- +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247620
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
+argspec = inspect.getargspec(current.func)
+# Don't count initial args fulfilled by the partial
+spec_args = argspec.args[len(current.args):]
+# Don't count keyword args fulfilled by the partial
+spec_args = tuple(a for a in spec_args if a not in
current.keywords)
+else:
+argspec = inspect.getargspec(current)
+spec_args = argspec.args
+
+callable_kwargs = {}
+if isinstance(arg, dict):
+# If the first arg is a dict, treat it as our kwargs
+# TODO: what if the first argument really needs to be a
dict?
+callable_kwargs = arg
+index += 1
+
+if argspec.varargs is not None:
+# Gobble the rest of the args
+callable_args = args[index:]
+else:
+# Take only what we need
+spec_args = tuple(a for a in spec_args if a not in
callable_kwargs)
+spec_args_count = len(spec_args)
+if inspect.ismethod(current):
+# Don't count "self" argument
+spec_args_count -= 1
+callable_args = args[index:index + spec_args_count]
+# Note: we might actually have fewer args than the
args_count, but that could be OK
+# if the user expects subsequent args to have default
values
+
+args_count = len(callable_args)
+if args_count > 1:
+index += args_count - 1
+
+current = current(*callable_args, **callable_kwargs)
+
else:
-raise RuntimeError('{0} cannot be processed in
{1}'.format(arg, args))
+raise RuntimeError('`{0}` cannot be processed in
{1
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247609
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
+argspec = inspect.getargspec(current.func)
+# Don't count initial args fulfilled by the partial
+spec_args = argspec.args[len(current.args):]
+# Don't count keyword args fulfilled by the partial
+spec_args = tuple(a for a in spec_args if a not in
current.keywords)
+else:
+argspec = inspect.getargspec(current)
+spec_args = argspec.args
+
+callable_kwargs = {}
+if isinstance(arg, dict):
+# If the first arg is a dict, treat it as our kwargs
+# TODO: what if the first argument really needs to be a
dict?
+callable_kwargs = arg
+index += 1
+
+if argspec.varargs is not None:
+# Gobble the rest of the args
+callable_args = args[index:]
+else:
+# Take only what we need
+spec_args = tuple(a for a in spec_args if a not in
callable_kwargs)
+spec_args_count = len(spec_args)
+if inspect.ismethod(current):
+# Don't count "self" argument
+spec_args_count -= 1
+callable_args = args[index:index + spec_args_count]
--- End diff --
See comment above: we can't just grab all remaining args, because we might
need these args for further delving into the returned value of the callable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247616
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247602
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
--- End diff --
+1
The main reason for all of this is that we sometimes need to continue
access properties on returned values on callables. The previous implementation
simpled "gobbled" all remaining args, but we might need those args. Example
from Clearwater:
`ctx service get_policy_by_type clearwater.Configuration properties zone`
But generally I suggest you take a look at the various uses of `ctx` in the
[Clearwater
example](https://github.com/apache/incubator-ariatosca/blob/ARIA-321-clearwater/examples/clearwater/scripts/host/configure.sh).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247513
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
--- End diff --
This happens a lot, actually. I forget which of our tests would showcase
this exactly, but I think even `ctx logger` is based on partials.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247497
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
--- End diff --
I don't understand this comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247490
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247485
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
--- End diff --
Because that is broken on wrapped instrumented models...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247470 --- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py --- @@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs): self.close() -def _process_ctx_request(ctx, args): +def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements current = ctx -num_args = len(args) index = 0 + --- End diff -- This was the whole point of this PR. The previous implementation assumed either one arg or two: one arg meant "read" and two args meant "write". But this required us to support dot notation so that the one arg could refer to a deep nested attribute. Removing dot notation requires a different to support "write". This PR also supports working on the return value of function calls. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247255 --- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py --- @@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs): self.close() -def _process_ctx_request(ctx, args): +def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements --- End diff -- Of course we can, but it doesn't seem a very long function at all. This is one of those cases where PyLint is not being helpful. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247244 --- Diff: aria/storage/collection_instrumentation.py --- @@ -213,32 +235,32 @@ def __init__(self, mapi, *args, **kwargs): """ The original model. -:param wrapped: model to be instrumented :param mapi: MAPI for the wrapped model +:param wrapped: model to be instrumented +:param instrumentation: instrumentation dict +:param instrumentation_kwargs: arguments for instrumentation class """ -super(_InstrumentedModel, self).__init__(*args, **kwargs) +super(_InstrumentedModel, self).__init__(instrumentation_kwargs=dict(mapi=mapi), + *args, **kwargs) self._mapi = mapi self._apply_instrumentation() -def __getattr__(self, item): -return_value = getattr(self._wrapped, item) -if isinstance(return_value, self._wrapped.__class__): -return _create_instrumented_model(return_value, self._mapi, self._instrumentation) -if isinstance(return_value, (list, dict)): -return _create_wrapped_model(return_value, self._mapi, self._instrumentation) -return return_value - def _apply_instrumentation(self): for field in self._instrumentation: +if field.parent.class_ != type(self._wrapped): --- End diff -- It doesn't seem to break this in tests. Without this check, the only check is by field name, which caused a lot of breakage. You would node properties being instrumented for capability properties. Again, consider the use case of `ctx node capabilities host properties username`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247176 --- Diff: aria/orchestrator/workflows/api/task.py --- @@ -140,13 +140,18 @@ def __init__(self, self.arguments = modeling_utils.merge_parameter_values(arguments, operation.arguments, model_cls=models.Argument) -if getattr(self.actor, 'outbound_relationships', None) is not None: + +actor = self.actor +if hasattr(actor, '_wrapped'): --- End diff -- Because you often get instrumented/wrapped models here, and they would fail the `isinstance` check. (With duck typing check they succeed, but I thought duck typing was bad.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247158 --- Diff: aria/orchestrator/workflows/api/task.py --- @@ -140,13 +140,18 @@ def __init__(self, self.arguments = modeling_utils.merge_parameter_values(arguments, operation.arguments, model_cls=models.Argument) -if getattr(self.actor, 'outbound_relationships', None) is not None: --- End diff -- Why not import models? This should have been explained in comments. Duck typing seems very inaccurate and prone to errors. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130247107
--- Diff: aria/orchestrator/context/common.py ---
@@ -38,10 +38,27 @@ class BaseContext(object):
"""
INSTRUMENTATION_FIELDS = (
+modeling.models.Service.inputs,
--- End diff --
Because you should be able to do things like: {{ctx node capabilities host
properties username}}
The ctx proxy should let you access all models.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[2/3] incubator-ariatosca git commit: wip
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/90107846/aria/orchestrator/execution_plugin/instantiation.py
--
diff --git a/aria/orchestrator/execution_plugin/instantiation.py
b/aria/orchestrator/execution_plugin/instantiation.py
index f55aa50..49605b9 100644
--- a/aria/orchestrator/execution_plugin/instantiation.py
+++ b/aria/orchestrator/execution_plugin/instantiation.py
@@ -19,11 +19,7 @@ Instantiation of :class:`~aria.modeling.models.Operation`
models.
# TODO: this module will eventually be moved to a new "aria.instantiation"
package
-from ...utils.type import full_type_name
-from ...utils.formatting import safe_repr
from ...utils.collections import OrderedDict
-from ...parser import validation
-from ...parser.consumption import ConsumptionContext
from ...modeling.functions import Function
@@ -110,20 +106,24 @@ def _configure_remote(operation):
# Make sure we have a user
if fabric_env.get('user') is None:
-context = ConsumptionContext.get_thread_local()
-context.validation.report('must configure "ssh.user" for "{0}"'
- .format(operation.implementation),
- level=validation.Issue.BETWEEN_TYPES)
+# TODO: fix
+pass
+# context = ConsumptionContext.get_thread_local()
+# context.validation.report('must configure "ssh.user" for "{0}"'
+# .format(operation.implementation),
+# level=validation.Issue.BETWEEN_TYPES)
# Make sure we have an authentication value
if (fabric_env.get('password') is None) and \
(fabric_env.get('key') is None) and \
(fabric_env.get('key_filename') is None):
-context = ConsumptionContext.get_thread_local()
-context.validation.report('must configure "ssh.password", "ssh.key",
or "ssh.key_filename" '
- 'for "{0}"'
- .format(operation.implementation),
- level=validation.Issue.BETWEEN_TYPES)
+# TODO: fix
+pass
+# context = ConsumptionContext.get_thread_local()
+# context.validation.report(
+# 'must configure "ssh.password", "ssh.key", or "ssh.key_filename" for
"{0}"'.format(
+# operation.implementation),
+# level=validation.Issue.BETWEEN_TYPES)
operation.arguments['fabric_env'] = Argument.wrap('fabric_env', fabric_env,
'Fabric configuration.')
@@ -152,10 +152,12 @@ def _get_process(operation):
elif k == 'env':
_validate_type(v, dict, 'process.env')
else:
-context = ConsumptionContext.get_thread_local()
-context.validation.report('unsupported configuration parameter:
"process.{0}"'
- .format(k),
- level=validation.Issue.BETWEEN_TYPES)
+# TODO: fix
+pass
+# context = ConsumptionContext.get_thread_local()
+# context.validation.report('unsupported configuration parameter:
"process.{0}"'
+# .format(k),
+# level=validation.Issue.BETWEEN_TYPES)
return value
@@ -185,9 +187,11 @@ def _get_ssh(operation):
elif k == 'address':
_validate_type(v, basestring, 'ssh.address')
else:
-context = ConsumptionContext.get_thread_local()
-context.validation.report('unsupported configuration parameter:
"ssh.{0}"'.format(k),
- level=validation.Issue.BETWEEN_TYPES)
+# TODO: fix
+pass
+# context = ConsumptionContext.get_thread_local()
+# context.validation.report('unsupported configuration parameter:
"ssh.{0}"'.format(k),
+# level=validation.Issue.BETWEEN_TYPES)
return value
@@ -195,10 +199,12 @@ def _validate_type(value, the_type, name):
if isinstance(value, Function):
return
if not isinstance(value, the_type):
-context = ConsumptionContext.get_thread_local()
-context.validation.report('"{0}" configuration is not a {1}: {2}'
- .format(name, full_type_name(the_type),
safe_repr(value)),
- level=validation.Issue.BETWEEN_TYPES)
+# TODO: fix
+pass
+# context = ConsumptionContext.get_thread_local()
+# context.validation.report('"{0}" configuration is not a {1}: {2}'
+# .format(name, full_type_name(the_type),
safe_repr(value)),
+# level=validation.Issue.BETWEEN_TYPES)
def _coerce_bool(value, name):
@@ -212,10 +218,12 @@ def _co
[GitHub] incubator-ariatosca pull request #189: ARIA-174 Refactor instantiation phase
GitHub user mxmrlv opened a pull request: https://github.com/apache/incubator-ariatosca/pull/189 ARIA-174 Refactor instantiation phase You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/incubator-ariatosca ARIA-174-Refactor-instantiation-phase Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-ariatosca/pull/189.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #189 commit 9010784679c291738bba06c70f446b12893cd1df Author: max-orlov Date: 2017-07-13T13:49:15Z wip --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca issue #189: ARIA-174 Refactor instantiation phase
Github user asfgit commented on the issue: https://github.com/apache/incubator-ariatosca/pull/189 Can one of the admins verify this patch? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[3/3] incubator-ariatosca git commit: wip
wip
Project: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/repo
Commit:
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/commit/90107846
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/90107846
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/90107846
Branch: refs/heads/ARIA-174-Refactor-instantiation-phase
Commit: 9010784679c291738bba06c70f446b12893cd1df
Parents: b841c01
Author: max-orlov
Authored: Thu Jul 13 16:49:15 2017 +0300
Committer: max-orlov
Committed: Sun Jul 30 19:13:06 2017 +0300
--
aria/cli/commands/executions.py | 2 +-
aria/cli/commands/service_templates.py | 7 +-
aria/cli/commands/services.py | 6 +-
aria/core.py| 36 +-
aria/modeling/functions.py | 9 +-
aria/modeling/mixins.py | 34 +-
aria/modeling/service_common.py | 27 -
aria/modeling/service_instance.py | 535 +-
aria/modeling/service_template.py | 701 +--
aria/modeling/utils.py | 83 +--
.../execution_plugin/instantiation.py | 64 +-
aria/orchestrator/topology/__init__.py | 16 +
aria/orchestrator/topology/common.py| 52 ++
aria/orchestrator/topology/instance_handler.py | 644 +
aria/orchestrator/topology/template_handler.py | 591
aria/orchestrator/topology/topology.py | 266 +++
aria/orchestrator/topology/utils.py | 67 ++
aria/parser/consumption/__init__.py | 3 -
aria/parser/consumption/consumer.py | 6 +
aria/parser/consumption/context.py | 2 -
aria/parser/consumption/modeling.py | 49 +-
aria/parser/consumption/style.py| 50 --
aria/parser/modeling/context.py | 6 +-
aria/parser/reading/__init__.py | 6 +-
aria/parser/reading/locator.py | 35 -
aria/parser/validation/context.py | 59 +-
aria/parser/validation/issue.py | 72 +-
.../simple_v1_0/modeling/__init__.py| 4 +-
tests/instantiation/test_configuration.py | 13 +-
tests/parser/service_templates.py | 16 +-
tests/parser/test_reqs_caps.py | 29 +
.../tosca-simple-1.0/reqs_caps/reqs_caps1.yaml | 40 ++
tests/storage/__init__.py | 2 -
33 files changed, 1928 insertions(+), 1604 deletions(-)
--
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/90107846/aria/cli/commands/executions.py
--
diff --git a/aria/cli/commands/executions.py b/aria/cli/commands/executions.py
index 4783442..f130d95 100644
--- a/aria/cli/commands/executions.py
+++ b/aria/cli/commands/executions.py
@@ -181,7 +181,7 @@ def resume(execution_id,
executor = DryExecutor() if dry else None # use WorkflowRunner's default
executor
execution = model_storage.execution.get(execution_id)
-if execution.status != execution.status.CANCELLED:
+if execution.status != execution.CANCELLED:
logger.info("Can't resume execution {execution.id} - "
"execution is in status {execution.status}. "
"Can only resume executions in status {valid_status}"
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/90107846/aria/cli/commands/service_templates.py
--
diff --git a/aria/cli/commands/service_templates.py
b/aria/cli/commands/service_templates.py
index f567aa8..608d178 100644
--- a/aria/cli/commands/service_templates.py
+++ b/aria/cli/commands/service_templates.py
@@ -28,7 +28,7 @@ from ...core import Core
from ...storage import exceptions as storage_exceptions
from ...parser import consumption
from ...utils import (formatting, collections, console)
-
+from ... orchestrator.topology import Topology
DESCRIPTION_FIELD_LENGTH_LIMIT = 20
SERVICE_TEMPLATE_COLUMNS = \
@@ -73,10 +73,9 @@ def show(service_template_name, model_storage, mode_full,
mode_types, format_jso
elif format_yaml:
console.puts(formatting.yaml_dumps(collections.prune(service_template.as_raw)))
else:
-service_template.dump()
+console.puts(Topology().dump(service_template))
elif mode_types:
-consumption.ConsumptionContext()
-service_template.dump_types()
+console.puts(Topology().dump_types(service_template=service_template))
else:
logger.info('Showing service template
{0}...'.format(service_template_name))
service_template_dict = service_template.to_dict()
http:
[1/3] incubator-ariatosca git commit: wip [Forced Update!]
Repository: incubator-ariatosca
Updated Branches:
refs/heads/ARIA-174-Refactor-instantiation-phase 4b6e51b0a -> 901078467
(forced update)
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/90107846/aria/parser/consumption/modeling.py
--
diff --git a/aria/parser/consumption/modeling.py
b/aria/parser/consumption/modeling.py
index 44027b9..0afd555 100644
--- a/aria/parser/consumption/modeling.py
+++ b/aria/parser/consumption/modeling.py
@@ -13,8 +13,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from ...utils.formatting import json_dumps, yaml_dumps
from .consumer import Consumer, ConsumerChain
+from ...utils.formatting import json_dumps, yaml_dumps
+from ... import exceptions
class DeriveServiceTemplate(Consumer):
@@ -42,7 +43,7 @@ class CoerceServiceTemplateValues(Consumer):
"""
def consume(self):
-self.context.modeling.template.coerce_values(True)
+self.topology.coerce(self.context.modeling.template,
report_issues=True)
class ValidateServiceTemplate(Consumer):
@@ -51,7 +52,7 @@ class ValidateServiceTemplate(Consumer):
"""
def consume(self):
-self.context.modeling.template.validate()
+self.topology.validate(self.context.modeling.template)
class ServiceTemplate(ConsumerChain):
@@ -74,7 +75,7 @@ class ServiceTemplate(ConsumerChain):
raw = self.context.modeling.template_as_raw
self.context.write(json_dumps(raw, indent=indent))
else:
-self.context.modeling.template.dump()
+self.topology.dump(self.context.modeling.template)
class Types(Consumer):
@@ -92,7 +93,7 @@ class Types(Consumer):
raw = self.context.modeling.types_as_raw
self.context.write(json_dumps(raw, indent=indent))
else:
-self.context.modeling.template.dump_types()
+self.topology.dump_types(self.context,
self.context.modeling.template)
class InstantiateServiceInstance(Consumer):
@@ -105,9 +106,26 @@ class InstantiateServiceInstance(Consumer):
self.context.validation.report('InstantiateServiceInstance
consumer: missing service '
'template')
return
-
-self.context.modeling.template.instantiate(None, None,
-
inputs=dict(self.context.modeling.inputs))
+self.context.modeling.instance = self.topology.instantiate(
+self.context.modeling.template,
+inputs=dict(self.context.modeling.inputs)
+)
+ConsumerChain(
+self.context,
+(
+CoerceServiceInstanceValues,
+ValidateServiceInstance,
+SatisfyRequirements,
+CoerceServiceInstanceValues,
+ValidateCapabilities,
+FindHosts,
+ConfigureOperations,
+CoerceServiceInstanceValues
+)).consume()
+
+if self.context.validation.dump_issues():
+raise exceptions.InstantiationError('Failed to instantiate service
template `{0}`'
+
.format(self.context.modeling.template.name))
class CoerceServiceInstanceValues(Consumer):
@@ -116,7 +134,7 @@ class CoerceServiceInstanceValues(Consumer):
"""
def consume(self):
-self.context.modeling.instance.coerce_values(True)
+self.topology.coerce(self.context.modeling.instance,
report_issues=True)
class ValidateServiceInstance(Consumer):
@@ -125,7 +143,7 @@ class ValidateServiceInstance(Consumer):
"""
def consume(self):
-self.context.modeling.instance.validate()
+self.topology.validate(self.context.modeling.instance)
class SatisfyRequirements(Consumer):
@@ -134,7 +152,7 @@ class SatisfyRequirements(Consumer):
"""
def consume(self):
-self.context.modeling.instance.satisfy_requirements()
+self.topology.satisfy_requirements(self.context.modeling.instance)
class ValidateCapabilities(Consumer):
@@ -143,7 +161,7 @@ class ValidateCapabilities(Consumer):
"""
def consume(self):
-self.context.modeling.instance.validate_capabilities()
+self.topology.validate_capabilities(self.context.modeling.instance)
class FindHosts(Consumer):
@@ -152,7 +170,7 @@ class FindHosts(Consumer):
"""
def consume(self):
-self.context.modeling.instance.find_hosts()
+self.topology.find_hosts(self.context.modeling.instance)
class ConfigureOperations(Consumer):
@@ -161,7 +179,7 @@ class ConfigureOperations(Consumer):
"""
def consume(self):
-self.context.modeling.instance.configure_operations()
+self.topology.configure_operations(self.context.modeling.instance)
class ServiceInstance(Co
incubator-ariatosca git commit: fixed issue with reqs and caps issue where target capability types are not properly satisfied
Repository: incubator-ariatosca
Updated Branches:
refs/heads/ARIA-174-Refactor-instantiation-phase d6b268958 -> 4b6e51b0a
fixed issue with reqs and caps issue where target capability types are not
properly satisfied
Project: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/repo
Commit:
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/commit/4b6e51b0
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/4b6e51b0
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/4b6e51b0
Branch: refs/heads/ARIA-174-Refactor-instantiation-phase
Commit: 4b6e51b0a589de8978ee6e90dd84a05f38387f50
Parents: d6b2689
Author: max-orlov
Authored: Sun Jul 30 18:17:07 2017 +0300
Committer: max-orlov
Committed: Sun Jul 30 18:17:07 2017 +0300
--
aria/orchestrator/topology/instance_handler.py | 8
tests/parser/service_templates.py | 18 +++--
tests/parser/test_reqs_caps.py | 29 ++
.../tosca-simple-1.0/reqs_caps/reqs_caps1.yaml | 40
4 files changed, 91 insertions(+), 4 deletions(-)
--
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/4b6e51b0/aria/orchestrator/topology/instance_handler.py
--
diff --git a/aria/orchestrator/topology/instance_handler.py
b/aria/orchestrator/topology/instance_handler.py
index 2c32466..dbebf48 100644
--- a/aria/orchestrator/topology/instance_handler.py
+++ b/aria/orchestrator/topology/instance_handler.py
@@ -289,6 +289,14 @@ class Node(common._OperatorHolderHandlerMixin):
return target_node_template, target_node_capability
+elif requirement_template.target_capability_type is not None:
+for target_node_template in \
+
self._model.node_template.service_template.node_templates.itervalues():
+target_node_capability = \
+self._get_capability(requirement_template,
target_node_template)
+if target_node_capability:
+return target_node_template, target_node_capability
+
return None, None
def _get_capability(self, requirement_template, target_node_template=None):
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/4b6e51b0/tests/parser/service_templates.py
--
diff --git a/tests/parser/service_templates.py
b/tests/parser/service_templates.py
index 13712df..6b12576 100644
--- a/tests/parser/service_templates.py
+++ b/tests/parser/service_templates.py
@@ -49,13 +49,23 @@ def consume_use_case(use_case_name,
consumer_class_name='instance', cache=True):
def consume_node_cellar(consumer_class_name='instance', cache=True):
+consume_test_case(
+get_service_template_uri('tosca-simple-1.0', 'node-cellar',
'node-cellar.yaml'),
+consumer_class_name=consumer_class_name,
+inputs_uri=get_service_template_uri('tosca-simple-1.0', 'node-cellar',
'inputs.yaml'),
+cache=cache
+
+)
+
+
+def consume_test_case(uri, inputs_uri=None, consumer_class_name='instance',
cache=True):
cachedmethod.ENABLED = cache
-uri = get_service_template_uri('tosca-simple-1.0', 'node-cellar',
'node-cellar.yaml')
+uri = get_service_template_uri(uri)
context = create_context(uri)
-context.args.append('--inputs=' +
get_service_template_uri('tosca-simple-1.0', 'node-cellar',
- 'inputs.yaml'))
+if inputs_uri:
+context.args.append('--inputs=' + get_service_template_uri(inputs_uri))
consumer, dumper = create_consumer(context, consumer_class_name)
consumer.consume()
context.validation.dump_issues()
assert not context.validation.has_issues
-return context, dumper
+return context, dumper
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/4b6e51b0/tests/parser/test_reqs_caps.py
--
diff --git a/tests/parser/test_reqs_caps.py b/tests/parser/test_reqs_caps.py
new file mode 100644
index 000..0af2487
--- /dev/null
+++ b/tests/parser/test_reqs_caps.py
@@ -0,0 +1,29 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distribut
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user AviaE commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130244198 --- Diff: tests/orchestrator/execution_plugin/test_ctx_proxy_server.py --- @@ -39,26 +39,26 @@ def test_dict_prop_access_get_key(self, server): assert response == 'value1' def test_dict_prop_access_get_key_nested(self, server): -response = self.request(server, 'node', 'properties', 'prop2.nested_prop1') +response = self.request(server, 'node', 'properties', 'prop2', 'nested_prop1') assert response == 'nested_value1' def test_dict_prop_access_get_with_list_index(self, server): -response = self.request(server, 'node', 'properties', 'prop3[2].value') +response = self.request(server, 'node', 'properties', 'prop3', 2, 'value') assert response == 'value_2' --- End diff -- Add a test for modifying a list element. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user AviaE commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130244114
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
--- End diff --
Maybe add a comment here that says that `.index` raises a `ValueError` if
the value is not in the list. And that is why you check for `ValueError` in the
end. It wasn't clear for @mxmrlv also.
In addition, why does all the other logic under this line is in the `try`
if it doesn't expected to raise `ValueError`. I understand that extracting this
part will force you to refactor the order of the statements a bit - just a
thought =)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user AviaE commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130244178
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
+argspec = inspect.getargspec(current.func)
+# Don't count initial args fulfilled by the partial
+spec_args = argspec.args[len(current.args):]
+# Don't count keyword args fulfilled by the partial
+spec_args = tuple(a for a in spec_args if a not in
current.keywords)
+else:
+argspec = inspect.getargspec(current)
+spec_args = argspec.args
+
+callable_kwargs = {}
+if isinstance(arg, dict):
+# If the first arg is a dict, treat it as our kwargs
+# TODO: what if the first argument really needs to be a
dict?
+callable_kwargs = arg
+index += 1
+
+if argspec.varargs is not None:
+# Gobble the rest of the args
+callable_args = args[index:]
+else:
+# Take only what we need
+spec_args = tuple(a for a in spec_args if a not in
callable_kwargs)
+spec_args_count = len(spec_args)
+if inspect.ismethod(current):
+# Don't count "self" argument
+spec_args_count -= 1
+callable_args = args[index:index + spec_args_count]
+# Note: we might actually have fewer args than the
args_count, but that could be OK
+# if the user expects subsequent args to have default
values
+
+args_count = len(callable_args)
+if args_count > 1:
+index += args_count - 1
+
+current = current(*callable_args, **callable_kwargs)
+
else:
-raise RuntimeError('{0} cannot be processed in
{1}'.format(arg, args))
+raise RuntimeError('`{0}` cannot be processed in
{1}
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130240698
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
+argspec = inspect.getargspec(current.func)
+# Don't count initial args fulfilled by the partial
+spec_args = argspec.args[len(current.args):]
+# Don't count keyword args fulfilled by the partial
+spec_args = tuple(a for a in spec_args if a not in
current.keywords)
+else:
+argspec = inspect.getargspec(current)
+spec_args = argspec.args
+
+callable_kwargs = {}
+if isinstance(arg, dict):
+# If the first arg is a dict, treat it as our kwargs
--- End diff --
Indeed, this leads to an odd behavior
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130237525 --- Diff: aria/orchestrator/workflows/api/task.py --- @@ -140,13 +140,18 @@ def __init__(self, self.arguments = modeling_utils.merge_parameter_values(arguments, operation.arguments, model_cls=models.Argument) -if getattr(self.actor, 'outbound_relationships', None) is not None: + +actor = self.actor +if hasattr(actor, '_wrapped'): --- End diff -- why is this needed all of a sudden? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130240873
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
+argspec = inspect.getargspec(current.func)
+# Don't count initial args fulfilled by the partial
+spec_args = argspec.args[len(current.args):]
+# Don't count keyword args fulfilled by the partial
+spec_args = tuple(a for a in spec_args if a not in
current.keywords)
+else:
+argspec = inspect.getargspec(current)
+spec_args = argspec.args
+
+callable_kwargs = {}
+if isinstance(arg, dict):
+# If the first arg is a dict, treat it as our kwargs
+# TODO: what if the first argument really needs to be a
dict?
+callable_kwargs = arg
+index += 1
+
+if argspec.varargs is not None:
+# Gobble the rest of the args
+callable_args = args[index:]
+else:
+# Take only what we need
+spec_args = tuple(a for a in spec_args if a not in
callable_kwargs)
+spec_args_count = len(spec_args)
+if inspect.ismethod(current):
+# Don't count "self" argument
+spec_args_count -= 1
+callable_args = args[index:index + spec_args_count]
--- End diff --
what about the case you have extra args, these will be silently ignored.
Why can't we just "spill" the remaining args/kwargs into the function?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130240766
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
--- End diff --
Maybe add some documentation why we inspect the called method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130239769
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
+current[arg] = {}
+current = current[arg]
+
+# Call
elif callable(current):
-kwargs = {}
-remaining_args = args[index:]
-if isinstance(remaining_args[-1], collections.MutableMapping):
-kwargs = remaining_args[-1]
-remaining_args = remaining_args[:-1]
-current = current(*remaining_args, **kwargs)
-break
+if isinstance(current, functools.partial):
--- End diff --
how current could be partial?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130239725
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
--- End diff --
why not isinstance(current, dict)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130237470 --- Diff: aria/orchestrator/workflows/api/task.py --- @@ -140,13 +140,18 @@ def __init__(self, self.arguments = modeling_utils.merge_parameter_values(arguments, operation.arguments, model_cls=models.Argument) -if getattr(self.actor, 'outbound_relationships', None) is not None: --- End diff -- The notion was to remove the models module from the imports, this would be achieved in the near future. So duck typing was used. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130238262 --- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py --- @@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs): self.close() -def _process_ctx_request(ctx, args): +def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements current = ctx -num_args = len(args) index = 0 + --- End diff -- I think we need to think this semantics through, in the current situation we support assignment of value, given the last argument is value. Inserting the `=` might imply you can insert `=` at any phase. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130241087
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
--- End diff --
According to these semantics we lose any knowledge of whether we are
accessing a list or a dict
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130237066 --- Diff: aria/orchestrator/context/common.py --- @@ -38,10 +38,27 @@ class BaseContext(object): """ INSTRUMENTATION_FIELDS = ( +modeling.models.Service.inputs, --- End diff -- Why did all of these attributes got instrumented? the scope of the ctx is that operation (be it a relationship or a node operation), why does it need simplified access to these attributes? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130238356 --- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py --- @@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs): self.close() -def _process_ctx_request(ctx, args): +def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements --- End diff -- can we break this function into smaller functions? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request: https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130237667 --- Diff: aria/storage/collection_instrumentation.py --- @@ -213,32 +235,32 @@ def __init__(self, mapi, *args, **kwargs): """ The original model. -:param wrapped: model to be instrumented :param mapi: MAPI for the wrapped model +:param wrapped: model to be instrumented +:param instrumentation: instrumentation dict +:param instrumentation_kwargs: arguments for instrumentation class """ -super(_InstrumentedModel, self).__init__(*args, **kwargs) +super(_InstrumentedModel, self).__init__(instrumentation_kwargs=dict(mapi=mapi), + *args, **kwargs) self._mapi = mapi self._apply_instrumentation() -def __getattr__(self, item): -return_value = getattr(self._wrapped, item) -if isinstance(return_value, self._wrapped.__class__): -return _create_instrumented_model(return_value, self._mapi, self._instrumentation) -if isinstance(return_value, (list, dict)): -return _create_wrapped_model(return_value, self._mapi, self._instrumentation) -return return_value - def _apply_instrumentation(self): for field in self._instrumentation: +if field.parent.class_ != type(self._wrapped): --- End diff -- I think this might break the recursive instrumentation, i.e. `relationship.source_node.outbound_relationships[0].attributes` would not be instrumented. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130239705
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,43 +150,101 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args):
+def _process_ctx_request(ctx, args): # pylint:
disable=too-many-branches,too-many-statements
current = ctx
-num_args = len(args)
index = 0
+
+try:
+# TODO: should there be a way to escape "=" in case it is needed
as real argument?
+equals_index = args.index('=')
+if equals_index == 0:
+raise RuntimeError('The "=" argument cannot be first')
+if equals_index != len(args) - 2:
+raise RuntimeError('The "=" argument must be penultimate')
+modifying = True
+modifying_key = args[-3]
+modifying_value = args[-1]
+args = args[:-3]
+except ValueError:
+modifying = False
+modifying_key = None
+modifying_value = None
+
+num_args = len(args)
+
while index < num_args:
arg = args[index]
+
+# Object attribute
attr = _desugar_attr(current, arg)
-if attr:
+if attr is not None:
current = getattr(current, attr)
-elif isinstance(current, collections.MutableMapping):
-key = arg
-path_dict = _PathDictAccess(current)
-if index + 1 == num_args:
-# read dict prop by path
-value = path_dict.get(key)
-current = value
-elif index + 2 == num_args:
-# set dict prop by path
-value = args[index + 1]
-path_dict.set(key, value)
-current = None
-else:
-raise RuntimeError('Illegal argument while accessing dict')
-break
+
+# List entry
+elif isinstance(current, list) and _is_int(arg):
+current = current[int(arg)]
+
+# Dict (or dict-like object) value
+elif hasattr(current, '__getitem__'):
+if modifying and (not arg in current):
--- End diff --
arg not in current
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[incubator-ariatosca] Git Push Summary
Repository: incubator-ariatosca Updated Branches: refs/heads/ARIA-313-fix-handling-the-required-field-of-inputs [deleted] 51e4ed004
[GitHub] incubator-ariatosca pull request #187: ARIA-313 Fix handling the `required` ...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-ariatosca/pull/187 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
incubator-ariatosca git commit: ARIA-313 Fix handling the `required` field of inputs
Repository: incubator-ariatosca Updated Branches: refs/heads/master c2b8e65b4 -> 51e4ed004 ARIA-313 Fix handling the `required` field of inputs The required field is handled in the following way: topology_template inputs: - Every input that is declared as required must be supplied a value while creating the service. In addition, supplying inputs that were not declared in the topology template is forbidden. workflow inputs:* Every input that is declared as required must be supplied a value while creating/starting the execution. In addition, supplying inputs that were not declared in the policy_type is forbidden. * workflow inputs are defined as properties of policy_types that are derived from aria.Workflow operation and interface inputs: --- The validation of the required field of inputs that belong to operations and interfaces is done only in the parsing stage. This reasoning follows the TOSCA spirit, where anything that is declared as required in the type, must be assigned in the corresponding template I split the logic of merging provided and declared input values into three steps: 1. Validate that no undeclared inputs were provided. 2. Validate that all required inputs were provided with a value. 3. The actual merging process, which includes type checking. Project: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/commit/51e4ed00 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/51e4ed00 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/51e4ed00 Branch: refs/heads/master Commit: 51e4ed0041c3221723eee27a81ebbc49156048b6 Parents: c2b8e65 Author: Avia Efrat Authored: Wed Jul 26 15:11:21 2017 +0300 Committer: Avia Efrat Committed: Sun Jul 30 12:13:12 2017 +0300 -- aria/modeling/exceptions.py | 4 +- aria/modeling/mixins.py | 2 - aria/modeling/service_common.py | 13 +++ aria/modeling/service_instance.py | 2 +- aria/modeling/service_template.py | 5 + aria/modeling/utils.py | 79 ++--- aria/orchestrator/workflow_runner.py| 4 + aria/parser/consumption/style.py| 4 + aria/parser/presentation/presentation.py| 5 +- .../simple_v1_0/modeling/__init__.py| 114 +++ .../simple_v1_0/modeling/data_types.py | 5 +- .../simple_v1_0/modeling/interfaces.py | 12 +- .../simple_v1_0/modeling/parameters.py | 3 +- tests/orchestrator/test_workflow_runner.py | 18 +-- 14 files changed, 159 insertions(+), 111 deletions(-) -- http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/51e4ed00/aria/modeling/exceptions.py -- diff --git a/aria/modeling/exceptions.py b/aria/modeling/exceptions.py index 573efaf..cddc049 100644 --- a/aria/modeling/exceptions.py +++ b/aria/modeling/exceptions.py @@ -45,7 +45,7 @@ class CannotEvaluateFunctionException(ModelingException): """ -class MissingRequiredParametersException(ParameterException): +class MissingRequiredInputsException(ParameterException): """ ARIA modeling exception: Required parameters have been omitted. """ @@ -57,7 +57,7 @@ class ParametersOfWrongTypeException(ParameterException): """ -class UndeclaredParametersException(ParameterException): +class UndeclaredInputsException(ParameterException): """ ARIA modeling exception: Undeclared parameters have been provided. """ http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/51e4ed00/aria/modeling/mixins.py -- diff --git a/aria/modeling/mixins.py b/aria/modeling/mixins.py index 883ff4a..4acbe6e 100644 --- a/aria/modeling/mixins.py +++ b/aria/modeling/mixins.py @@ -161,8 +161,6 @@ class ParameterMixin(TemplateModelMixin, caching.HasCachedMethods): :func:`~aria.modeling.functions.evaluate`. """ -__tablename__ = 'parameter' - type_name = Column(Text, doc=""" Type name. http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/51e4ed00/aria/modeling/service_common.py -- diff --git a/aria/modeling/service_common.py b/aria/modeling/service_common.py index b533a88..4ce9dae 100644 --- a/aria/modeling/service_common.py +++ b/aria/modeling/service_common.py @@ -22,6 +22,7 @@ ARIA modeling service common module from sqlalchemy import ( Column, Text, +Boolean ) from sqlalchemy.ext.declarative import declared_attr @@ -84,6 +85,18 @@ class InputBas
incubator-ariatosca git commit: ARIA-313 Fix handling the `required` field of inputs [Forced Update!]
Repository: incubator-ariatosca Updated Branches: refs/heads/ARIA-313-fix-handling-the-required-field-of-inputs ee55bf4ca -> 51e4ed004 (forced update) ARIA-313 Fix handling the `required` field of inputs The required field is handled in the following way: topology_template inputs: - Every input that is declared as required must be supplied a value while creating the service. In addition, supplying inputs that were not declared in the topology template is forbidden. workflow inputs:* Every input that is declared as required must be supplied a value while creating/starting the execution. In addition, supplying inputs that were not declared in the policy_type is forbidden. * workflow inputs are defined as properties of policy_types that are derived from aria.Workflow operation and interface inputs: --- The validation of the required field of inputs that belong to operations and interfaces is done only in the parsing stage. This reasoning follows the TOSCA spirit, where anything that is declared as required in the type, must be assigned in the corresponding template I split the logic of merging provided and declared input values into three steps: 1. Validate that no undeclared inputs were provided. 2. Validate that all required inputs were provided with a value. 3. The actual merging process, which includes type checking. Project: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/commit/51e4ed00 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/51e4ed00 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/51e4ed00 Branch: refs/heads/ARIA-313-fix-handling-the-required-field-of-inputs Commit: 51e4ed0041c3221723eee27a81ebbc49156048b6 Parents: c2b8e65 Author: Avia Efrat Authored: Wed Jul 26 15:11:21 2017 +0300 Committer: Avia Efrat Committed: Sun Jul 30 12:13:12 2017 +0300 -- aria/modeling/exceptions.py | 4 +- aria/modeling/mixins.py | 2 - aria/modeling/service_common.py | 13 +++ aria/modeling/service_instance.py | 2 +- aria/modeling/service_template.py | 5 + aria/modeling/utils.py | 79 ++--- aria/orchestrator/workflow_runner.py| 4 + aria/parser/consumption/style.py| 4 + aria/parser/presentation/presentation.py| 5 +- .../simple_v1_0/modeling/__init__.py| 114 +++ .../simple_v1_0/modeling/data_types.py | 5 +- .../simple_v1_0/modeling/interfaces.py | 12 +- .../simple_v1_0/modeling/parameters.py | 3 +- tests/orchestrator/test_workflow_runner.py | 18 +-- 14 files changed, 159 insertions(+), 111 deletions(-) -- http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/51e4ed00/aria/modeling/exceptions.py -- diff --git a/aria/modeling/exceptions.py b/aria/modeling/exceptions.py index 573efaf..cddc049 100644 --- a/aria/modeling/exceptions.py +++ b/aria/modeling/exceptions.py @@ -45,7 +45,7 @@ class CannotEvaluateFunctionException(ModelingException): """ -class MissingRequiredParametersException(ParameterException): +class MissingRequiredInputsException(ParameterException): """ ARIA modeling exception: Required parameters have been omitted. """ @@ -57,7 +57,7 @@ class ParametersOfWrongTypeException(ParameterException): """ -class UndeclaredParametersException(ParameterException): +class UndeclaredInputsException(ParameterException): """ ARIA modeling exception: Undeclared parameters have been provided. """ http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/51e4ed00/aria/modeling/mixins.py -- diff --git a/aria/modeling/mixins.py b/aria/modeling/mixins.py index 883ff4a..4acbe6e 100644 --- a/aria/modeling/mixins.py +++ b/aria/modeling/mixins.py @@ -161,8 +161,6 @@ class ParameterMixin(TemplateModelMixin, caching.HasCachedMethods): :func:`~aria.modeling.functions.evaluate`. """ -__tablename__ = 'parameter' - type_name = Column(Text, doc=""" Type name. http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/51e4ed00/aria/modeling/service_common.py -- diff --git a/aria/modeling/service_common.py b/aria/modeling/service_common.py index b533a88..4ce9dae 100644 --- a/aria/modeling/service_common.py +++ b/aria/modeling/service_common.py @@ -22,6 +22,7 @@ ARIA modeling service common module from sqlalchemy import ( Column, Text,
