diff --git a/sdk/RELEASE.md b/sdk/RELEASE.md index 5109fdf6bcf..a5ec828971c 100644 --- a/sdk/RELEASE.md +++ b/sdk/RELEASE.md @@ -30,6 +30,8 @@ * Depends on `google-auth>=1.6.1,<3` [\#6939](https://github.com/kubeflow/pipelines/pull/6939) * Change otherwise to else in yaml [\#6952](https://github.com/kubeflow/pipelines/pull/6952) * Avoid pydantic bug on Union type [\#6957](https://github.com/kubeflow/pipelines/pull/6957) +* Fix bug for if and concat placeholders [\#6978](https://github.com/kubeflow/pipelines/pull/6978) + ## Documentation Updates diff --git a/sdk/python/kfp/v2/compiler_cli_tests/compiler_cli_tests.py b/sdk/python/kfp/v2/compiler_cli_tests/compiler_cli_tests.py index fdae3cdf2a1..5c90df01f06 100644 --- a/sdk/python/kfp/v2/compiler_cli_tests/compiler_cli_tests.py +++ b/sdk/python/kfp/v2/compiler_cli_tests/compiler_cli_tests.py @@ -107,15 +107,13 @@ def test_pipeline_with_importer(self): # def test_pipeline_with_ontology(self): # self._test_compile_py_to_json('pipeline_with_ontology') - # TODO: re-enable the test, debug load_component_from_file error - # def test_pipeline_with_if_placeholder(self): - # self._test_compile_py_to_json('pipeline_with_if_placeholder') + def test_pipeline_with_if_placeholder(self): + self._test_compile_py_to_json('pipeline_with_if_placeholder') - # TODO: re-enable the test, debug load_component_from_file error - # def test_pipeline_with_concat_placeholder(self): - # self._test_compile_py_to_json('pipeline_with_concat_placeholder') + def test_pipeline_with_concat_placeholder(self): + self._test_compile_py_to_json('pipeline_with_concat_placeholder') - # TODO: re-enable the test, debug load_component_from_file error + # TODO: re-enable the test, debug add_node_selector_constraint error # def test_pipeline_with_resource_spec(self): # self._test_compile_py_to_json('pipeline_with_resource_spec') diff --git a/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_concat_placeholder.json b/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_concat_placeholder.json index 4eb955c2f4b..780ba6bc193 100644 --- a/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_concat_placeholder.json +++ b/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_concat_placeholder.json @@ -1,63 +1,59 @@ { - "pipelineSpec": { - "components": { - "comp-component-with-concat-placeholder": { - "executorLabel": "exec-component-with-concat-placeholder", - "inputDefinitions": { - "parameters": { - "input_prefix": { - "parameterType": "STRING" - } + "components": { + "comp-component-with-concat-placeholder": { + "executorLabel": "exec-component-with-concat-placeholder", + "inputDefinitions": { + "parameters": { + "input_prefix": { + "parameterType": "STRING" } } } - }, - "deploymentSpec": { - "executors": { - "exec-component-with-concat-placeholder": { - "container": { - "args": [ - "--arg0", - "{{$.inputs.parameters['input_prefix']}}some value" - ], - "image": "gcr.io/my-project/my-image" - } + } + }, + "defaultPipelineRoot": "dummy_root", + "deploymentSpec": { + "executors": { + "exec-component-with-concat-placeholder": { + "container": { + "args": [ + "--arg0", + "{{$.inputs.parameters['input_prefix']}}some value" + ], + "image": "gcr.io/my-project/my-image" } } - }, - "pipelineInfo": { - "name": "one-step-pipeline-with-concat-placeholder" - }, - "root": { - "dag": { - "tasks": { - "component-with-concat-placeholder": { - "cachingOptions": { - "enableCache": true - }, - "componentRef": { - "name": "comp-component-with-concat-placeholder" - }, - "inputs": { - "parameters": { - "input_prefix": { - "runtimeValue": { - "constant": "some prefix:" - } + } + }, + "pipelineInfo": { + "name": "one-step-pipeline-with-concat-placeholder" + }, + "root": { + "dag": { + "tasks": { + "component-with-concat-placeholder": { + "cachingOptions": { + "enableCache": true + }, + "componentRef": { + "name": "comp-component-with-concat-placeholder" + }, + "inputs": { + "parameters": { + "input_prefix": { + "runtimeValue": { + "constant": "some prefix:" } } - }, - "taskInfo": { - "name": "component-with-concat-placeholder" } + }, + "taskInfo": { + "name": "component-with-concat-placeholder" } } } - }, - "schemaVersion": "2.1.0", - "sdkVersion": "kfp-1.8.6" + } }, - "runtimeConfig": { - "gcsOutputDirectory": "dummy_root" - } + "schemaVersion": "2.1.0", + "sdkVersion": "kfp-1.8.9" } \ No newline at end of file diff --git a/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_if_placeholder.json b/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_if_placeholder.json index a3e0df000bb..32665fd1e22 100644 --- a/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_if_placeholder.json +++ b/sdk/python/kfp/v2/compiler_cli_tests/test_data/pipeline_with_if_placeholder.json @@ -1,84 +1,80 @@ { - "pipelineSpec": { - "components": { - "comp-component-with-optional-inputs": { - "executorLabel": "exec-component-with-optional-inputs", - "inputDefinitions": { - "parameters": { - "optional_input_1": { - "parameterType": "STRING" - }, - "required_input": { - "parameterType": "STRING" - } + "components": { + "comp-component-with-optional-inputs": { + "executorLabel": "exec-component-with-optional-inputs", + "inputDefinitions": { + "parameters": { + "optional_input_1": { + "parameterType": "STRING" + }, + "required_input": { + "parameterType": "STRING" } } } - }, - "deploymentSpec": { - "executors": { - "exec-component-with-optional-inputs": { - "container": { - "args": [ - "--arg0", - "{{$.inputs.parameters['required_input']}}", - "--arg1", - "{{$.inputs.parameters['optional_input_1']}}", - "--arg3", - "default value" - ], - "image": "gcr.io/my-project/my-image" - } + } + }, + "defaultPipelineRoot": "dummy_root", + "deploymentSpec": { + "executors": { + "exec-component-with-optional-inputs": { + "container": { + "args": [ + "--arg0", + "{{$.inputs.parameters['required_input']}}", + "--arg1", + "{{$.inputs.parameters['optional_input_1']}}", + "--arg3", + "default value" + ], + "image": "gcr.io/my-project/my-image" } } - }, - "pipelineInfo": { - "name": "one-step-pipeline-with-if-placeholder" - }, - "root": { - "dag": { - "tasks": { - "component-with-optional-inputs": { - "cachingOptions": { - "enableCache": true - }, - "componentRef": { - "name": "comp-component-with-optional-inputs" - }, - "inputs": { - "parameters": { - "optional_input_1": { - "componentInputParameter": "input1" - }, - "required_input": { - "componentInputParameter": "input0" - } + } + }, + "pipelineInfo": { + "name": "one-step-pipeline-with-if-placeholder" + }, + "root": { + "dag": { + "tasks": { + "component-with-optional-inputs": { + "cachingOptions": { + "enableCache": true + }, + "componentRef": { + "name": "comp-component-with-optional-inputs" + }, + "inputs": { + "parameters": { + "optional_input_1": { + "componentInputParameter": "input1" + }, + "required_input": { + "componentInputParameter": "input0" } - }, - "taskInfo": { - "name": "component-with-optional-inputs" } - } - } - }, - "inputDefinitions": { - "parameters": { - "input0": { - "parameterType": "STRING" - }, - "input1": { - "parameterType": "STRING" }, - "input2": { - "parameterType": "STRING" + "taskInfo": { + "name": "component-with-optional-inputs" } } } }, - "schemaVersion": "2.1.0", - "sdkVersion": "kfp-1.8.6" + "inputDefinitions": { + "parameters": { + "input0": { + "parameterType": "STRING" + }, + "input1": { + "parameterType": "STRING" + }, + "input2": { + "parameterType": "STRING" + } + } + } }, - "runtimeConfig": { - "gcsOutputDirectory": "dummy_root" - } + "schemaVersion": "2.1.0", + "sdkVersion": "kfp-1.8.9" } \ No newline at end of file diff --git a/sdk/python/kfp/v2/components/structures.py b/sdk/python/kfp/v2/components/structures.py index 47dbe70ea77..15dc35cbb2e 100644 --- a/sdk/python/kfp/v2/components/structures.py +++ b/sdk/python/kfp/v2/components/structures.py @@ -373,8 +373,8 @@ def _check_valid_placeholder_reference(cls, valid_inputs: Sequence[str], if arg.if_structure.input_name not in valid_inputs: raise ValueError( f'Argument "{arg}" references non-existing input.') - for placeholder in itertools.chain(arg.if_structure.then, - arg.if_structure.otherwise): + for placeholder in itertools.chain(arg.if_structure.then or [], + arg.if_structure.otherwise or []): cls._check_valid_placeholder_reference(valid_inputs, valid_outputs, placeholder) @@ -573,7 +573,6 @@ def load_from_component_yaml(cls, component_yaml: str) -> 'ComponentSpec': Returns: Component spec in the form of V2 ComponentSpec. """ - json_component = yaml.safe_load(component_yaml) try: return ComponentSpec.parse_obj(json_component) diff --git a/sdk/python/kfp/v2/components/yaml_component.py b/sdk/python/kfp/v2/components/yaml_component.py index bbe20e4ae3c..1a0e009b8f8 100644 --- a/sdk/python/kfp/v2/components/yaml_component.py +++ b/sdk/python/kfp/v2/components/yaml_component.py @@ -43,7 +43,7 @@ def load_component_from_file(file_path: str) -> base_component.BaseComponent: file_path: A string containing path to the YAML file. """ with open(file_path, 'rb') as component_stream: - return load_component_from_text(component_stream) + return load_component_from_text(component_stream.read()) def load_component_from_url(url: str,