From 5892bf97d08bbb51501d640cabfc310370a9f457 Mon Sep 17 00:00:00 2001 From: Yaqi Ji Date: Thu, 7 Oct 2021 16:48:11 -0700 Subject: [PATCH] fix(sdk): update v2 yaml format (#6661) * fix(sdk): change names to input_name and output_name * ordered dict and schema version numbering * Remove schema_version, annotatations and labels * change container/dagspec format * change to bracket * fix tests * fix py3.6 forwardref issue * fix ordered dict * address comments * release notes + test * address comments --- sdk/RELEASE.md | 1 + .../test_data/experimental_v2_component.py | 9 +- .../experimental/base_component_test.py | 8 +- .../components/experimental/component_spec.py | 232 +++++----- .../experimental/component_spec_test.py | 407 ++++++++---------- 5 files changed, 309 insertions(+), 348 deletions(-) diff --git a/sdk/RELEASE.md b/sdk/RELEASE.md index 10f60b177ac..63d3c51960c 100644 --- a/sdk/RELEASE.md +++ b/sdk/RELEASE.md @@ -17,6 +17,7 @@ * Fix executor getting None as value when float 0 is passed in. [\#6682](https://github.com/kubeflow/pipelines/pull/6682) * Fix function-based components not preserving the namespace of GCPC artifact types. [\#6702](https://github.com/kubeflow/pipelines/pull/6702) * Depends on `typing-extensions>=3.7.4,<4; python_version<"3.9"` [\#6683](https://github.com/kubeflow/pipelines/pull/6683) +* Update v2 yaml format [\#6661](https://github.com/kubeflow/pipelines/pull/6661) ## Documentation Updates diff --git a/sdk/python/kfp/v2/compiler_cli_tests/test_data/experimental_v2_component.py b/sdk/python/kfp/v2/compiler_cli_tests/test_data/experimental_v2_component.py index 711037c9efc..ff15cfd3e9b 100644 --- a/sdk/python/kfp/v2/compiler_cli_tests/test_data/experimental_v2_component.py +++ b/sdk/python/kfp/v2/compiler_cli_tests/test_data/experimental_v2_component.py @@ -27,16 +27,17 @@ def execute(self, *args, **kwargs): component_op = TestComponent( component_spec=component_spec.ComponentSpec( name='component_1', - implementation=component_spec.ContainerSpec( + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( image='alpine', commands=[ 'sh', '-c', 'set -ex\necho "$0" > "$1"', - component_spec.InputValuePlaceholder(input_value='input1'), - component_spec.OutputPathPlaceholder(output_path='output1'), + component_spec.InputValuePlaceholder(input_name='input1'), + component_spec.OutputPathPlaceholder(output_name='output1'), ], - ), + )), inputs={'input1': component_spec.InputSpec(type='String')}, outputs={'output1': component_spec.OutputSpec(type='String')}, )) diff --git a/sdk/python/kfp/v2/components/experimental/base_component_test.py b/sdk/python/kfp/v2/components/experimental/base_component_test.py index 9ea3880e113..fc59da573ef 100644 --- a/sdk/python/kfp/v2/components/experimental/base_component_test.py +++ b/sdk/python/kfp/v2/components/experimental/base_component_test.py @@ -36,10 +36,10 @@ def execute(self, *args, **kwargs): 'sh', '-c', 'set -ex\necho "$0" "$1" "$2" > "$3"', - component_spec.InputValuePlaceholder(input_value='input1'), - component_spec.InputValuePlaceholder(input_value='input2'), - component_spec.InputValuePlaceholder(input_value='input3'), - component_spec.OutputPathPlaceholder(output_path='output1'), + component_spec.InputValuePlaceholder(input_name='input1'), + component_spec.InputValuePlaceholder(input_name='input2'), + component_spec.InputValuePlaceholder(input_name='input3'), + component_spec.OutputPathPlaceholder(output_name='output1'), ], ), inputs={ diff --git a/sdk/python/kfp/v2/components/experimental/component_spec.py b/sdk/python/kfp/v2/components/experimental/component_spec.py index 637e168f72f..6caa8c8ff3b 100644 --- a/sdk/python/kfp/v2/components/experimental/component_spec.py +++ b/sdk/python/kfp/v2/components/experimental/component_spec.py @@ -14,7 +14,6 @@ """Definitions for component spec.""" import dataclasses -import enum import itertools import json from typing import Any, Dict, Mapping, Optional, Sequence, Union @@ -25,7 +24,14 @@ import yaml -class InputSpec(pydantic.BaseModel): +class BaseModel(pydantic.BaseModel): + + class Config: + allow_population_by_field_name = True + arbitrary_types_allowed = True + + +class InputSpec(BaseModel): """Component input definitions. Attributes: @@ -37,7 +43,7 @@ class InputSpec(pydantic.BaseModel): default: Optional[Union[str, int, float, bool, dict, list]] = None -class OutputSpec(pydantic.BaseModel): +class OutputSpec(BaseModel): """Component output definitions. Attributes: @@ -46,7 +52,7 @@ class OutputSpec(pydantic.BaseModel): type: str -class BasePlaceholder(pydantic.BaseModel): +class BasePlaceholder(BaseModel): """Base class for placeholders that could appear in container cmd and args. """ pass @@ -56,51 +62,51 @@ class InputValuePlaceholder(BasePlaceholder): """Class that holds input value for conditional cases. Attributes: - input_value: name of the input. + input_name: name of the input. """ - input_value: str + input_name: str = pydantic.Field(alias='inputValue') class InputPathPlaceholder(BasePlaceholder): """Class that holds input path for conditional cases. Attributes: - input_path: name of the input. + input_name: name of the input. """ - input_path: str + input_name: str = pydantic.Field(alias='inputPath') class InputUriPlaceholder(BasePlaceholder): """Class that holds input uri for conditional cases. Attributes: - input_uri: name of the input. + input_name: name of the input. """ - input_uri: str + input_name: str = pydantic.Field(alias='inputUri') class OutputPathPlaceholder(BasePlaceholder): """Class that holds output path for conditional cases. Attributes: - output_path: name of the output. + output_name: name of the output. """ - output_path: str + output_name: str = pydantic.Field(alias='outputPath') class OutputUriPlaceholder(BasePlaceholder): """Class that holds output uri for conditional cases. Attributes: - output_uri: name of the output. + output_name: name of the output. """ - output_uri: str + output_name: str = pydantic.Field(alias='outputUri') ValidCommandArgs = Union[str, InputValuePlaceholder, InputPathPlaceholder, InputUriPlaceholder, OutputPathPlaceholder, - OutputUriPlaceholder, "IfPresentPlaceholder", - "ConcatPlaceholder"] + OutputUriPlaceholder, 'IfPresentPlaceholder', + 'ConcatPlaceholder'] class ConcatPlaceholder(BasePlaceholder): @@ -112,11 +118,11 @@ class ConcatPlaceholder(BasePlaceholder): concat: Sequence[ValidCommandArgs] -class IfPresentPlaceholderStructure(pydantic.BaseModel): +class IfPresentPlaceholderStructure(BaseModel): """Class that holds structure for conditional cases. Attributes: - name: name of the input/output. + input_name: name of the input/output. then: If the input/output specified in name is present, the command-line argument will be replaced at run-time by the expanded value of then. @@ -124,12 +130,12 @@ class IfPresentPlaceholderStructure(pydantic.BaseModel): the command-line argument will be replaced at run-time by the expanded value of otherwise. """ - name: str + input_name: str = pydantic.Field(alias='inputName') then: Sequence[ValidCommandArgs] otherwise: Optional[Sequence[ValidCommandArgs]] = None - @pydantic.validator('otherwise') - def empty_sequence(cls, v): + @pydantic.validator('otherwise', allow_reuse=True) + def empty_otherwise_sequence(cls, v): if v == []: return None return v @@ -141,7 +147,13 @@ class IfPresentPlaceholder(BasePlaceholder): Attributes: if_present (ifPresent): holds structure for conditional cases. """ - if_present: IfPresentPlaceholderStructure + if_present: IfPresentPlaceholderStructure = pydantic.Field( + alias='ifPresent') + + +IfPresentPlaceholderStructure.update_forward_refs() +IfPresentPlaceholder.update_forward_refs() +ConcatPlaceholder.update_forward_refs() @dataclasses.dataclass @@ -161,7 +173,7 @@ class ResourceSpec: accelerator_count: Optional[int] = None -class ContainerSpec(pydantic.BaseModel): +class ContainerSpec(BaseModel): """Container implementation definition. Attributes: @@ -177,38 +189,20 @@ class ContainerSpec(pydantic.BaseModel): env: Optional[Mapping[str, ValidCommandArgs]] = None resources: Optional[ResourceSpec] = None - @pydantic.validator('commands', 'arguments') + @pydantic.validator('commands', 'arguments', allow_reuse=True) def empty_sequence(cls, v): if v == []: return None return v - @pydantic.validator('env') + @pydantic.validator('env', allow_reuse=True) def empty_map(cls, v): if v == {}: return None return v -@dataclasses.dataclass -class ImporterSpec: - """ImporterSpec definition. - - Attributes: - artifact_uri: The URI of the artifact. - type_schema: The type of the artifact. - reimport: Whether or not import an artifact regardless it has been - imported before. - metadata: Optional; the properties of the artifact. - """ - artifact_uri: str - type_schema: str - reimport: bool - metadata: Optional[Mapping[str, Any]] = None - - -@dataclasses.dataclass -class TaskSpec: +class TaskSpec(BaseModel): """The spec of a pipeline task. Attributes: @@ -238,8 +232,7 @@ class TaskSpec: iterator_item_input: Optional[str] = None -@dataclasses.dataclass -class DagSpec: +class DagSpec(BaseModel): """DAG(graph) implementation definition. Attributes: @@ -251,46 +244,65 @@ class DagSpec: outputs: Mapping[str, Any] -class SchemaVersion(str, enum.Enum): - V1 = 'v1' - V2 = 'v2' +class ImporterSpec(BaseModel): + """ImporterSpec definition. + + Attributes: + artifact_uri: The URI of the artifact. + type_schema: The type of the artifact. + reimport: Whether or not import an artifact regardless it has been + imported before. + metadata: Optional; the properties of the artifact. + """ + artifact_uri: str + type_schema: str + reimport: bool + metadata: Optional[Mapping[str, Any]] = None + + +class Implementation(BaseModel): + """Implementation definition. + + Attributes: + container: container implementation details. + graph: graph implementation details. + importer: importer implementation details. + """ + container: Optional[ContainerSpec] = None + graph: Optional[DagSpec] = None + importer: Optional[ImporterSpec] = None -class ComponentSpec(pydantic.BaseModel): +class ComponentSpec(BaseModel): """The definition of a component. Attributes: name: The name of the component. - implementation: The implementation of the component. Either an executor - (container, importer) or a DAG consists of other components. + description: Optional; the description of the component. inputs: Optional; the input definitions of the component. outputs: Optional; the output definitions of the component. - description: Optional; the description of the component. - annotations: Optional; the annotations of the component as key-value pairs. - labels: Optional; the labels of the component as key-value pairs. - schema_version: Internal field for tracking component version. + implementation: The implementation of the component. Either an executor + (container, importer) or a DAG consists of other components. """ - + # TODO(ji-yaqi): Update to OrderedDict for inputs and outputs once we drop + # Python 3.6 support name: str - implementation: Union[ContainerSpec, ImporterSpec, DagSpec] + description: Optional[str] = None inputs: Optional[Dict[str, InputSpec]] = None outputs: Optional[Dict[str, OutputSpec]] = None - description: Optional[str] = None - annotations: Optional[Mapping[str, str]] = None - labels: Optional[Mapping[str, str]] = None - schema_version: SchemaVersion = SchemaVersion.V2 + implementation: Implementation - @pydantic.validator('inputs', 'outputs', 'annotations', 'labels') + @pydantic.validator('inputs', 'outputs', allow_reuse=True) def empty_map(cls, v): if v == {}: return None return v - @pydantic.root_validator + @pydantic.root_validator(allow_reuse=True) def validate_placeholders(cls, values): - if values.get('implementation') is None or not isinstance( - values.get('implementation'), ContainerSpec): + if values.get('implementation').container is None: return values + containerSpec: ContainerSpec = values.get('implementation').container try: valid_inputs = values.get('inputs').keys() @@ -302,9 +314,8 @@ def validate_placeholders(cls, values): except AttributeError: valid_outputs = [] - for arg in itertools.chain( - (values.get('implementation').commands or []), - (values.get('implementation').arguments or [])): + for arg in itertools.chain((containerSpec.commands or []), + (containerSpec.arguments or [])): cls._check_valid_placeholder_reference(valid_inputs, valid_outputs, arg) @@ -328,28 +339,18 @@ def _check_valid_placeholder_reference(cls, valid_inputs: Sequence[str], instance. """ - if isinstance(arg, InputValuePlaceholder): - if arg.input_value not in valid_inputs: - raise ValueError( - f'Argument "{arg}" references non-existing input.') - elif isinstance(arg, InputPathPlaceholder): - if arg.input_path not in valid_inputs: + if isinstance( + arg, + (InputValuePlaceholder, InputPathPlaceholder, InputUriPlaceholder)): + if arg.input_name not in valid_inputs: raise ValueError( f'Argument "{arg}" references non-existing input.') - elif isinstance(arg, InputUriPlaceholder): - if arg.input_uri not in valid_inputs: - raise ValueError( - f'Argument "{arg}" references non-existing input.') - elif isinstance(arg, OutputPathPlaceholder): - if arg.output_path not in valid_outputs: - raise ValueError( - f'Argument "{arg}" references non-existing output.') - elif isinstance(arg, OutputUriPlaceholder): - if arg.output_uri not in valid_outputs: + elif isinstance(arg, (OutputPathPlaceholder, OutputUriPlaceholder)): + if arg.output_name not in valid_outputs: raise ValueError( f'Argument "{arg}" references non-existing output.') elif isinstance(arg, IfPresentPlaceholder): - if arg.if_present.name not in valid_inputs: + if arg.if_present.input_name not in valid_inputs: raise ValueError( f'Argument "{arg}" references non-existing input.') for placeholder in itertools.chain(arg.if_present.then, @@ -391,15 +392,15 @@ def _transform_arg(arg: Union[str, Dict[str, str]]) -> ValidCommandArgs: if isinstance(arg, str): return arg if 'inputValue' in arg: - return InputValuePlaceholder(input_value=arg['inputValue']) + return InputValuePlaceholder(input_name=arg['inputValue']) if 'inputPath' in arg: - return InputPathPlaceholder(input_path=arg['inputPath']) + return InputPathPlaceholder(input_name=arg['inputPath']) if 'inputUri' in arg: - return InputUriPlaceholder(input_uri=arg['inputUri']) + return InputUriPlaceholder(input_name=arg['inputUri']) if 'outputPath' in arg: - return OutputPathPlaceholder(output_path=arg['outputPath']) + return OutputPathPlaceholder(output_name=arg['outputPath']) if 'outputUri' in arg: - return OutputUriPlaceholder(output_uri=arg['outputUri']) + return OutputUriPlaceholder(output_name=arg['outputUri']) if 'if' in arg: if_placeholder_values = arg['if'] if_placeholder_values_then = list(if_placeholder_values['then']) @@ -412,7 +413,7 @@ def _transform_arg(arg: Union[str, Dict[str, str]]) -> ValidCommandArgs: IfPresentPlaceholderStructure.update_forward_refs() return IfPresentPlaceholder( if_present=IfPresentPlaceholderStructure( - name=if_placeholder_values['cond']['isPresent'], + input_name=if_placeholder_values['cond']['isPresent'], then=list( _transform_arg(val) for val in if_placeholder_values_then), @@ -446,7 +447,7 @@ def _transform_arg(arg: Union[str, Dict[str, str]]) -> ValidCommandArgs: return ComponentSpec( name=component_dict.get('name', 'name'), description=component_dict.get('description'), - implementation=container_spec, + implementation=Implementation(container=container_spec,), inputs={ spec['name']: InputSpec( type=spec.get('type', 'Artifact'), @@ -456,8 +457,7 @@ def _transform_arg(arg: Union[str, Dict[str, str]]) -> ValidCommandArgs: outputs={ spec['name']: OutputSpec(type=spec.get('type', 'Artifact')) for spec in component_dict.get('outputs', []) - }, - schema_version=SchemaVersion.V1) + }) def to_v1_component_spec(self) -> structures.ComponentSpec: """Converts to v1 ComponentSpec. @@ -467,22 +467,20 @@ def to_v1_component_spec(self) -> structures.ComponentSpec: Needed until downstream accept new ComponentSpec. """ - if isinstance(self.implementation, DagSpec): - raise NotImplementedError def _transform_arg(arg: ValidCommandArgs) -> Any: if isinstance(arg, str): return arg if isinstance(arg, InputValuePlaceholder): - return structures.InputValuePlaceholder(arg.input_value) + return structures.InputValuePlaceholder(arg.input_name) if isinstance(arg, InputPathPlaceholder): - return structures.InputPathPlaceholder(arg.input_path) + return structures.InputPathPlaceholder(arg.input_name) if isinstance(arg, InputUriPlaceholder): - return structures.InputUriPlaceholder(arg.input_uri) + return structures.InputUriPlaceholder(arg.input_name) if isinstance(arg, OutputPathPlaceholder): - return structures.OutputPathPlaceholder(arg.output_path) + return structures.OutputPathPlaceholder(arg.output_name) if isinstance(arg, OutputUriPlaceholder): - return structures.OutputUriPlaceholder(arg.output_uri) + return structures.OutputUriPlaceholder(arg.output_name) if isinstance(arg, IfPresentPlaceholder): return structures.IfPlaceholder(arg.if_present) if isinstance(arg, ConcatPlaceholder): @@ -508,18 +506,18 @@ def _transform_arg(arg: ValidCommandArgs) -> Any: ], implementation=structures.ContainerImplementation( container=structures.ContainerSpec( - image=self.implementation.image, + image=self.implementation.container.image, command=[ _transform_arg(cmd) - for cmd in self.implementation.commands or [] + for cmd in self.implementation.container.commands or [] ], args=[ - _transform_arg(arg) - for arg in self.implementation.arguments or [] + _transform_arg(arg) for arg in + self.implementation.container.arguments or [] ], env={ - name: _transform_arg(value) - for name, value in self.implementation.env or {} + name: _transform_arg(value) for name, value in + self.implementation.container.env or {} }, )), ) @@ -536,13 +534,12 @@ def load_from_component_yaml(cls, component_yaml: str) -> 'ComponentSpec': """ json_component = yaml.safe_load(component_yaml) - if 'schema_version' in json_component and json_component[ - 'schema_version'] == SchemaVersion.V2: + try: return ComponentSpec.parse_obj(json_component) - - v1_component = _components._load_component_spec_from_component_text( - component_yaml) - return cls.from_v1_component_spec(v1_component) + except pydantic.ValidationError: + v1_component = _components._load_component_spec_from_component_text( + component_yaml) + return cls.from_v1_component_spec(v1_component) def save_to_component_yaml(self, output_file: str) -> None: """Saves ComponentSpec into yaml file. @@ -551,7 +548,10 @@ def save_to_component_yaml(self, output_file: str) -> None: output_file: File path to store the component yaml. """ with open(output_file, 'a') as output_file: - json_component = self.json(exclude_none=True, exclude_unset=True) + json_component = self.json( + exclude_none=True, exclude_unset=True, by_alias=True) yaml_file = yaml.safe_dump( - json.loads(json_component), sort_keys=False) + json.loads(json_component), + default_flow_style=None, + sort_keys=False) output_file.write(yaml_file) diff --git a/sdk/python/kfp/v2/components/experimental/component_spec_test.py b/sdk/python/kfp/v2/components/experimental/component_spec_test.py index 0f62ff1b2e5..373033110af 100644 --- a/sdk/python/kfp/v2/components/experimental/component_spec_test.py +++ b/sdk/python/kfp/v2/components/experimental/component_spec_test.py @@ -23,7 +23,7 @@ from kfp.v2.components.experimental import component_spec V1_YAML_IF_PLACEHOLDER = textwrap.dedent("""\ - name: component_1 + name: component_if inputs: - {name: optional_input_1, type: String, optional: true} implementation: @@ -42,49 +42,42 @@ """) V2_YAML_IF_PLACEHOLDER = textwrap.dedent("""\ - name: component_1 - implementation: - image: alpine - arguments: - - if_present: - name: optional_input_1 - then: - - --arg1 - - input_value: optional_input_1 - otherwise: - - --arg2 - - default + name: component_if inputs: - optional_input_1: - type: String - schema_version: v2 + optional_input_1: {type: String} + implementation: + container: + image: alpine + arguments: + - ifPresent: + inputName: optional_input_1 + then: + - --arg1 + - {inputValue: optional_input_1} + otherwise: [--arg2, default] """) - -def v2_component_spec_if_placeholder( - schema_version: component_spec.SchemaVersion): - return component_spec.ComponentSpec( - name='component_1', - implementation=component_spec.ContainerSpec( +V2_COMPONENT_SPEC_IF_PLACEHOLDER = component_spec.ComponentSpec( + name='component_if', + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( image='alpine', arguments=[ component_spec.IfPresentPlaceholder( if_present=component_spec.IfPresentPlaceholderStructure( - name='optional_input_1', + input_name='optional_input_1', then=[ '--arg1', component_spec.InputValuePlaceholder( - input_value='optional_input_1'), + input_name='optional_input_1'), ], otherwise=[ '--arg2', 'default', ])) - ]), - inputs={'optional_input_1': component_spec.InputSpec(type='String')}, - schema_version=schema_version, - ) - + ])), + inputs={'optional_input_1': component_spec.InputSpec(type='String')}, +) V1_YAML_CONCAT_PLACEHOLDER = textwrap.dedent("""\ name: component_concat @@ -99,81 +92,70 @@ def v2_component_spec_if_placeholder( V2_YAML_CONCAT_PLACEHOLDER = textwrap.dedent("""\ name: component_concat - implementation: - image: alpine - arguments: - - concat: - - --arg1 - - input_value: input_prefix inputs: - input_prefix: - type: String - schema_version: v2 + input_prefix: {type: String} + implementation: + container: + image: alpine + arguments: + - concat: + - --arg1 + - {inputValue: input_prefix} """) - -def v2_component_spec_concat_placeholder( - schema_version: component_spec.SchemaVersion): - return component_spec.ComponentSpec( - name='component_concat', - implementation=component_spec.ContainerSpec( +V2_COMPONENT_SPEC_CONCAT_PLACEHOLDER = component_spec.ComponentSpec( + name='component_concat', + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( image='alpine', arguments=[ component_spec.ConcatPlaceholder(concat=[ '--arg1', component_spec.InputValuePlaceholder( - input_value='input_prefix'), + input_name='input_prefix'), ]) - ]), - inputs={'input_prefix': component_spec.InputSpec(type='String')}, - schema_version=schema_version, - ) - + ])), + inputs={'input_prefix': component_spec.InputSpec(type='String')}, +) V2_YAML_NESTED_PLACEHOLDER = textwrap.dedent("""\ - name: component_concat + name: component_nested + inputs: + input_prefix: {type: String} implementation: - image: alpine - arguments: - - concat: - - --arg1 - - if_present: - name: input_prefix - then: - - --arg1 - - input_value: input_prefix - otherwise: - - --arg2 - - default - - concat: + container: + image: alpine + arguments: + - concat: + - --arg1 + - ifPresent: + inputName: input_prefix + then: - --arg1 - - input_value: input_prefix - inputs: - input_prefix: - type: String - schema_version: v2 + - {inputValue: input_prefix} + otherwise: + - --arg2 + - default + - concat: + - --arg1 + - {inputValue: input_prefix} """) - -def v2_component_spec_nested_placeholder( - schema_version: component_spec.SchemaVersion): - component_spec.ConcatPlaceholder.update_forward_refs() - component_spec.IfPresentPlaceholderStructure.update_forward_refs() - - return component_spec.ComponentSpec( - name='component_concat', - implementation=component_spec.ContainerSpec( +V2_COMPONENT_SPEC_NESTED_PLACEHOLDER = component_spec.ComponentSpec( + name='component_nested', + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( image='alpine', arguments=[ component_spec.ConcatPlaceholder(concat=[ '--arg1', component_spec.IfPresentPlaceholder( if_present=component_spec.IfPresentPlaceholderStructure( - name='input_prefix', + input_name='input_prefix', then=[ '--arg1', component_spec.InputValuePlaceholder( - input_value='input_prefix'), + input_name='input_prefix'), ], otherwise=[ '--arg2', @@ -181,14 +163,13 @@ def v2_component_spec_nested_placeholder( component_spec.ConcatPlaceholder(concat=[ '--arg1', component_spec.InputValuePlaceholder( - input_value='input_prefix'), + input_name='input_prefix'), ]), ])), ]) - ]), - inputs={'input_prefix': component_spec.InputSpec(type='String')}, - schema_version=schema_version, - ) + ])), + inputs={'input_prefix': component_spec.InputSpec(type='String')}, +) class ComponentSpecTest(parameterized.TestCase): @@ -196,44 +177,46 @@ class ComponentSpecTest(parameterized.TestCase): def test_component_spec_with_placeholder_referencing_nonexisting_input_output( self): with self.assertRaisesRegex( - pydantic.ValidationError, 'Argument "input_value=\'input000\'" ' + pydantic.ValidationError, 'Argument "input_name=\'input000\'" ' 'references non-existing input.'): component_spec.ComponentSpec( name='component_1', - implementation=component_spec.ContainerSpec( - image='alpine', - commands=[ - 'sh', - '-c', - 'set -ex\necho "$0" > "$1"', - component_spec.InputValuePlaceholder( - input_value='input000'), - component_spec.OutputPathPlaceholder( - output_path='output1'), - ], - ), + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( + image='alpine', + commands=[ + 'sh', + '-c', + 'set -ex\necho "$0" > "$1"', + component_spec.InputValuePlaceholder( + input_name='input000'), + component_spec.OutputPathPlaceholder( + output_name='output1'), + ], + )), inputs={'input1': component_spec.InputSpec(type='String')}, outputs={'output1': component_spec.OutputSpec(type='String')}, ) with self.assertRaisesRegex( pydantic.ValidationError, - 'Argument "output_path=\'output000\'" ' + 'Argument "output_name=\'output000\'" ' 'references non-existing output.'): component_spec.ComponentSpec( name='component_1', - implementation=component_spec.ContainerSpec( - image='alpine', - commands=[ - 'sh', - '-c', - 'set -ex\necho "$0" > "$1"', - component_spec.InputValuePlaceholder( - input_value='input1'), - component_spec.OutputPathPlaceholder( - output_path='output000'), - ], - ), + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( + image='alpine', + commands=[ + 'sh', + '-c', + 'set -ex\necho "$0" > "$1"', + component_spec.InputValuePlaceholder( + input_name='input1'), + component_spec.OutputPathPlaceholder( + output_name='output000'), + ], + )), inputs={'input1': component_spec.InputSpec(type='String')}, outputs={'output1': component_spec.OutputSpec(type='String')}, ) @@ -242,47 +225,45 @@ def test_simple_component_spec_save_to_component_yaml(self): open_mock = mock.mock_open() expected_yaml = textwrap.dedent("""\ name: component_1 - implementation: - image: alpine - commands: - - sh - - -c - - 'set -ex - - echo "$0" > "$1"' - - input_value: input1 - - output_path: output1 inputs: - input1: - type: String + input1: {type: String} outputs: - output1: - type: String - schema_version: v2 + output1: {type: String} + implementation: + container: + image: alpine + commands: + - sh + - -c + - 'set -ex + + echo "$0" > "$1"' + - {inputValue: input1} + - {outputPath: output1} """) with mock.patch("builtins.open", open_mock, create=True): component_spec.ComponentSpec( name='component_1', - implementation=component_spec.ContainerSpec( - image='alpine', - commands=[ - 'sh', - '-c', - 'set -ex\necho "$0" > "$1"', - component_spec.InputValuePlaceholder( - input_value='input1'), - component_spec.OutputPathPlaceholder( - output_path='output1'), - ], - ), + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( + image='alpine', + commands=[ + 'sh', + '-c', + 'set -ex\necho "$0" > "$1"', + component_spec.InputValuePlaceholder( + input_name='input1'), + component_spec.OutputPathPlaceholder( + output_name='output1'), + ], + )), inputs={ 'input1': component_spec.InputSpec(type='String') }, outputs={ 'output1': component_spec.OutputSpec(type='String') }, - schema_version=component_spec.SchemaVersion.V2, ).save_to_component_yaml('test_save_file.txt') open_mock.assert_called_with('test_save_file.txt', 'a') @@ -290,25 +271,16 @@ def test_simple_component_spec_save_to_component_yaml(self): @parameterized.parameters( { - 'expected_yaml': - V2_YAML_IF_PLACEHOLDER, - 'component': - v2_component_spec_if_placeholder( - schema_version=component_spec.SchemaVersion.V2) + 'expected_yaml': V2_YAML_IF_PLACEHOLDER, + 'component': V2_COMPONENT_SPEC_IF_PLACEHOLDER }, { - 'expected_yaml': - V2_YAML_CONCAT_PLACEHOLDER, - 'component': - v2_component_spec_concat_placeholder( - schema_version=component_spec.SchemaVersion.V2) + 'expected_yaml': V2_YAML_CONCAT_PLACEHOLDER, + 'component': V2_COMPONENT_SPEC_CONCAT_PLACEHOLDER }, { - 'expected_yaml': - V2_YAML_NESTED_PLACEHOLDER, - 'component': - v2_component_spec_nested_placeholder( - schema_version=component_spec.SchemaVersion.V2) + 'expected_yaml': V2_YAML_NESTED_PLACEHOLDER, + 'component': V2_COMPONENT_SPEC_NESTED_PLACEHOLDER }, ) def test_component_spec_placeholder_save_to_component_yaml( @@ -324,23 +296,23 @@ def test_component_spec_placeholder_save_to_component_yaml( def test_simple_component_spec_load_from_v2_component_yaml(self): component_yaml_v2 = textwrap.dedent("""\ name: component_1 - implementation: - image: alpine - commands: - - sh - - -c - - 'set -ex - - echo "$0" > "$1"' - - input_value: input1 - - output_path: output1 inputs: input1: type: String outputs: output1: type: String - schema_version: v2 + implementation: + container: + image: alpine + commands: + - sh + - -c + - 'set -ex + + echo "$0" > "$1"' + - inputValue: input1 + - outputPath: output1 """) generated_spec = component_spec.ComponentSpec.load_from_component_yaml( @@ -348,56 +320,43 @@ def test_simple_component_spec_load_from_v2_component_yaml(self): expected_spec = component_spec.ComponentSpec( name='component_1', - implementation=component_spec.ContainerSpec( - image='alpine', - commands=[ - 'sh', - '-c', - 'set -ex\necho "$0" > "$1"', - component_spec.InputValuePlaceholder(input_value='input1'), - component_spec.OutputPathPlaceholder(output_path='output1'), - ], - ), + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( + image='alpine', + commands=[ + 'sh', + '-c', + 'set -ex\necho "$0" > "$1"', + component_spec.InputValuePlaceholder( + input_name='input1'), + component_spec.OutputPathPlaceholder( + output_name='output1'), + ], + )), inputs={'input1': component_spec.InputSpec(type='String')}, - outputs={'output1': component_spec.OutputSpec(type='String')}, - schema_version=component_spec.SchemaVersion.V2) + outputs={'output1': component_spec.OutputSpec(type='String')}) self.assertEqual(generated_spec, expected_spec) @parameterized.parameters( { - 'yaml': - V1_YAML_IF_PLACEHOLDER, - 'expected_component': - v2_component_spec_if_placeholder( - schema_version=component_spec.SchemaVersion.V1) + 'yaml': V1_YAML_IF_PLACEHOLDER, + 'expected_component': V2_COMPONENT_SPEC_IF_PLACEHOLDER }, { - 'yaml': - V2_YAML_IF_PLACEHOLDER, - 'expected_component': - v2_component_spec_if_placeholder( - schema_version=component_spec.SchemaVersion.V2) + 'yaml': V2_YAML_IF_PLACEHOLDER, + 'expected_component': V2_COMPONENT_SPEC_IF_PLACEHOLDER }, { - 'yaml': - V1_YAML_CONCAT_PLACEHOLDER, - 'expected_component': - v2_component_spec_concat_placeholder( - schema_version=component_spec.SchemaVersion.V1) + 'yaml': V1_YAML_CONCAT_PLACEHOLDER, + 'expected_component': V2_COMPONENT_SPEC_CONCAT_PLACEHOLDER }, { - 'yaml': - V2_YAML_CONCAT_PLACEHOLDER, - 'expected_component': - v2_component_spec_concat_placeholder( - schema_version=component_spec.SchemaVersion.V2) + 'yaml': V2_YAML_CONCAT_PLACEHOLDER, + 'expected_component': V2_COMPONENT_SPEC_CONCAT_PLACEHOLDER }, { - 'yaml': - V2_YAML_NESTED_PLACEHOLDER, - 'expected_component': - v2_component_spec_nested_placeholder( - schema_version=component_spec.SchemaVersion.V2) + 'yaml': V2_YAML_NESTED_PLACEHOLDER, + 'expected_component': V2_COMPONENT_SPEC_NESTED_PLACEHOLDER }, ) def test_component_spec_placeholder_load_from_v2_component_yaml( @@ -437,26 +396,27 @@ def test_component_spec_load_from_v1_component_yaml(self): expected_spec = component_spec.ComponentSpec( name='Component with 2 inputs and 2 outputs', - implementation=component_spec.ContainerSpec( - image='busybox', - commands=[ - 'sh', - '-c', - (' mkdir -p $(dirname "$2") mkdir -p $(dirname "$3") ' - 'echo "$0" > "$2" cp "$1" "$3" '), - ], - arguments=[ - component_spec.InputValuePlaceholder( - input_value='Input parameter'), - component_spec.InputPathPlaceholder( - input_path='Input artifact'), - component_spec.OutputPathPlaceholder( - output_path='Output 1'), - component_spec.OutputPathPlaceholder( - output_path='Output 2'), - ], - env={}, - ), + implementation=component_spec.Implementation( + container=component_spec.ContainerSpec( + image='busybox', + commands=[ + 'sh', + '-c', + (' mkdir -p $(dirname "$2") mkdir -p $(dirname "$3") ' + 'echo "$0" > "$2" cp "$1" "$3" '), + ], + arguments=[ + component_spec.InputValuePlaceholder( + input_name='Input parameter'), + component_spec.InputPathPlaceholder( + input_name='Input artifact'), + component_spec.OutputPathPlaceholder( + output_name='Output 1'), + component_spec.OutputPathPlaceholder( + output_name='Output 2'), + ], + env={}, + )), inputs={ 'Input parameter': component_spec.InputSpec(type='Artifact'), 'Input artifact': component_spec.InputSpec(type='Artifact') @@ -464,8 +424,7 @@ def test_component_spec_load_from_v1_component_yaml(self): outputs={ 'Output 1': component_spec.OutputSpec(type='Artifact'), 'Output 2': component_spec.OutputSpec(type='Artifact'), - }, - schema_version=component_spec.SchemaVersion.V1) + }) self.assertEqual(generated_spec, expected_spec)