You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ariatosca.apache.org by AviaE <gi...@git.apache.org> on 2017/07/26 12:28:57 UTC
[GitHub] incubator-ariatosca pull request #187: ARIA-313 Fix handling the `required` ...
GitHub user AviaE opened a pull request:
https://github.com/apache/incubator-ariatosca/pull/187
ARIA-313 Fix handling the `required` field of inputs
I split the logic of merging provided and declared input values into
three steps:
1. Validate that no undeclared inputs were provided.
2. Validate that all required inputs were provided with a value.
3. The actual merging process, which includes type checking.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-ariatosca ARIA-313-fix-handling-the-required-field-of-inputs
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-ariatosca/pull/187.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 #187
----
commit fd380f44888aa2b18209a831d0855547ba1634bb
Author: Avia Efrat <av...@gigaspaces.com>
Date: 2017-07-26T12:11:21Z
ARIA-313 Fix handling the `required` field of inputs
I split the logic of merging provided and declared input values into
three steps:
1. Validate that no undeclared inputs were provided.
2. Validate that all required inputs were provided with a value.
3. The actual merging process, which includes type checking.
----
---
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129571836
--- Diff: aria/modeling/utils.py ---
@@ -64,81 +64,84 @@ def service_template(self):
return self.container.service_template
-def merge_parameter_values(parameter_values, declared_parameters, model_cls):
+def validate_no_undeclared_inputs(declared_inputs, supplied_inputs):
+
+ undeclared_inputs = [input for input in supplied_inputs if input not in declared_inputs]
+ if undeclared_inputs:
+ raise exceptions.UndeclaredInputsException(
+ 'Undeclared inputs have been provided: {0}; Declared inputs: {1}'
+ .format(string_list_as_string(list(undeclared_inputs)),
+ string_list_as_string(declared_inputs.keys())))
+
+
+def validate_required_inputs_are_supplied(declared_inputs, supplied_inputs):
+ required_inputs = [input for input in declared_inputs.values() if input.required]
+ missing_required_inputs = [input for input in required_inputs
+ if input.name not in supplied_inputs and not input.value]
+ if missing_required_inputs:
+ raise exceptions.MissingRequiredInputsException(
+ 'Required inputs {0} have not been provided values'
+ .format(string_list_as_string(missing_required_inputs)))
+
+
+def merge_parameter_values(provided_values, declared_parameters, model_cls):
"""
Merges parameter values according to those declared by a type.
Exceptions will be raised for validation errors.
- :param parameter_values: provided parameter values or None
- :type parameter_values: {:obj:`basestring`: object}
- :param declared_parameters: declared parameters
+ :param provided_values: provided parameter values or None
+ :type provided_values: {:obj:`basestring`: object}
+ :param declared_parameters: declared code_parameters
:type declared_parameters: {:obj:`basestring`: :class:`~aria.modeling.models.Parameter`}
- :return: the merged parameters
+ :param model_cls: the model class that should be created from a provided value
+ :type model_cls: :class:`~aria.modeling.models.Input` or :class:`~aria.modeling.models.Argument`
+ :return: the merged code_parameters
--- End diff --
`code_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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129567839
--- Diff: aria/modeling/service_common.py ---
@@ -84,6 +85,18 @@ class InputBase(ParameterMixin):
__tablename__ = 'input'
+ required = Column(Boolean, doc="""
+ Is the input mandatory.
+
+ :type: :obj:`bool`
+ """)
+
+ @classmethod
+ def wrap(cls, name, value, description=None, required=True):
--- End diff --
the parent function has different signature (no required), maybe add kwargs to the parent 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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129574408
--- Diff: aria/orchestrator/workflows/api/task.py ---
@@ -137,6 +137,11 @@ def __init__(self,
operation = self.actor.interfaces[self.interface_name].operations[self.operation_name]
self.plugin = operation.plugin
self.function = operation.function
+
+ modeling_utils.validate_required_inputs_are_supplied(
+ declared_inputs=operation.inputs,
+ supplied_inputs=operation.arguments)
--- End diff --
`supplied_inputs` should be both `operation.arguments` and `arguments`. And add explanation why the validate and the actual merging process are different.
---
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129576919
--- Diff: examples/tosca-simple-1.0/use-cases/webserver-dbms-2/webserver-dbms-2.yaml ---
@@ -50,7 +50,7 @@ topology_template:
interfaces:
Standard:
configure:
- implementation: scripts/nodejs/configure.sh
+ implementation: scripts/nodejs/c\onfigure.sh
--- End diff --
\\?
---
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129570988
--- Diff: aria/modeling/service_template.py ---
@@ -343,6 +343,11 @@ def instantiate(self, container, model_storage, inputs=None): # pylint: disable
context = ConsumptionContext.get_thread_local()
context.modeling.instance = service
+ utils.validate_no_undeclared_inputs(declared_inputs=self.inputs,
--- End diff --
Handle cases where `supplied_inputs` is `None`
---
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129580541
--- Diff: extensions/aria_extension_tosca/simple_v1_0/modeling/__init__.py ---
@@ -577,14 +554,44 @@ def added_all():
container.children.append(model)
-def create_parameter_models_from_values(properties, source_properties, model_cls):
+def create_input_models_from_values(inputs_field, template_inputs):
--- End diff --
Try and remove in-place update....
---
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 #187: ARIA-313 Fix handling the `required` ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-ariatosca/pull/187
---
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129571753
--- Diff: aria/modeling/utils.py ---
@@ -64,81 +64,84 @@ def service_template(self):
return self.container.service_template
-def merge_parameter_values(parameter_values, declared_parameters, model_cls):
+def validate_no_undeclared_inputs(declared_inputs, supplied_inputs):
+
+ undeclared_inputs = [input for input in supplied_inputs if input not in declared_inputs]
+ if undeclared_inputs:
+ raise exceptions.UndeclaredInputsException(
+ 'Undeclared inputs have been provided: {0}; Declared inputs: {1}'
+ .format(string_list_as_string(list(undeclared_inputs)),
+ string_list_as_string(declared_inputs.keys())))
+
+
+def validate_required_inputs_are_supplied(declared_inputs, supplied_inputs):
+ required_inputs = [input for input in declared_inputs.values() if input.required]
+ missing_required_inputs = [input for input in required_inputs
+ if input.name not in supplied_inputs and not input.value]
+ if missing_required_inputs:
+ raise exceptions.MissingRequiredInputsException(
+ 'Required inputs {0} have not been provided values'
+ .format(string_list_as_string(missing_required_inputs)))
+
+
+def merge_parameter_values(provided_values, declared_parameters, model_cls):
"""
Merges parameter values according to those declared by a type.
Exceptions will be raised for validation errors.
- :param parameter_values: provided parameter values or None
- :type parameter_values: {:obj:`basestring`: object}
- :param declared_parameters: declared parameters
+ :param provided_values: provided parameter values or None
+ :type provided_values: {:obj:`basestring`: object}
+ :param declared_parameters: declared code_parameters
--- End diff --
just `parameters`, not `code_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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129910938
--- Diff: aria/orchestrator/workflows/api/task.py ---
@@ -137,6 +137,11 @@ def __init__(self,
operation = self.actor.interfaces[self.interface_name].operations[self.operation_name]
self.plugin = operation.plugin
self.function = operation.function
+
+ modeling_utils.validate_required_inputs_are_supplied(
+ declared_inputs=operation.inputs,
+ supplied_inputs=operation.arguments)
--- End diff --
After consulting with @tliron and reviewing the TOSCA spec, I came to the conclusion that the validation of the `required` field of inputs that belong to operations and interfaces (as opposed to topology template and workflow inputs) should be done in the parsing stage.
This reasoning follows the TOSCA spirit, where anything that is declared in the type and is required, must be assigned in the corresponding template.
---
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129569763
--- Diff: aria/modeling/utils.py ---
@@ -64,81 +64,84 @@ def service_template(self):
return self.container.service_template
-def merge_parameter_values(parameter_values, declared_parameters, model_cls):
+def validate_no_undeclared_inputs(declared_inputs, supplied_inputs):
+
+ undeclared_inputs = [input for input in supplied_inputs if input not in declared_inputs]
+ if undeclared_inputs:
+ raise exceptions.UndeclaredInputsException(
+ 'Undeclared inputs have been provided: {0}; Declared inputs: {1}'
+ .format(string_list_as_string(list(undeclared_inputs)),
--- End diff --
No need for `list`, it is already 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 issue #187: ARIA-313 Fix handling the `required` field o...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-ariatosca/pull/187
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 #187: ARIA-313 Fix handling the `required` ...
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/187#discussion_r129573532
--- Diff: aria/orchestrator/workflow_runner.py ---
@@ -137,6 +137,10 @@ def _create_execution_model(self, inputs):
else:
workflow_inputs = self.service.workflows[self._workflow_name].inputs
+ modeling_utils.validate_no_undeclared_inputs(declared_inputs=workflow_inputs,
--- End diff --
`supplied_inputs` could be `None`
---
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.
---