You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ariatosca.apache.org by tliron <gi...@git.apache.org> on 2017/07/28 02:20:46 UTC
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx access
GitHub user tliron opened a pull request:
https://github.com/apache/incubator-ariatosca/pull/188
ARIA-324 Refactor ctx access
Our previous use of "." to delimit nested dict keys was wrong (keys
could have a ".") and inflexible. The new implementation uses subsequent
args to move into the dict.
The same format can now be used to access object attributes.
This commit also changes how to support setting values: we must now use
"=" as the penultimate argument with the new value following.
Also fixed: callables will now "grab" the number of args they need
instead of all remaining args, making it possible to do further
inspection on the returned value from the callable. To allow for this,
kwargs are now expected as the first arg rather than the last.
Furthmore, this commit fixes a bad null check in the ctx client, and
also allows it to retrieve Unicode data.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-ariatosca ARIA-324-refactor-ctx-access
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-ariatosca/pull/188.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 #188
----
commit 956e98b20874b99e80b12849b3c10dc869a4b4f6
Author: Tal Liron <ta...@gmail.com>
Date: 2017-07-27T22:58:17Z
ARIA-324 Refactor ctx proxy access
Our previous use of "." to delimit nested dict keys was wrong (keys
could have a ".") and inflexible. The new implementation uses subsequent
args to move into the dict. The same format can now be used to access
object attributes.
This commit also changes how to support setting values: we must now use
"=" as the penultimate argument with the new value following.
Also fixed: callables will now "grab" the number of args they need
instead of all remaining args, making it possible to do further
inspection on the returned value from the callable. To allow for this,
kwargs are now expected as the first arg rather than the last.
Relatedly, this commit instruments all parameter fields from all models
and fixes related bugs in the instrumentation implementation.
Furthmore, this commit fixes a bad null check in the ctx client, and
also allows it to retrieve Unicode data.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536784
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
--- End diff --
consider `isinstance(obj, (dict, list))`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536756
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
--- End diff --
consider `isinstance(obj, (list, 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca issue #188: ARIA-324 Refactor ctx proxy access
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-ariatosca/pull/188
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
GitHub user tliron reopened a pull request:
https://github.com/apache/incubator-ariatosca/pull/188
ARIA-324 Refactor ctx proxy access
Our previous use of "." to delimit nested dict keys was wrong (keys
could have a ".") and inflexible. The new implementation uses subsequent
args to move into the dict.
The same format can now be used to access object attributes.
This commit also changes how to support setting values: we must now use
"=" as the penultimate argument with the new value following.
Also fixed: callables will now "grab" the number of args they need
instead of all remaining args, making it possible to do further
inspection on the returned value from the callable. To allow for this,
kwargs are now expected as the first arg rather than the last.
Furthmore, this commit fixes a bad null check in the ctx client, and
also allows it to retrieve Unicode data.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-ariatosca ARIA-324-refactor-ctx-access
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-ariatosca/pull/188.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 #188
----
commit 8a3f8b73560350b40d6c2ff47d0bbbb5f589b0be
Author: Tal Liron <ta...@gmail.com>
Date: 2017-07-27T22:58:17Z
ARIA-324 Refactor ctx proxy access
Our previous use of "." to delimit nested dict keys was wrong (keys
could have a ".") and inflexible. The new implementation uses subsequent
args to move into the dict. The same format can now be used to access
object attributes.
This commit also changes how to support setting values: we must now use
"=" as the penultimate argument with the new value following.
Calling functions now requires delimiting function arguments via "[" and
"]" arguments. This makes it possible to continue accessing the return
value of a function after the "]".
Also fixed: callables will now "grab" the number of args they need
instead of all remaining args, making it possible to do further
inspection on the returned value from the callable. To allow for this,
kwargs are now expected as the first arg rather than the last.
Relatedly, this commit instruments all parameter fields from all models
and fixes related bugs in the instrumentation implementation.
Furthmore, this commit fixes a bad null check in the ctx client, and
also allows it to retrieve Unicode data.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131905254
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
--- End diff --
but who promises my the support of `has_key`?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by AviaE <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131702731
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
--- End diff --
But you wouldn't know in advance which function to call until you start parsing. I renamed it and did some little cleanups to add clarity.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131693627
--- Diff: tests/orchestrator/execution_plugin/test_ctx_proxy_server.py ---
@@ -55,61 +55,32 @@ def test_dict_prop_access_set(self, server, ctx):
assert ctx.node.properties['prop3'][2]['value'] == 'new_value_2'
assert ctx.node.properties['prop4']['some']['new']['path'] == 'some_new_value'
+ def test_dict_prop_access_set_with_list_index(self, server, ctx):
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131904400
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
--- End diff --
ducktyping much? :smiley: but ok
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536661
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
--- End diff --
consider renaming it, since it accepts all the args (and not a single arg)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536426
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,17 +146,29 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements
- current = ctx
- index = 0
+class ProcessingError(RuntimeError):
+ pass
+
+class ParsingError(ProcessingError):
+ pass
+
+
+def _parse_request(ctx, request):
+ request = json.loads(request)
+ args = request['args']
+ return _parse_arguments(ctx, args)
+
+
+def _parse_arguments(obj, args):
+ # Modifying?
try:
# TODO: should there be a way to escape "=" in case it is needed as real argument?
- equals_index = args.index('=')
+ equals_index = args.index('=') # raises ValueError if not found
--- End diff --
as @AviaE mentioned, consider confining the try and except to this line only?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536398
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,17 +146,29 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements
- current = ctx
- index = 0
+class ProcessingError(RuntimeError):
+ pass
+
+class ParsingError(ProcessingError):
+ pass
+
+
+def _parse_request(ctx, request):
+ request = json.loads(request)
+ args = request['args']
+ return _parse_arguments(ctx, args)
+
+
+def _parse_arguments(obj, args):
--- End diff --
consider better arguments naming
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron closed the pull request at:
https://github.com/apache/incubator-ariatosca/pull/188
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536369
--- Diff: tests/orchestrator/execution_plugin/test_ctx_proxy_server.py ---
@@ -55,61 +55,32 @@ def test_dict_prop_access_set(self, server, ctx):
assert ctx.node.properties['prop3'][2]['value'] == 'new_value_2'
assert ctx.node.properties['prop4']['some']['new']['path'] == 'some_new_value'
+ def test_dict_prop_access_set_with_list_index(self, server, ctx):
--- End diff --
:+1:
consider also adding test for something like `node.some_attr.some_method(*some_args).return_attr = some_value`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca issue #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131703940
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
+ obj[arg] = {}
--- End diff --
I think it's non-trivial and not useful to set nested lists that have no value. Consider this:
```
x = ['a','b','c']
ctx x 10 name = 2
```
What is to be done with indexes between 3 and 9 of the list? Fill them with None? With empty dicts? It is unclear what would be best. I think it is best in such situations for ARIA to fail with an error message. Only let users modify existing index values.
I recommend leaving it as is.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131906519
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
+ obj[arg] = {}
--- End diff --
but what about ctx x 2 name = value?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca issue #188: ARIA-324 Refactor ctx access
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-ariatosca/pull/188
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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}'.format(arg, args))
+
index += 1
+
if callable(current):
current = current()
+
+ if modifying:
+ if hasattr(current, '__setitem__'):
+ # Modify dict value
+ current[modifying_key] = modifying_value
--- End diff --
+1
Will add a test for this, too.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r130399573
--- 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 --
To fix: check by isubclass instead of equality
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536517
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
--- End diff --
`not obj.has_keyt(arg)` -> `arg not in obj`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by AviaE <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-ariatosca/pull/188
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131700830
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
--- End diff --
Cannot, because it wouldn't support dict-like objects like instrumented wrapper classes. This is generally better because it would support anything that supported __get_item__ and __set_item__, which allows for any kind of classes.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by AviaE <gi...@git.apache.org>.
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}'.format(arg, args))
+
index += 1
+
if callable(current):
current = current()
+
+ if modifying:
+ if hasattr(current, '__setitem__'):
+ # Modify dict value
+ current[modifying_key] = modifying_value
--- End diff --
since `modifying_value` is a string, this raises an error when trying to modify the value of a list element.
So in the current implementation you won't able to do thing of the sorts:
`node properties prop5 0 = new_value`
(I tested this)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536933
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,17 +146,29 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements
- current = ctx
- index = 0
+class ProcessingError(RuntimeError):
--- End diff --
consider `CtxArgProcessingError`, or any other name since `ProcessingError` makes it sound like its a process related issue.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536376
--- Diff: examples/hello-world/scripts/stop.sh ---
@@ -16,14 +16,13 @@
set -e
-TEMP_DIR="/tmp"
-PYTHON_FILE_SERVER_ROOT=${TEMP_DIR}/python-simple-http-webserver
-PID_FILE="server.pid"
+TEMP_DIR=/tmp
+PYTHON_FILE_SERVER_ROOT="${TEMP_DIR}/python-simple-http-webserver"
+PID_FILE=server.pid
+PID=$(cat "$PYTHON_FILE_SERVER_ROOT/$PID_FILE")
-PID=`cat ${PYTHON_FILE_SERVER_ROOT}/${PID_FILE}`
+ctx logger info [ "Shutting down web server, pid = ${PID}." ]
+kill -9 "$PID" || exit $?
-ctx logger info "Shutting down web server, pid = ${PID}."
-kill -9 ${PID} || exit $?
-
-ctx logger info "Removing web server root folder: ${PYTHON_FILE_SERVER_ROOT}."
-rm -rf ${PYTHON_FILE_SERVER_ROOT}
+ctx logger info [ "Removing web server root folder: $PYTHON_FILE_SERVER_ROOT." ]
--- End diff --
looks awesome
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131703163
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
--- End diff --
Not a good idea. :( Because the "not in" requires an implementation of __iter__, but __iter__ might not necessarily adhere to the dict contract of iterating only keys. (I did add the "right" kind of __iter__ to the instrumented wrapper, but nobody can promise us that other objects would behave as expected here.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536641
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
+ obj[arg] = {}
--- End diff --
why is it set to a dict? couldn't it be a list?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131693585
--- Diff: examples/hello-world/scripts/stop.sh ---
@@ -16,14 +16,13 @@
set -e
-TEMP_DIR="/tmp"
-PYTHON_FILE_SERVER_ROOT=${TEMP_DIR}/python-simple-http-webserver
-PID_FILE="server.pid"
+TEMP_DIR=/tmp
+PYTHON_FILE_SERVER_ROOT="${TEMP_DIR}/python-simple-http-webserver"
+PID_FILE=server.pid
+PID=$(cat "$PYTHON_FILE_SERVER_ROOT/$PID_FILE")
-PID=`cat ${PYTHON_FILE_SERVER_ROOT}/${PID_FILE}`
+ctx logger info [ "Shutting down web server, pid = ${PID}." ]
+kill -9 "$PID" || exit $?
-ctx logger info "Shutting down web server, pid = ${PID}."
-kill -9 ${PID} || exit $?
-
-ctx logger info "Removing web server root folder: ${PYTHON_FILE_SERVER_ROOT}."
-rm -rf ${PYTHON_FILE_SERVER_ROOT}
+ctx logger info [ "Removing web server root folder: $PYTHON_FILE_SERVER_ROOT." ]
--- End diff --
Heh, not 100% sure I agree with that, but it does look clear. :)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131700894
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -150,17 +146,29 @@ def __exit__(self, *args, **kwargs):
self.close()
-def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-many-statements
- current = ctx
- index = 0
+class ProcessingError(RuntimeError):
+ pass
+
+class ParsingError(ProcessingError):
+ pass
+
+
+def _parse_request(ctx, request):
+ request = json.loads(request)
+ args = request['args']
+ return _parse_arguments(ctx, args)
+
+
+def _parse_arguments(obj, args):
+ # Modifying?
try:
# TODO: should there be a way to escape "=" in case it is needed as real argument?
- equals_index = args.index('=')
+ equals_index = args.index('=') # raises ValueError if not found
--- End diff --
It looks more awkward, but +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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131909167
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
--- End diff --
Correct, this is *exactly* a case for ducktyping. :) Any object can be "dict-like" here by just implementing this magic method, and ctx will be happy.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131536706
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
--- End diff --
i would consider splitting this function into two different functions (since the only usage for the entire args is being made in case of a function call
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #188: ARIA-324 Refactor ctx proxy access
Posted by tliron <gi...@git.apache.org>.
Github user tliron commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/188#discussion_r131909427
--- Diff: aria/orchestrator/execution_plugin/ctx_proxy/server.py ---
@@ -170,101 +178,59 @@ def _process_ctx_request(ctx, args): # pylint: disable=too-many-branches,too-man
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 is not None:
- current = getattr(current, attr)
-
- # 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):
- 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))
-
- index += 1
-
- if callable(current):
- current = current()
+ # Parse all arguments
+ while len(args) > 0:
+ obj, args = _parse_argument(obj, args, modifying)
if modifying:
- if hasattr(current, '__setitem__'):
- # Modify dict value
- current[modifying_key] = modifying_value
- else:
+ if hasattr(obj, '__setitem__'):
+ # Modify item value (dict, list, and similar)
+ if isinstance(obj, (list, tuple)):
+ modifying_key = int(modifying_key)
+ obj[modifying_key] = modifying_value
+ elif hasattr(obj, modifying_key):
# Modify object attribute
- setattr(current, modifying_key, modifying_value)
-
- return current
+ setattr(obj, modifying_key, modifying_value)
+ else:
+ raise ProcessingError('Cannot modify `{0}` of `{1!r}`'
+ .format(modifying_key, obj))
+ return obj
-def _desugar_attr(obj, attr):
- if not isinstance(attr, basestring):
- return None
- if hasattr(obj, attr):
- return attr
- attr = attr.replace('-', '_')
- if hasattr(obj, attr):
- return attr
- return None
+def _parse_argument(obj, args, modifying):
+ args = list(args)
+ arg = args.pop(0)
-def _is_int(arg):
- try:
- int(arg)
- except ValueError:
- return False
- return True
+ # Call?
+ if arg == '[':
+ # TODO: should there be a way to escape "[" and "]" in case they are needed as real
+ # arguments?
+ try:
+ closing_index = args.index(']')
+ except ValueError:
+ raise ParsingError('Opening "[" without a closing "]')
+ callable_args = args[:closing_index]
+ args = args[closing_index + 1:]
+ if not callable(obj):
+ raise ProcessingError('Used "[" and "] on an object that is not callable')
+ return obj(*callable_args), args
+
+ # Attribute?
+ if isinstance(arg, basestring):
+ if hasattr(obj, arg):
+ return getattr(obj, arg), args
+ arg_sugared = arg.replace('-', '_')
+ if hasattr(obj, arg_sugared):
+ return getattr(obj, arg_sugared), args
+
+ # Item? (dict, lists, and similar)
+ if hasattr(obj, '__getitem__'):
+ if modifying and hasattr(obj, 'has_key') and (not obj.has_key(arg)):
+ obj[arg] = {}
--- End diff --
That works, as long as index 2 already exists in `x`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---