add init container for container op #1650
Conversation
|
fix #1528 |
sdk/python/pipeline.yaml
Outdated
| @@ -0,0 +1,170 @@ | |||
| apiVersion: argoproj.io/v1alpha1 | |||
There was a problem hiding this comment.
This file probably should not be checked in.
| @@ -290,6 +290,10 @@ def test_py_compile_basic(self): | |||
| """Test basic sequential pipeline.""" | |||
| self._test_py_compile_zip('basic') | |||
|
|
|||
| def test_py_compile_with_init_container(self): | |||
There was a problem hiding this comment.
Can you please make a test that tests the behavior of the new API feature (as opposed to just checking that nothing has changed)? Good examples are test_set_display_name, test_op_transformers.
sdk/python/kfp/dsl/_container_op.py
Outdated
| @@ -630,6 +630,95 @@ def inputs(self): | |||
| return _pipeline_param.extract_pipelineparams_from_any(self) | |||
|
|
|||
|
|
|||
| class InitContainer(Container): | |||
There was a problem hiding this comment.
InitContainer is exactly same as Sidecar if my understanding is correct. Could you use the same class to represent both of them to avoid duplication? Maybe call it UserContainer?
There was a problem hiding this comment.
done. also i need to keep the sidecar interface to avoid breaking change but switch and reuse user container as implementation.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun 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 |
sdk/python/kfp/dsl/_container_op.py
Outdated
| @@ -630,6 +630,95 @@ def inputs(self): | |||
| return _pipeline_param.extract_pipelineparams_from_any(self) | |||
|
|
|||
|
|
|||
| class InitContainer(Container): | |||
There was a problem hiding this comment.
should we update doc here to add initcontainer?
do we also need a sample to illustrate how to use initcontainer?
There was a problem hiding this comment.
yeah i'll add it separately.
|
/lgtm |
|
/test kubeflow-pipeline-e2e-test |
* misc_patch_dev.sh: add resource limit for storage initializer Signed-off-by: zhangzhengyuan <zhangzhengyuan0604@gmail.com> * rename misc_patch_dev.sh, as it is now single purpose Signed-off-by: zhangzhengyuan <zhangzhengyuan0604@gmail.com>
/assign @hongye-sun
This change is