fix(sdk): fix load_test

kubeflow · Nov 29, 2021 · a8ebc5e · a8ebc5e
1 parent cd20373
commit a8ebc5e
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 76 deletions.
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
@@ -107,9 +107,8 @@ 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):

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
@@ -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"
 }
diff --git a/sdk/python/kfp/v2/components/structures.py b/sdk/python/kfp/v2/components/structures.py
@@ -374,8 +374,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)
@@ -574,7 +574,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
@@ -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,