[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119449742
  
--- Diff: aria/modeling/utils.py ---
@@ -52,84 +53,95 @@ def service_template(self):
 return self.container.service_template
 
 
-def create_parameters(parameters, declared_parameters):
+def merge_parameter_values(parameter_values, declared_parameters, 
forbidden_names=None):
--- End diff --

:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119449679
  
--- Diff: extensions/aria_extension_tosca/simple_v1_0/modeling/functions.py 
---
@@ -69,7 +69,7 @@ def __evaluate__(self, container_holder):
 e, final = evaluate(e, final, container_holder)
 if e is not None:
 value.write(unicode(e))
-value = value.getvalue()
+value = value.getvalue() or u''
--- End diff --

:+1:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119449695
  
--- Diff: aria/orchestrator/workflows/events_logging.py ---
@@ -35,20 +35,20 @@ def _get_task_name(task):
 
 @events.start_task_signal.connect
 def _start_task_handler(task, **kwargs):
-# If the task has not implementation this is an empty task.
-if task.implementation:
+# If the task has no function this is an empty task.
+if task.function:
 suffix = 'started...'
 logger = task.context.logger.info
 else:
-suffix = 'has no implementation'
+suffix = 'has no function'
--- End diff --

:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119449568
  
--- Diff: aria/modeling/exceptions.py ---
@@ -57,3 +57,9 @@ class UndeclaredParametersException(ParameterException):
 """
 ARIA modeling exception: Undeclared parameters have been provided.
 """
+
+
+class ForbiddenParameterNamesException(ParameterException):
--- End diff --

:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119446847
  
--- Diff: tests/orchestrator/test_workflow_runner.py ---
@@ -259,8 +258,9 @@ def _setup_mock_workflow_in_service(request, 
inputs=None):
 workflow = models.Operation(
 name=mock_workflow_name,
 service=service,
-implementation='workflow.mock_workflow',
-inputs=inputs or {})
+function='workflow.mock_workflow',
+inputs=inputs or {},
+arguments=inputs or {})
--- End diff --

No, not in the way this particular test works, since it creates an 
operation model directly and doesn't call `configure()`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119446252
  
--- Diff: aria/modeling/exceptions.py ---
@@ -57,3 +57,9 @@ class UndeclaredParametersException(ParameterException):
 """
 ARIA modeling exception: Undeclared parameters have been provided.
 """
+
+
+class ForbiddenParameterNamesException(ParameterException):
--- End diff --

Changing to "Reserved".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119446194
  
--- Diff: extensions/aria_extension_tosca/simple_v1_0/modeling/functions.py 
---
@@ -69,7 +69,7 @@ def __evaluate__(self, container_holder):
 e, final = evaluate(e, final, container_holder)
 if e is not None:
 value.write(unicode(e))
-value = value.getvalue()
+value = value.getvalue() or u''
--- End diff --

We default to Unicode everywhere possible in values -- this avoids thorny 
problems later down the road with values.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119445955
  
--- Diff: aria/orchestrator/workflow_runner.py ---
@@ -136,10 +137,11 @@ def _validate_no_active_executions(self, execution):
 active_executions = [e for e in self.service.executions if 
e.is_active()]
 if active_executions:
 raise exceptions.ActiveExecutionsError(
-"Can't start execution; Service {0} has an active 
execution with id {1}"
+"Can't start execution; Service {0} has an active 
execution with ID {1}"
 .format(self.service.name, active_executions[0].id))
 
-def _get_workflow_fn(self):
+@property
+def _workflow_fn(self):
--- End diff --

Changing back to function.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119445875
  
--- Diff: aria/orchestrator/execution_plugin/instantiation.py ---
@@ -16,107 +16,132 @@
 # TODO: this module will eventually be moved to a new "aria.instantiation" 
package
 
 from ...utils.type import full_type_name
-from ...utils.collections import OrderedDict
+from ...utils.formatting import safe_repr
 from ...parser import validation
 from ...parser.consumption import ConsumptionContext
+from ...modeling.functions import Function
 
 
 def configure_operation(operation):
-configuration = OrderedDict(operation.configuration) if 
operation.configuration else {}
-
-arguments = OrderedDict()
-arguments['script_path'] = operation.implementation
-arguments['process'] = _get_process(configuration.pop('process')) \
-if 'process' in configuration else dict()
-
 host = None
 interface = operation.interface
 if interface.node is not None:
 host = interface.node.host
 elif interface.relationship is not None:
 if operation.relationship_edge is True:
 host = interface.relationship.target_node.host
-else: # either False or None
+else: # either False or None (None meaning that edge was not 
specified)
 host = interface.relationship.source_node.host
 
+_configure_common(operation)
 if host is None:
 _configure_local(operation)
 else:
-_configure_remote(operation, configuration, arguments)
+_configure_remote(operation)
+
+# Any remaining un-handled configuration parameters will become extra 
arguments, available as
+# kwargs in either "run_script_locally" or "run_script_with_ssh"
+for key, value in operation.configuration.iteritems():
+if key not in ('process', 'ssh'):
+operation.arguments[key] = value.instantiate()
 
-# Any remaining unhandled configuration values will become extra 
arguments, available as kwargs
-# in either "run_script_locally" or "run_script_with_ssh"
-arguments.update(configuration)
 
-return arguments
+def _configure_common(operation):
+"""
+Local and remote operations.
+"""
+
+from ...modeling.models import Parameter
+operation.arguments['script_path'] = Parameter.wrap('script_path', 
operation.implementation,
+'Relative path to 
the executable file.')
+operation.arguments['process'] = Parameter.wrap('process', 
_get_process(operation),
+'Sub-process 
configuration.')
+
 
 def _configure_local(operation):
 """
 Local operation.
 """
+
 from . import operations
-operation.implementation = '{0}.{1}'.format(operations.__name__,
-
operations.run_script_locally.__name__)
+operation.function = '{0}.{1}'.format(operations.__name__,
+  
operations.run_script_locally.__name__)
 
 
-def _configure_remote(operation, configuration, arguments):
+def _configure_remote(operation):
 """
 Remote SSH operation via Fabric.
 """
+
+from ...modeling.models import Parameter
+from . import operations
+
+ssh = _get_ssh(operation)
+
+# Defaults
 # TODO: find a way to configure these generally in the service template
 default_user = ''
 default_password = ''
-
-ssh = _get_ssh(configuration.pop('ssh')) if 'ssh' in configuration 
else {}
 if 'user' not in ssh:
 ssh['user'] = default_user
 if ('password' not in ssh) and ('key' not in ssh) and ('key_filename' 
not in ssh):
 ssh['password'] = default_password
 
-arguments['use_sudo'] = ssh.get('use_sudo', False)
-arguments['hide_output'] = ssh.get('hide_output', [])
-arguments['fabric_env'] = {}
+operation.arguments['use_sudo'] = Parameter.wrap('use_sudo', 
ssh.get('use_sudo', False),
--- End diff --

If you notice I am also giving the parameters descriptions here. These are 
not mere dicts, but true parameters.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119444625
  
--- Diff: 
tests/resources/service-templates/tosca-simple-1.0/node-cellar/node-cellar.yaml 
---
@@ -309,7 +311,7 @@ policy_types:
   client connections cleanly and shut down services. 
 derived_from: aria.Workflow
 properties:
-  implementation:
+  function:
--- End diff --

:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119443130
  
--- Diff: aria/modeling/exceptions.py ---
@@ -57,3 +57,9 @@ class UndeclaredParametersException(ParameterException):
 """
 ARIA modeling exception: Undeclared parameters have been provided.
 """
+
+
+class ForbiddenParameterNamesException(ParameterException):
--- End diff --

perhaps "Reserved" is more appropriate than "Forbidden"?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119427199
  
--- Diff: aria/orchestrator/workflow_runner.py ---
@@ -136,10 +137,11 @@ def _validate_no_active_executions(self, execution):
 active_executions = [e for e in self.service.executions if 
e.is_active()]
 if active_executions:
 raise exceptions.ActiveExecutionsError(
-"Can't start execution; Service {0} has an active 
execution with id {1}"
+"Can't start execution; Service {0} has an active 
execution with ID {1}"
 .format(self.service.name, active_executions[0].id))
 
-def _get_workflow_fn(self):
+@property
+def _workflow_fn(self):
--- End diff --

what's a `property` about this? it's a function with possible side effects, 
its private, and it isn't really meant to be used repeatedly, so i think it 
actually stands better as a function.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119428842
  
--- Diff: extensions/aria_extension_tosca/simple_v1_0/modeling/functions.py 
---
@@ -69,7 +69,7 @@ def __evaluate__(self, container_holder):
 e, final = evaluate(e, final, container_holder)
 if e is not None:
 value.write(unicode(e))
-value = value.getvalue()
+value = value.getvalue() or u''
--- End diff --

why do the value we return here need to be unicode again?
i understand the need to support unicode values which are set by the user, 
but why does it matter whether the value returned in case there never was a 
value is unicode or not?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119428083
  
--- Diff: aria/orchestrator/workflows/events_logging.py ---
@@ -35,20 +35,20 @@ def _get_task_name(task):
 
 @events.start_task_signal.connect
 def _start_task_handler(task, **kwargs):
-# If the task has not implementation this is an empty task.
-if task.implementation:
+# If the task has no function this is an empty task.
+if task.function:
 suffix = 'started...'
 logger = task.context.logger.info
 else:
-suffix = 'has no implementation'
+suffix = 'has no function'
--- End diff --

actually, in this specific instance, it should probably remain 
`implementation` - function is an implementation (sorry for the confusing term 
in this context :D) detail, but in practice what the message means is that the 
user didn't input (there i go with the terms again.. :D) any implementation for 
the operation (AKA empty operation)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r119443917
  
--- Diff: aria/orchestrator/execution_plugin/instantiation.py ---
@@ -16,107 +16,132 @@
 # TODO: this module will eventually be moved to a new "aria.instantiation" 
package
 
 from ...utils.type import full_type_name
-from ...utils.collections import OrderedDict
+from ...utils.formatting import safe_repr
 from ...parser import validation
 from ...parser.consumption import ConsumptionContext
+from ...modeling.functions import Function
 
 
 def configure_operation(operation):
-configuration = OrderedDict(operation.configuration) if 
operation.configuration else {}
-
-arguments = OrderedDict()
-arguments['script_path'] = operation.implementation
-arguments['process'] = _get_process(configuration.pop('process')) \
-if 'process' in configuration else dict()
-
 host = None
 interface = operation.interface
 if interface.node is not None:
 host = interface.node.host
 elif interface.relationship is not None:
 if operation.relationship_edge is True:
 host = interface.relationship.target_node.host
-else: # either False or None
+else: # either False or None (None meaning that edge was not 
specified)
 host = interface.relationship.source_node.host
 
+_configure_common(operation)
 if host is None:
 _configure_local(operation)
 else:
-_configure_remote(operation, configuration, arguments)
+_configure_remote(operation)
+
+# Any remaining un-handled configuration parameters will become extra 
arguments, available as
+# kwargs in either "run_script_locally" or "run_script_with_ssh"
+for key, value in operation.configuration.iteritems():
+if key not in ('process', 'ssh'):
+operation.arguments[key] = value.instantiate()
 
-# Any remaining unhandled configuration values will become extra 
arguments, available as kwargs
-# in either "run_script_locally" or "run_script_with_ssh"
-arguments.update(configuration)
 
-return arguments
+def _configure_common(operation):
+"""
+Local and remote operations.
+"""
+
+from ...modeling.models import Parameter
+operation.arguments['script_path'] = Parameter.wrap('script_path', 
operation.implementation,
+'Relative path to 
the executable file.')
+operation.arguments['process'] = Parameter.wrap('process', 
_get_process(operation),
+'Sub-process 
configuration.')
+
 
 def _configure_local(operation):
 """
 Local operation.
 """
+
 from . import operations
-operation.implementation = '{0}.{1}'.format(operations.__name__,
-
operations.run_script_locally.__name__)
+operation.function = '{0}.{1}'.format(operations.__name__,
+  
operations.run_script_locally.__name__)
 
 
-def _configure_remote(operation, configuration, arguments):
+def _configure_remote(operation):
 """
 Remote SSH operation via Fabric.
 """
+
+from ...modeling.models import Parameter
+from . import operations
+
+ssh = _get_ssh(operation)
+
+# Defaults
 # TODO: find a way to configure these generally in the service template
 default_user = ''
 default_password = ''
-
-ssh = _get_ssh(configuration.pop('ssh')) if 'ssh' in configuration 
else {}
 if 'user' not in ssh:
 ssh['user'] = default_user
 if ('password' not in ssh) and ('key' not in ssh) and ('key_filename' 
not in ssh):
 ssh['password'] = default_password
 
-arguments['use_sudo'] = ssh.get('use_sudo', False)
-arguments['hide_output'] = ssh.get('hide_output', [])
-arguments['fabric_env'] = {}
+operation.arguments['use_sudo'] = Parameter.wrap('use_sudo', 
ssh.get('use_sudo', False),
--- End diff --

It's true that configuration parameters are wrapped in another place when 
it's not intended for the execution plugin, but my question is why bother 
wrapping them at this stage at all - as opposed to working with dicts at this 
stage and then have the instantiation code which calls this module do the 
wrapping. it just makes more sense IMO.


---
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 

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron closed the pull request at:

https://github.com/apache/incubator-ariatosca/pull/138


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

GitHub user tliron reopened a pull request:

https://github.com/apache/incubator-ariatosca/pull/138

ARIA-149 Enhance operation configuration

* Parse special "dependencies" configuration parameters as YAML and
  treat as Parameter models, allowing them full use of intrinsic
  functions, type coersions, and validations
* Rename various functions that process "properties" to more generically
  process "parameters" (properties, inputs, attributes, arguments, etc.)
* The "configuration" field in OperationTemplate and Operation models
  is now now a dict of Parameter models
* Added "function" and "arguments" fields to Operation model to preserve
  user data (in "implementation" and "inputs") and to clearly demarcate
  orchestration data from user data
* Some cleanup of parser code touched by this commit

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/incubator-ariatosca 
ARIA-149-functions-in-operation-configuration

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-ariatosca/pull/138.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 #138


commit f6ee65a9eaa8eb252c4431152327635a43dff425
Author: Tal Liron 
Date:   2017-04-20T22:54:47Z

ARIA-149 Enhance operation configuration

* Parse special "dependencies" configuration parameters as YAML and
  treat as Parameter models, allowing them full use of intrinsic
  functions, type coersions, and validations
* Rename various functions that process "properties" to more generically
  process "parameters" (properties, inputs, attributes, arguments, etc.)
* The "configuration" field in OperationTemplate and Operation models
  is now now a dict of Parameter models
* Added "function" and "arguments" fields to Operation model to preserve
  user data (in "implementation" and "inputs") and to clearly demarcate
  orchestration data from user data
* Some cleanup of parser code touched by this commit

commit 888c5cd6f86a12e096a8ce040aedd0f62c5eac0e
Author: Tal Liron 
Date:   2017-05-24T19:54:07Z

Fixes

* Rename implementation/inputs to function/arguments in Task API
* Rename "create_parameters" to "merge_parameter_values" and improve
* Change workflow "function" back to "implementation"




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r118375237
  
--- Diff: aria/orchestrator/execution_plugin/instantiation.py ---
@@ -16,107 +16,132 @@
 # TODO: this module will eventually be moved to a new "aria.instantiation" 
package
 
 from ...utils.type import full_type_name
-from ...utils.collections import OrderedDict
+from ...utils.formatting import safe_repr
 from ...parser import validation
 from ...parser.consumption import ConsumptionContext
+from ...modeling.functions import Function
 
 
 def configure_operation(operation):
-configuration = OrderedDict(operation.configuration) if 
operation.configuration else {}
-
-arguments = OrderedDict()
-arguments['script_path'] = operation.implementation
-arguments['process'] = _get_process(configuration.pop('process')) \
-if 'process' in configuration else dict()
-
 host = None
 interface = operation.interface
 if interface.node is not None:
 host = interface.node.host
 elif interface.relationship is not None:
 if operation.relationship_edge is True:
 host = interface.relationship.target_node.host
-else: # either False or None
+else: # either False or None (None meaning that edge was not 
specified)
 host = interface.relationship.source_node.host
 
+_configure_common(operation)
 if host is None:
 _configure_local(operation)
 else:
-_configure_remote(operation, configuration, arguments)
+_configure_remote(operation)
+
+# Any remaining un-handled configuration parameters will become extra 
arguments, available as
+# kwargs in either "run_script_locally" or "run_script_with_ssh"
+for key, value in operation.configuration.iteritems():
+if key not in ('process', 'ssh'):
+operation.arguments[key] = value.instantiate()
 
-# Any remaining unhandled configuration values will become extra 
arguments, available as kwargs
-# in either "run_script_locally" or "run_script_with_ssh"
-arguments.update(configuration)
 
-return arguments
+def _configure_common(operation):
+"""
+Local and remote operations.
+"""
+
+from ...modeling.models import Parameter
+operation.arguments['script_path'] = Parameter.wrap('script_path', 
operation.implementation,
+'Relative path to 
the executable file.')
+operation.arguments['process'] = Parameter.wrap('process', 
_get_process(operation),
+'Sub-process 
configuration.')
+
 
 def _configure_local(operation):
 """
 Local operation.
 """
+
 from . import operations
-operation.implementation = '{0}.{1}'.format(operations.__name__,
-
operations.run_script_locally.__name__)
+operation.function = '{0}.{1}'.format(operations.__name__,
+  
operations.run_script_locally.__name__)
 
 
-def _configure_remote(operation, configuration, arguments):
+def _configure_remote(operation):
 """
 Remote SSH operation via Fabric.
 """
+
+from ...modeling.models import Parameter
+from . import operations
+
+ssh = _get_ssh(operation)
+
+# Defaults
 # TODO: find a way to configure these generally in the service template
 default_user = ''
 default_password = ''
-
-ssh = _get_ssh(configuration.pop('ssh')) if 'ssh' in configuration 
else {}
 if 'user' not in ssh:
 ssh['user'] = default_user
 if ('password' not in ssh) and ('key' not in ssh) and ('key_filename' 
not in ssh):
 ssh['password'] = default_password
 
-arguments['use_sudo'] = ssh.get('use_sudo', False)
-arguments['hide_output'] = ssh.get('hide_output', [])
-arguments['fabric_env'] = {}
+operation.arguments['use_sudo'] = Parameter.wrap('use_sudo', 
ssh.get('use_sudo', False),
--- End diff --

We went through the code together, this happens elsewhere.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r118375175
  
--- Diff: extensions/aria_extension_tosca/simple_v1_0/modeling/__init__.py 
---
@@ -384,18 +387,37 @@ def create_operation_template_model(context, 
service_template, operation):
 model.relationship_edge = True
 
 dependencies = implementation.dependencies
+configuration = OrderedDict()
 if dependencies:
 for dependency in dependencies:
 key, value = split_prefix(dependency)
 if key is not None:
-if model.configuration is None:
-model.configuration = {}
-set_nested(model.configuration, key.split('.'), value)
+# Parse as YAML
+try:
+value = yaml.load(value)
+except yaml.parser.MarkedYAMLError as e:
+context.validation.report(
+'YAML parser {0} in operation configuration: 
{1}'
+.format(e.problem, value),
+locator=implementation._locator,
+level=Issue.FIELD)
+continue
--- End diff --

Validation issues accumulate and are displayed in the end. None are 
considered fatal enough to start the parsing, but of course a single error 
means that parsing failed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[tliron]

Github user tliron commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r118374966
  
--- Diff: aria/modeling/exceptions.py ---
@@ -22,9 +22,9 @@ class ModelingException(AriaException):
 """
 
 
-class InputsException(ModelingException):
+class ParameterException(ModelingException):
--- End diff --

Discussed and we agreed that more general is better in this case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...

[ran-z]

Github user ran-z commented on a diff in the pull request:

https://github.com/apache/incubator-ariatosca/pull/138#discussion_r118293230
  
--- Diff: aria/modeling/service_instance.py ---
@@ -1728,34 +1732,41 @@ def operation_template(cls):
 def inputs(cls):
 return relationship.many_to_many(cls, 'parameter', 
prefix='inputs', dict_key='name')
 
+@declared_attr
+def configuration(cls):
+return relationship.many_to_many(cls, 'parameter', 
prefix='configuration', dict_key='name')
+
+@declared_attr
+def arguments(cls):
+return relationship.many_to_many(cls, 'parameter', 
prefix='arguments', dict_key='name')
+
 # endregion
 
 description = Column(Text)
 relationship_edge = Column(Boolean)
 implementation = Column(Text)
-configuration = Column(modeling_types.StrictDict(key_cls=basestring))
 dependencies = Column(modeling_types.StrictList(item_cls=basestring))
+function = Column(Text)
 executor = Column(Text)
 max_attempts = Column(Integer)
 retry_interval = Column(Integer)
 
 def configure(self):
-from . import models
-# Note: for workflows (operations attached directly to the 
service) "interface" will be None
-if (self.implementation is None) or (self.interface is None):
+if (self.implementation is None) and (self.function is None):
 return
 
-if self.plugin is None:
-arguments = 
execution_plugin.instantiation.configure_operation(self)
+if (self.plugin is None) and (self.interface is not None):
+# Default to execution plugin ("interface" is None for 
workflow operations)
+execution_plugin.instantiation.configure_operation(self)
 else:
 # In the future plugins may be able to add their own 
"configure_operation" hook that
-# can validate the configuration and otherwise return 
specially derived arguments
-arguments = self.configuration
+# can validate the configuration and otherwise create 
specially derived arguments. For
+# now, we just send all configuration parameters as arguments
+utils.instantiate_dict(self, self.arguments, 
self.configuration)
 
-# Note: the arguments will *override* operation inputs of the same 
name
-if arguments:
-for k, v in arguments.iteritems():
-self.inputs[k] = models.Parameter.wrap(k, v)
--- End diff --

:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---