fix(sdk): fix upload_pipeline when no pipeline name is provided

kubeflow · Jan 31, 2023 · 2d66b52 · 2d66b52
1 parent c79d35f
commit 2d66b52
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 3 deletions.
diff --git a/sdk/RELEASE.md b/sdk/RELEASE.md
@@ -7,6 +7,7 @@
 ## Deprecations
 
 ## Bug fixes and other changes
+* Fix upload_pipeline method on client when no name is provided
 
 ## Documentation updates
 # 2.0.0-beta.11

diff --git a/sdk/python/kfp/client/client.py b/sdk/python/kfp/client/client.py
@@ -1372,9 +1372,8 @@ def upload_pipeline(
             ``ApiPipeline`` object.
         """
         if pipeline_name is None:
-            pipeline_name = os.path.splitext(
-                os.path.basename('something/file.txt'))[0]
-
+            pipeline_yaml = self._extract_pipeline_yaml(pipeline_package_path)
+            pipeline_name = pipeline_yaml['pipelineInfo']['name']
         validate_pipeline_resource_name(pipeline_name)
         response = self._upload_api.upload_pipeline(
             pipeline_package_path, name=pipeline_name, description=description)

diff --git a/sdk/python/kfp/client/client_test.py b/sdk/python/kfp/client/client_test.py
@@ -284,6 +284,45 @@ def test_get_kfp_healthz_should_raise_error(self, mock_get_kfp_healthz):
             self.client.get_kfp_healthz(sleep_duration=0)
             mock_get_kfp_healthz.assert_called()
 
+    def test_upload_pipeline_without_name(self):
+
+        @component
+        def return_bool(boolean: bool) -> bool:
+            return boolean
+
+        @pipeline(name='test-upload-without-name', description='description')
+        def pipeline_test_upload_without_name(boolean: bool = True):
+            return_bool(boolean=boolean)
+
+        with patch.object(self.client._upload_api,
+                          'upload_pipeline') as mock_upload_pipeline:
+            with patch.object(self.client, '_is_ipython', return_value=False):
+                with tempfile.TemporaryDirectory() as tmp_path:
+                    pipeline_test_path = os.path.join(tmp_path, 'test.yaml')
+                    Compiler().compile(
+                        pipeline_func=pipeline_test_upload_without_name,
+                        package_path=pipeline_test_path)
+                    self.client.upload_pipeline(
+                        pipeline_package_path=pipeline_test_path,
+                        description='description')
+                    mock_upload_pipeline.assert_called_once_with(
+                        pipeline_test_path,
+                        name='test-upload-without-name',
+                        description='description')
+
+    def test_upload_pipeline_with_name(self):
+        with patch.object(self.client._upload_api,
+                          'upload_pipeline') as mock_upload_pipeline:
+            with patch.object(self.client, '_is_ipython', return_value=False):
+                self.client.upload_pipeline(
+                    pipeline_package_path='fake.yaml',
+                    pipeline_name='overwritten-name',
+                    description='description')
+                mock_upload_pipeline.assert_called_once_with(
+                    'fake.yaml',
+                    name='overwritten-name',
+                    description='description')
+
 
 if __name__ == '__main__':
     unittest.main()