Skip to content

Commit 2120dcd

Browse files
alyssacgoinsalyssacgoins
committed
update python formatting.
Signed-off-by: agoins <[email protected]>
1 parent 4aa1556 commit 2120dcd

File tree

6 files changed

+119
-28
lines changed

6 files changed

+119
-28
lines changed

‎backend/src/v2/component/launcher_v2.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ func getExecutorOutputFile(path string) (*pipelinespec.ExecutorOutput, error) {
910910

911911
func LocalPathForURI(uri string) (string, error) {
912912
if strings.HasPrefix(uri, "gs://") {
913-
return "/gcs/" + strings.TrimPrefix(uri, "gs://"), nil
913+
return "/home/agoins/gcs/" + strings.TrimPrefix(uri, "gs://"), nil
914914
}
915915
if strings.HasPrefix(uri, "minio://") {
916916
return "/minio/" + strings.TrimPrefix(uri, "minio://"), nil
@@ -938,14 +938,16 @@ func prepareOutputFolders(executorInput *pipelinespec.ExecutorInput) error {
938938
}
939939

940940
for _, outputArtifact := range artifactList.Artifacts {
941+
// If custom path is set, do not create local directory for output artifact.
942+
if outputArtifact.CustomPath == nil {
943+
localPath, err := LocalPathForURI(outputArtifact.Uri)
944+
if err != nil {
945+
return fmt.Errorf("failed to generate local storage path for output artifact %q: %w", name, err)
946+
}
941947

942-
localPath, err := LocalPathForURI(outputArtifact.Uri)
943-
if err != nil {
944-
return fmt.Errorf("failed to generate local storage path for output artifact %q: %w", name, err)
945-
}
946-
947-
if err := os.MkdirAll(filepath.Dir(localPath), 0755); err != nil {
948-
return fmt.Errorf("unable to create directory %q for output artifact %q: %w", filepath.Dir(localPath), name, err)
948+
if err := os.MkdirAll(filepath.Dir(localPath), 0755); err != nil {
949+
return fmt.Errorf("unable to create directory %q for output artifact %q: %w", filepath.Dir(localPath), name, err)
950+
}
949951
}
950952
}
951953
}

‎backend/src/v2/component/launcher_v2_test.go‎

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,17 @@ var addNumbersComponent = &pipelinespec.ComponentSpec{
5151
}
5252

5353
// Tests that launcher correctly executes the user component and successfully writes output parameters to file.
54-
func Test_executeV2_Parameters(t *testing.T) {
55-
customLocalPath := "/tmp/custom/artifact.txt"
54+
func Test_executeV2_Artifacts(t *testing.T) {
55+
uri := "gs:///tmp/custom/artifact.txt"
56+
custom := "mem://test-bucket/pipeline-root/temp.txt"
5657
tests := []struct {
5758
name string
5859
executorInput *pipelinespec.ExecutorInput
5960
executorArgs []string
6061
wantErr bool
6162
}{
6263
{
63-
"happy pass",
64+
"custom path",
6465
&pipelinespec.ExecutorInput{
6566
Inputs: &pipelinespec.ExecutorInput_Inputs{
6667
ParameterValues: map[string]*structpb.Value{"a": structpb.NewNumberValue(1), "b": structpb.NewNumberValue(2)},
@@ -70,13 +71,94 @@ func Test_executeV2_Parameters(t *testing.T) {
7071
"dataset": {
7172
Artifacts: []*pipelinespec.RuntimeArtifact{
7273
{
73-
Uri: customLocalPath,
74+
Uri: uri,
75+
CustomPath: &custom,
76+
Type: &pipelinespec.ArtifactTypeSchema{
77+
Kind: &pipelinespec.ArtifactTypeSchema_InstanceSchema{InstanceSchema: "title: kfp.Model\ntype: object\nproperties:\n framework:\n type: string\n framework_version:\n type: string\n"},
78+
},
79+
},
80+
},
81+
},
82+
},
83+
},
84+
},
85+
[]string{"-c", "echo \"{{$.inputs.parameters['content']}}\" > {{$.outputs.artifacts['data'].path}}"},
86+
false,
87+
},
88+
{
89+
"default path",
90+
&pipelinespec.ExecutorInput{
91+
Inputs: &pipelinespec.ExecutorInput_Inputs{
92+
ParameterValues: map[string]*structpb.Value{"b": structpb.NewNumberValue(2)},
93+
},
94+
Outputs: &pipelinespec.ExecutorInput_Outputs{
95+
Artifacts: map[string]*pipelinespec.ArtifactList{
96+
"dataset": {
97+
Artifacts: []*pipelinespec.RuntimeArtifact{
98+
{
99+
Uri: uri,
100+
CustomPath: &custom,
101+
Type: &pipelinespec.ArtifactTypeSchema{
102+
Kind: &pipelinespec.ArtifactTypeSchema_InstanceSchema{InstanceSchema: "title: kfp.Model\ntype: object\nproperties:\n framework:\n type: string\n framework_version:\n type: string\n"},
103+
},
74104
},
75105
},
76106
},
77107
},
78108
},
79109
},
110+
[]string{"-c", "echo 'ok' > {{$.outputs.artifacts['artifact'].path}}"},
111+
false,
112+
},
113+
}
114+
115+
for _, test := range tests {
116+
t.Run(test.name, func(t *testing.T) {
117+
fakeKubernetesClientset := &fake.Clientset{}
118+
fakeMetadataClient := metadata.NewFakeClient()
119+
bucket, err := blob.OpenBucket(context.Background(), "mem://test-bucket")
120+
assert.Nil(t, err)
121+
bucketConfig, err := objectstore.ParseBucketConfig("mem://test-bucket/pipeline-root/", nil)
122+
assert.Nil(t, err)
123+
_, _, err = executeV2(
124+
context.Background(),
125+
test.executorInput,
126+
addNumbersComponent,
127+
"sh",
128+
test.executorArgs,
129+
bucket,
130+
bucketConfig,
131+
fakeMetadataClient,
132+
"namespace",
133+
fakeKubernetesClientset,
134+
"false",
135+
)
136+
//
137+
//if test.wantErr {
138+
// assert.NotNil(t, err)
139+
//} else {
140+
// assert.Nil(t, err)
141+
//}
142+
assert.NotEmpty(t, bucket)
143+
})
144+
}
145+
}
146+
147+
// Tests that launcher correctly executes the user component and successfully writes output parameters to file.
148+
func Test_executeV2_Parameters(t *testing.T) {
149+
tests := []struct {
150+
name string
151+
executorInput *pipelinespec.ExecutorInput
152+
executorArgs []string
153+
wantErr bool
154+
}{
155+
{
156+
"happy pass",
157+
&pipelinespec.ExecutorInput{
158+
Inputs: &pipelinespec.ExecutorInput_Inputs{
159+
ParameterValues: map[string]*structpb.Value{"a": structpb.NewNumberValue(1), "b": structpb.NewNumberValue(2)},
160+
},
161+
},
80162
[]string{"-c", "test {{$.inputs.parameters['a']}} -eq 1 || exit 1\ntest {{$.inputs.parameters['b']}} -eq 2 || exit 1"},
81163
false,
82164
},

‎samples/v2/component_with_runtime_artifct_custom_path.py‎ renamed to ‎samples/v2/component_with_runtime_artifact_custom_path.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ def validate_custom_path(exp_path: str, input_list: Output[Artifact]) -> bool:
1717
if input_list.path is not exp_path:
1818
raise ValueError(f"File uri is {input_list.path} but should be {exp_path}.")
1919

20+
def component(artifact: Output[Artifact]) -> bool:
21+
return True
22+
2023
@dsl.pipeline
2124
def pipeline_with_custom_path_artifact():
2225
task1 = create_list()
2326
task1.output.set_custom_path('/etc/test/file/path')
24-
# task2 = validate_custom_path(path='/etc/test/file/path', input_list=task1.output)
27+
# task2 = validate_custom_path(path='/etc/test/file/path', input_list=task1.output)

‎samples/v2/sample_test.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,13 @@ def get_kfp_pipeline_spec_path() -> str:
4141
return path
4242

4343
def get_package_path(subdir: str) -> str:
44-
repo_name = os.environ.get('REPO_NAME', 'kubeflow/pipelines')
45-
if os.environ.get('PULL_NUMBER'):
46-
path = f'git+https://github.com/{repo_name}.git@refs/pull/{os.environ["PULL_NUMBER"]}/merge#subdirectory={subdir}'
47-
else:
48-
path = f'git+https://github.com/{repo_name}.git@master#subdirectory={subdir}'
49-
return path
44+
return f'git+https://github.com/alyssacgoins/data-science-pipelines.git@refs/pull/12248/merge#subdirectory=sdk/python'
45+
# repo_name = os.environ.get('REPO_NAME', 'kubeflow/pipelines')
46+
# if os.environ.get('PULL_NUMBER'):
47+
# path = f'git+https://github.com/{repo_name}.git@refs/pull/{os.environ["PULL_NUMBER"]}/merge#subdirectory={subdir}'
48+
# else:
49+
# path = f'git+https://github.com/{repo_name}.git@master#subdirectory={subdir}'
50+
# return path
5051

5152
# Set the component configuration BEFORE importing any pipeline modules
5253
# To have pipeline execution code leverage source kfp-pipeline-spec

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def __init__(self,
8282
self.uri = uri or ''
8383
self.name = name or ''
8484
self.metadata = metadata or {}
85-
self.custom_path: str = ''
85+
self.custom_path = ''
8686

8787
@property
8888
def path(self) -> str:
@@ -93,7 +93,7 @@ def path(self, path: str) -> None:
9393
self._set_path(path)
9494

9595
def _get_path(self) -> Optional[str]:
96-
if self.custom_path != "":
96+
if self.custom_path is not '':
9797
return self.custom_path
9898
if self.uri.startswith(RemotePrefix.GCS.value):
9999
return _GCS_LOCAL_MOUNT_PREFIX + self.uri[len(RemotePrefix.GCS.value
@@ -116,14 +116,12 @@ def _set_path(self, path: str) -> None:
116116

117117
@property
118118
def custom_path(self) -> str:
119-
return self.custom_path
119+
return self._custom_path
120120

121121
@custom_path.setter
122122
def custom_path(self, value):
123-
self._set_custom_path = value
123+
self._custom_path = value
124124

125-
def _set_custom_path(self, path: str) -> None:
126-
self.custom_path = path
127125

128126
def convert_local_path_to_remote_path(path: str) -> str:
129127
if path.startswith(_GCS_LOCAL_MOUNT_PREFIX):

‎sdk/python/test_data/pipelines/component_with_runtime_artifact_custom_path.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,30 @@
1+
from kfp import dsl
12
from kfp.dsl import Output
23
from kfp.dsl.types.artifact_types import Artifact
3-
from kfp.v2 import dsl
44

55

66
@dsl.component
77
def create_list() -> list:
88
return [1, 2, 3, 4]
99

10+
1011
@dsl.component
1112
def append_to_list(digit: int, input_list: Output[Artifact]) -> list:
1213
input_list.append(digit)
1314
return input_list
1415

16+
1517
@dsl.component
1618
def validate_custom_path(exp_path: str, input_list: Output[Artifact]) -> bool:
1719
#todo: is this the correct comparison? (or should use != instead?)
1820
if input_list.path is not exp_path:
19-
raise ValueError(f"File uri is {input_list.path} but should be {exp_path}.")
21+
raise ValueError(
22+
f"File uri is {input_list.path} but should be {exp_path}.")
23+
2024

2125
@dsl.pipeline
2226
def pipeline_with_custom_path_artifact():
2327
task1 = create_list()
2428
task1.output.set_custom_path('/etc/test/file/path')
25-
task2 = validate_custom_path(path='/etc/test/file/path', input_list=task1.output)
29+
task2 = validate_custom_path(
30+
path='/etc/test/file/path', input_list=task1.output)

0 commit comments

Comments
 (0)