Mailing-List: contact commits-help@ariatosca.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ariatosca.incubator.apache.org
From: tliron <git@git.apache.org>
To: commits@ariatosca.apache.org
Reply-To: commits@ariatosca.apache.org
References: <git-pr-138-incubator-ariatosca@git.apache.org>
In-Reply-To: <git-pr-138-incubator-ariatosca@git.apache.org>
Subject: [GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...
Content-Type: text/plain
Message-Id: <20170524215558.42C39DFB01@git1-us-west.apache.org>
Date: Wed, 24 May 2017 21:55:58 +0000 (UTC)
archived-at: Wed, 24 May 2017 21:56:00 -0000

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---