feat(sdk): SDK - Deprecation warning when using ContainerOp

[Ark-kun]

We have long advised our users to create reusable components.
Creating reusable components is as easy as creating ContainerOp instances, but the components are shareable, portable and are easier to support going forward.

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[kubeflow-bot]

This change is 

[Bobgy]

Does deprecation warning mean container op will be removed in the future?

have compatibility and support guarantees

Can you explain this part?

I think people typically don't want a layer of abstraction before thinking about reusability. Is the best option a component with no arguments in that case?

[Ark-kun]

Does deprecation warning mean container op will be removed in the future?
Not necessary. But its role would be "task-like object that you get when creating an instance of component when compiling pipeline using KFP's compiler". So the users will still be interacting with ContainerOp objects, but they won't be creating those objects manually.

have compatibility and support guarantees
Can you explain this part?

Components have better support throughout the system compared with raw ContainerOp.

• ContainerOps do not have inputs
• ContainerOps do not have types (so the artifacts in MLMD are untyped)
• ContainerOps are not supported in graph components
• There are no plans to support ContainerOps in TFX, but there are already multiple experimenta implementations of component support

Components have a more supported way forward compared to ContainerOp. There are already multiple experimental implementations of KFP component support for TFX, but it's not really possible to support ContainerOps there.

I think people typically don't want a layer of abstraction before thinking about reusability.

I understand. But the equivalent components are the same number of lines/characters as ContainerOp code while providing more benefits. Also, I've answered dozens of issues from people using ContainerOps and many of those would not happen is they just used components. For example, with ContainerOp people are not thinking how they consume outputs, so they sometimes consume big outputs as values and then get runtime errors due to the data size.

Is the best option a component with no arguments in that case?

I think so.

Example 1:

hello_op = kfp.components.load_component_from_text('''
implementation:
  container:
    image: alpine
    command: ["echo", "Hello world"]
''')

instead of

def hello_op():
    return kfp.dsl.ContainerOp(
        name="Hello",
        image="alpine",
        command=["echo", "Hello world"],
    )

Example 2:

hello_op = kfp.components.load_component_from_text('''
inputs:
- name: message
implementation:
  container:
    image: alpine
    command: ["echo", {inputValue: message}]
''')

instead of

def hello_op(message):
    return kfp.dsl.ContainerOp(
        name="Hello",
        image="alpine",
        command=["echo", message],
    )

Example 3:

hello_op = kfp.components.load_component_from_text('''
inputs:
- name: message
inputs:
- name: result
implementation:
  container:
    image: alpine
    command: ["sh", "-c", 'mkdir -p "$(dirname "$1")"; echo "$0" > "$1"', {inputValue: message}, {outputPath: result}]
''')

instead of

def hello_op(message):
    return kfp.dsl.ContainerOp(
        name="Hello",
        image="alpine",
        command: ["sh", "-c", 'mkdir -p "$(dirname "$1")"; echo "$0" > "$1"', message, '/tmp/outputs/result/data'],
        output_artifact_paths={
            'result': '/tmp/outputs/result/data',
        },
    )

[Bobgy]

Thanks! I'm convinced. It would be great adding these to documentation.

So the plan is deprecating ContainerOp right now and removing it in the future?
SGTM

/lgtm

[Ark-kun]

So the plan is deprecating ContainerOp right now and removing it in the future?

For now there are no plans to actually deprecate the class or remove it. Here is how it currently works:

KFP: component.yaml --load--> Callable --instantiate--> ContainerOp object
TFX: component.yaml --load--> Callable --instantiate--> BaseComponent object

So ContainerOp will still exist. You'll be able to access ContainerOp.outputs, ContainerOp.container.*, , ContainerOp.set_display_name(), ContainerOp.after(), ContainerOp.apply(), ContainerOp.add_volume() etc. It's still ContainerOp. Basically, we're only deprecating calling ContainerOp.__init__, but everything else remains.

[Ark-kun]

It would be great adding these to documentation.

Good idea. I'm working on updating the documentation.

[Bobgy]

Got it, sounds good

[darthsuogles]

KFP: component.yaml --load--> Callable --instantiate--> ContainerOp object
Will this continue to be the case? We have code depending on manipulating the returned ContainerOp in kfp.dsl.pipeline annotated functions.

[Ark-kun]

/retest

[Ark-kun]

/retest

[Ark-kun]

/approve

[Bobgy]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ark-kun, Bobgy

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 [Ark-kun,Bobgy]

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

[Bobgy]

edited title to make it clearer ContainerOp is the one being deprecated