SDK/Compiler - Invoke the op_transformers as early as possible

[kvalev]

Invoking the op_transformers functions after the container inputs have been determined will result in an invalid argo yaml definition, in case a transformation function is used to set pod labels based on a pipeline parameter.

Before this PR:

  - dag:
      tasks:
      - name: cop
        template: cop
    name: parameters-in-op-transformation-functions

After this PR:

  - dag:
      tasks:
      - arguments:
          parameters:
          - name: param
            value: '{{inputs.parameters.param}}'
        name: cop
        template: cop
    inputs:
      parameters:
      - name: param
    name: parameters-in-op-transformation-functions

---

This change is 

[k8s-ci-robot]

Hi @kvalev. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[kvalev]

/assign @hongye-sun

[Ark-kun]

/lgtm

[hongye-sun]

I don't have any problem with this PR. Alexey, do you have any comment?

[Ark-kun]

I don't have any problem with this PR. Alexey, do you have any comment?

I think it's good. There might be cases where the old behavior is desirable, but the proposed variant seems to be more useful overall: It's now on par with calling .apply in the pipeline function (since that's also executed early).

/approve

BTW, Initially I wanted to have two transformers - op_transformer that runs before compilation to template and the template_transformer that runs after. But we've decided to drop the template_transformer

[Ark-kun]

/test kubeflow-pipeline-sample-test

[Ark-kun]

/hold
The tests are failing. I'm looking why the unit tests are not failing.

[Ark-kun]

The code is not compatible with Python 3.5 which we still support (partially due to Debian)
/approve cancel

[Ark-kun]

/lgtm
Waiting for all tests to pass.

[kvalev]

I was not aware that it should be compatible with Python 3.5 - the travis build only tests 3.6. I reverted the type hinting, would you mind taking another look @Ark-kun?

[Ark-kun]

I was not aware that it should be compatible with Python 3.5

In latest stable Debian the latest version of python you can get using apt-get is 3.5.

pipelines/sdk/python/setup.py

Line 59 in 5061fcf

'Programming Language :: Python :: 3.5',

I'd like to get a data point from you: If the KF Pipelines SDK needed the python version that you cannot install by just running apt-get install python3, would that be a problem for you? Do you think that can be a problem for other potential users?

[Ark-kun]

/hold cancel

[kvalev]

Well, it would not stop me from using the SDK, but it would be somewhat annoying, if I cant just pip install it. Sure, you can always either compile a newer Python version yourself or run it in a docker container, but it is just not the same. I would suggest keeping the Python 3.5 compatibility - I dont think there are significant downsides.

[Ark-kun]

/approve

There seems to be a conflict. Can you please resolve it?

[k8s-ci-robot]

[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:

• sdk/OWNERS [Ark-kun]

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

[Ark-kun]

/lgtm