feat(sdk): supporting bring your own container for arbitrary input and outputs #8066
Conversation
|
Hi @zichuan-scott-xu. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cc @connor-mccarthy |
|
@zichuan-scott-xu: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
sdk/python/kfp/compiler/test_data/components/container_with_artifact_output.py
Outdated
Show resolved
Hide resolved
|
/ok-to-test |
|
/retest |
There was a problem hiding this comment.
/lgtm
Great work, @zichuan-scott-xu !
I left one question, but it doesn't necessarily block this PR from merging.
| '--epochs', | ||
| num_epochs, | ||
| '--model_path', | ||
| model.uri, |
There was a problem hiding this comment.
What if user just puts model here? Do we throw an error or return the whole artifact json? Also what if users access some arbitrary valid or invalid property/method here? Do we throw errors?
There was a problem hiding this comment.
Thanks for the reminder! The current design is to throw error in both cases and I will add the error handling logic before merging the PR.
There was a problem hiding this comment.
@zichuan-scott-xu, can you add a test to validate that the exception in bb282dc is raised correctly?
|
/retest |
|
/approve |
|
/approve Great work, @zichuan-scott-xu ! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chensun, connor-mccarthy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
…d outputs (kubeflow#8066) * support container_component decorator for function with no inputs * resolve review comments * add sample tests for milestone 1 * modify compiler test data * resolve reviews * resolve reviews * WIP * implementation of function of no inputs * fixed sample test * re-fix sample test * fix rebase merge conflict * resolve formatting * resolve isort error for test data * resolve comments * fix nit * resolve nit * add implementation for placeholders i/o, sample and compiler tests * resolve comments and merge logic for constructing container component * resolve comments * resolve comments * fix assertion messages * add error handling for accessing artifact by itself * add test for raising error for accessing artifact by itself
Description of your changes:
The first 16 commits handling functions with no inputs are already reviewed and approved in #7973.
Checklist: