Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk): supporting bring your own container for arbitrary input and outputs #8066

Merged
merged 25 commits into from
Aug 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2df8d19
support container_component decorator for function with no inputs
zichuan-scott-xu Jun 29, 2022
101b615
resolve review comments
zichuan-scott-xu Jul 1, 2022
56ed6d5
add sample tests for milestone 1
zichuan-scott-xu Jul 6, 2022
40c9b81
modify compiler test data
zichuan-scott-xu Jul 7, 2022
e9a8bf1
resolve reviews
zichuan-scott-xu Jul 7, 2022
56ac809
resolve reviews
zichuan-scott-xu Jul 7, 2022
702f3ef
WIP
zichuan-scott-xu Jul 11, 2022
b6eb5e2
implementation of function of no inputs
zichuan-scott-xu Jul 13, 2022
4f7745a
fixed sample test
zichuan-scott-xu Jul 14, 2022
8275381
re-fix sample test
zichuan-scott-xu Jul 14, 2022
7b92223
fix rebase merge conflict
zichuan-scott-xu Jul 15, 2022
dc064b5
resolve formatting
zichuan-scott-xu Jul 15, 2022
eb31781
resolve isort error for test data
zichuan-scott-xu Jul 15, 2022
8db6d89
resolve comments
zichuan-scott-xu Jul 18, 2022
b146ae0
fix nit
zichuan-scott-xu Jul 18, 2022
bb9484e
resolve nit
zichuan-scott-xu Jul 18, 2022
920a30f
add implementation for placeholders i/o, sample and compiler tests
zichuan-scott-xu Jul 22, 2022
85cb271
Merge branch 'kubeflow:master' into cuj3-milestone2
zichuan-scott-xu Jul 22, 2022
4199c72
resolve comments and merge logic for constructing container component
zichuan-scott-xu Jul 27, 2022
48ec85b
Merge branch 'cuj3-milestone2' of https://github.com/zichuan-scott-xu…
zichuan-scott-xu Jul 27, 2022
97a1e1a
resolve comments
zichuan-scott-xu Jul 28, 2022
2dc74c4
resolve comments
zichuan-scott-xu Aug 1, 2022
e8f393c
fix assertion messages
zichuan-scott-xu Aug 1, 2022
bb282dc
add error handling for accessing artifact by itself
zichuan-scott-xu Aug 4, 2022
fb26f91
add test for raising error for accessing artifact by itself
zichuan-scott-xu Aug 5, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions samples/v2/two_step_pipeline_containerized_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,31 +54,26 @@ def verify(t: unittest.TestCase, run: kfp_server_api.ApiRun,
'outputs': {
'artifacts': [{
'name': 'output_gcs',
'type': 'system.Artifact',
# 'type': 'system.Dataset'
'type': 'system.Dataset'
}],
},
'type': 'system.ContainerExecution',
'state': Execution.State.COMPLETE,
},
component1_dict)
}, component1_dict)

t.assertEqual(
{
'name': 'component2',
'inputs': {
'artifacts': [{
'name': 'input_gcs',
# TODO(chesu): compiled pipeline spec incorrectly sets importer artifact type to system.Artifact, but in the pipeline, it should be system.Dataset.
'type': 'system.Artifact',
# 'type': 'system.Dataset'
'type': 'system.Dataset'
}],
},
'outputs': {},
'type': 'system.ContainerExecution',
'state': Execution.State.COMPLETE,
},
component2_dict)
}, component2_dict)


if __name__ == '__main__':
Expand Down
6 changes: 3 additions & 3 deletions sdk/python/kfp/compiler/compiler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -874,12 +874,12 @@ def container_with_artifact_output(
])

with tempfile.TemporaryDirectory() as tempdir:
output_json = os.path.join(tempdir, 'component.yaml')
output_yaml = os.path.join(tempdir, 'component.yaml')
compiler.Compiler().compile(
pipeline_func=container_with_artifact_output,
package_path=output_json,
package_path=output_yaml,
pipeline_name='container-with-artifact-output')
with open(output_json, 'r') as f:
with open(output_yaml, 'r') as f:
pipeline_spec = yaml.safe_load(f)
self.assertEqual(
zichuan-scott-xu marked this conversation as resolved.
Show resolved Hide resolved
pipeline_spec['components']['comp-container-with-artifact-output']
Expand Down
10 changes: 6 additions & 4 deletions sdk/python/kfp/components/component_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ def __getattr__(
.InputPathPlaceholder, placeholders.OutputUriPlaceholder,
placeholders.OutputPathPlaceholder]:
zichuan-scott-xu marked this conversation as resolved.
Show resolved Hide resolved
if _name not in ['uri', 'path']:
raise AttributeError(
'Accessing artifact attribute other than uri or path is not supported.'
)
raise AttributeError('Cannot access artifact attribute "{_name}".')
zichuan-scott-xu marked this conversation as resolved.
Show resolved Hide resolved
if self._io_type == 'input':
if _name == 'uri':
return placeholders.InputUriPlaceholder(self._var_name)
Expand Down Expand Up @@ -279,8 +277,8 @@ def extract_component_interface(
inputs[io_name] = input_spec

#Analyzing the return type annotations.
return_ann = signature.return_annotation
if not containerized:
zichuan-scott-xu marked this conversation as resolved.
Show resolved Hide resolved
return_ann = signature.return_annotation
if hasattr(return_ann, '_fields'): #NamedTuple
# Getting field type annotations.
# __annotations__ does not exist in python 3.5 and earlier
Expand Down Expand Up @@ -321,6 +319,10 @@ def extract_component_interface(
signature.return_annotation)
output_spec = structures.OutputSpec(type=type_struct)
outputs[output_name] = output_spec
elif return_ann != inspect.Parameter.empty and return_ann != structures.ContainerSpec:
raise TypeError(
'Return annotation should be either ContainerSpec or ignored for container components.'
zichuan-scott-xu marked this conversation as resolved.
Show resolved Hide resolved
)

# Component name and description are derived from the function's name and
# docstring. The name can be overridden by setting setting func.__name__
Expand Down