ariatosca-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ran-z <...@git.apache.org>
Subject [GitHub] incubator-ariatosca pull request #138: ARIA-149 Enhance operation configurat...
Date Wed, 31 May 2017 18:57:04 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message