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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---