fix(sdk): sanitize op name. Fix #6433

[lynnmatrix]

Description of your changes:
Fix #6433
Fix #6576

Fix the error that kfp v1 compiler failed to provide unique name for ops of the same component.

Checklist:

• The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

[chensun]

/assign @ji-yaqi is also looking into the issue, she can follow up on the review or alternative changes. Thanks!

[ji-yaqi]

Hi @lynnmatrix, thanks for your PR!

For sanitizing, it might be more than replacing spaces with dashes etc. Could you use the sanitize_k8s_name function to sanitize self._component_spec_inputs_with_pipeline_params?

[chensun]

Hi @lynnmatrix, thanks for your PR!

For sanitizing, it might be more than replacing spaces with dashes etc. Could you use the sanitize_k8s_name function to sanitize self._component_spec_inputs_with_pipeline_params?

I agree this would be the ideal change. That said, it also feels silly that we know that op.name needs to be sanitized, yet still put a space in it. So I think the current change also makes sense.

Given the bug is pretty bad, and the sanitizing everywhere would likely require some more time to fix a dozen places. Maybe we can go with this change first so it would catch the release train this week, and we can do the proposed change as part of #5831.

@lynnmatrix can you please add a release note for this change?

[lynnmatrix]

Sure, @chensun.
@ji-yaqi, I feel only sanitizing self._component_spec_inputs_with_pipeline_params is neither the ideal change, maybe we should do sanitizing as soon as possible, or make sure the name is sanitized in the constructor of PipelineParam.

Hi @lynnmatrix, thanks for your PR!
For sanitizing, it might be more than replacing spaces with dashes etc. Could you use the sanitize_k8s_name function to sanitize self._component_spec_inputs_with_pipeline_params?

I agree this would be the ideal change. That said, it also feels silly that we know that op.name needs to be sanitized, yet still put a space in it. So I think the current change also makes sense.

Given the bug is pretty bad, and the sanitizing everywhere would likely require some more time to fix a dozen places. Maybe we can go with this change first so it would catch the release train this week, and we can do the proposed change as part of #5831.

@lynnmatrix can you please add a release note for this change?

[ji-yaqi]

/retest

[ji-yaqi]

/lgtm
/approve
Thanks @lynnmatrix

[google-oss-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ji-yaqi

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:

• sdk/OWNERS [ji-yaqi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[chensun]

This actually affected both v1 and v2 compiler.
@ji-yaqi can you please fix this description when you do the release? Thanks!