Fix infinite recursion problem on artifact_type custom_path property.

Signed-off-by: agoins <[email protected]>

Author: alyssacgoins

sdk/python/kfp/dsl/executor.py

@@ -102,7 +102,6 @@ def assign_input_and_output_artifacts(self) -> None:
                     self.func,
                 )
                 self.output_artifacts[name] = output_artifact
-                #todo: I think this is where we are creating artifact files in local container storage.
                 makedirs_recursively(output_artifact.path)
 
     def make_artifact(
@@ -305,7 +304,6 @@ def write_executor_output(self,
             write_file = cluster_spec['task']['type'] in CHIEF_NODE_LABELS
 
         if write_file:
-            #todo. this method is called by execute(), and here is where it is dumping the artifact output locally.
             makedirs_recursively(self.executor_output_path)
             with open(self.executor_output_path, 'w') as f:
                 f.write(json.dumps(self.excutor_output))

sdk/python/kfp/dsl/executor_test.py

@@ -616,7 +616,7 @@ def test_artifact_output1(self):
                   "schemaTitle": "system.Artifact"
                 },
                 "uri": "gs://some-bucket/output",
-                'custom_path':""
+                "custom_path": ""
               }
             ]
           }
@@ -648,7 +648,9 @@ def test_func(first: str, second: str, output: Output[Artifact]) -> str:
                             'name':
                                 'projects/123/locations/us-central1/metadataStores/default/artifacts/123',
                             'uri':
-                                'gs://some-bucket/output'
+                                'gs://some-bucket/output',
+                            'custom_path':
+                                ''
                         }]
                     }
                 },
@@ -1061,7 +1063,7 @@ def test_func(
                                 '',
                             'uri':
                                 'gs://mlpipeline/v2/artifacts/my-test-pipeline-beta/b2b0cdee-b15c-48ff-b8bc-a394ae46c854/train/model',
-                            'custom_path':
+                            "custom_path":
                                 '',
                             'metadata': {
                                 'accuracy': 0.9

sdk/python/kfp/dsl/types/artifact_types.py

@@ -78,13 +78,12 @@ def __init__(self,
                  name: Optional[str] = None,
                  uri: Optional[str] = None,
                  metadata: Optional[Dict] = None) -> None:
-        """Initializes the Artifact with the given name, URI and metadata."""
+        """Initializes the Artifact with the given name, URI, metadata, and blank custom path."""
         self.uri = uri or ''
         self.name = name or ''
         self.metadata = metadata or {}
-        self.custom_path = None
+        self._custom_path = ''
 
-    #todo: this is designed to return custom path if set, and otherwise return local container storage path.s
     @property
     def path(self) -> str:
         return self._get_path()
@@ -93,10 +92,9 @@ def path(self) -> str:
     def path(self, path: str) -> None:
         self._set_custom_path(path)
 
-    #todo: here the path property returns the custom path if set --> otherwise, returns the regular path/
     def _get_path(self) -> Optional[str]:
-        if self.custom_path is not None:
-            return self.custom_path
+        if self.custom_path is not '':
+            return self._get_custom_path()
         if self.uri.startswith(RemotePrefix.GCS.value):
             return _GCS_LOCAL_MOUNT_PREFIX + self.uri[len(RemotePrefix.GCS.value
                                                          ):]
@@ -115,23 +113,19 @@ def _get_path(self) -> Optional[str]:
 
     @property
     def custom_path(self) -> str:
-        return self._get_custom_path()
+        return self._custom_path
 
-    def _get_custom_path(self) -> Optional[str]:
-        if self.custom_path is not None:
-            return self.custom_path
-        return None
+    def _get_custom_path(self) -> str:
+        return self._custom_path
 
-    #todo: this is the internal function to set the path.
     def _set_path(self, path: str) -> None:
         self.uri = convert_local_path_to_remote_path(path)
 
-    #todo: what differentiates this from setting the path regularly?
-    def _set_custom_path(self, custom_path: str) -> None:
-        self.custom_path = custom_path
+    def _set_custom_path(self, value: str) -> None:
+        self._custom_path = value
 
     @custom_path.setter
-    def custom_path(self, value):
+    def custom_path(self, value: str):
         self._custom_path = value