[SDK] Relax k8s sanitization #2634
Conversation
|
/assign @Ark-kun |
|
/hold need fine-grained control over sanitization |
|
/retest |
|
/test kubeflow-pipeline-sample-test |
|
/hold cancel |
| @@ -42,7 +53,7 @@ def match_serialized_pipelineparam(payload: str): | |||
| for match in matches: | |||
| pattern = '{{pipelineparam:op=%s;name=%s}}' % (match[0], match[1]) | |||
| param_tuples.append(PipelineParamTuple( | |||
| name=sanitize_k8s_name(match[1]), | |||
| name=sanitize_k8s_name(match[1], True), | |||
| op=sanitize_k8s_name(match[0]), | |||
There was a problem hiding this comment.
I'm not sure this is a proper place for sanitization. The names should probably be converted in the compiler:
There was a problem hiding this comment.
Agreed. I was curious why there are two sanitize functions as well.
|
/lgtm |
|
/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 |
|
It seems like @k8s-ci-robot forgot this one. |
* Use kubectl -k instead of kustomize Signed-off-by: Dan Sun <dsun20@bloomberg.net> * Pin fastapi to 0.88.0 Signed-off-by: Dan Sun <dsun20@bloomberg.net> * Update make deploy-ci Signed-off-by: Dan Sun <dsun20@bloomberg.net> * Update make deploy-ci Signed-off-by: Dan Sun <dsun20@bloomberg.net> Signed-off-by: Dan Sun <dsun20@bloomberg.net>
Let k8s sanitization allows capital letters and under score in the names of parameters/outputs. This change will allow users to refer to self-defined attributes of a complex structure in a more Pythonic way.
One implication of this change would be, we have to delegate to users the responsibility of ensuring the argo naming convention when they create any k8s object within the pipeline. For example, in the volume ops example, a user should specify
name='create-pvc'instead ofname='create_pvc'to comply with argo naming convention.Fixes #2570
Fixes #2629
This change is