feat(sdk): support dynamic machine type parameters in pipeline task setters

[KevinGrantLee]

Description of your changes:

Enables dynamic values for cpu_limit, cpu_request, memory_limit, memory_request, accelerator_type, accelerator_limit.

Enables pipelines like

@dsl.component
def cpu_limit() -> str:
    return '4000m'

@dsl.component
def sum_numbers(a: int, b:int) -> int:
    return a + b


@dsl.pipeline
def pipeline(
):
    sum_numbers_task = sum_numbers(a=1, b=2)
    sum_numbers_task.set_cpu_limit(cpu_limit().output)

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.

[KevinGrantLee]

Tests succeed locally, but fail here. Need to release new kfp-pipeline-spec to pass tests. Also wait for backend changes to roll to prod.

[gregsheremeta]

Nice! Is there an open issue for this by any chance? I see it's related to your previous work on this.

[KevinGrantLee]

Marking this as draft so we don't accidentally merge it before backend changes are ready. But code is ready to be reviewed.

Hi Mark, I don't believe we have an open issue for this.

[convect-bot]

Tests succeed locally, but fail here. Need to release new kfp-pipeline-spec to pass tests. Also wait for backend changes to roll to prod.

So given your current sdk modification, can the generate pipeline yaml be run successfully with the current kfp server( version=2.2.0)? or need to adjust the kfp server in order to run the pipeline?

This feasture is needed in our project, since it seems that v2 remove the original support for handle pipeline parameters for setup resource limit.

Thanks a lot!

[KevinGrantLee]

Backend changes are rolled out. Still need to wait for kfp-pipeline-spec release.

[KevinGrantLee]

Hi @convect-bot , yes this sdk change will be able to be run with the current kfp server version. However, you will need to install the next version of kfp-pipeline-spec https://pypi.org/project/kfp-pipeline-spec/ once it is released.

[chensun]

/lgtm

Thank you, @KevinGrantLee !

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

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 [chensun]

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

[gregsheremeta]

merging master into your branch resulted in your PR having 33 contributors 😂

94b1a0d

weird eh?

@chensun have you seen that before? How do we avoid that kind of thing?

[chensun]

merging master into your branch resulted in your PR having 33 contributors 😂

94b1a0d

weird eh?

@chensun have you seen that before? How do we avoid that kind of thing?

Fixed that (kind of, now I'm the additional co-author as I had to rewrite and reorder the commits).

I definitely saw this in the past as well. I think rebase after pull can definitely avoid this, although there might be more work for the authors in some cases. Maybe PR reviewers can ask author to rebase if we think avoiding this is necessary?

[HumairAK]

Yeah I think it's good form to just have the author rebase and keep the commits clean in the PR (reviewers and approvers should keep the author honest here)
The approver could squash/rebase/clean up, but then they will end up being co-author. Better to just have the author do it if feasible and non urgent.

[gregsheremeta]

+1 to asking author to rebase