fix(backend): fixes "cannot save parameter" error message. Fixes #9678

[hbelmiro]

Fixes: #9678

Description of your changes:

I propose these changes as an initial solution, but I'm not sure it's the best one. So, feel free to recommend a better one and I'll be happy to implement it.

This PR modifies the driver so it creates the needed files with default values when IterationCount and Condition are nil.

The errors happen because we return an Execution without setting IterationCount and Condition here:

• pipelines/backend/src/v2/driver/driver.go

  Line 175 in 87db18e

  return &Execution{ID: exec.GetID()}, nil

• pipelines/backend/src/v2/driver/driver.go

  Line 343 in 87db18e

  return execution, nil

Which prevents it from creating the files here:

• https://github.com/opendatahub-io/data-science-pipelines/blob/410a82d45e6c51b259eeccab2e85f9b42d627880/backend/src/v2/cmd/driver/main.go#L194
• https://github.com/opendatahub-io/data-science-pipelines/blob/410a82d45e6c51b259eeccab2e85f9b42d627880/backend/src/v2/cmd/driver/main.go#L204

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.

[google-oss-prow]

Hi @hbelmiro. 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.

[HumairAK]

/ok-to-test

[hbelmiro]

/retest

[hbelmiro]

The failing test is unrelated.

AttributeError: module 'kfp_server_api' has no attribute 'ApiGetHealthzResponse'
make: *** [Makefile:18: sample-test] Error 1

[rimolive]

@chensun @connor-mccarthy @zijianjoy Can we ignore this failed test and just merge?

[zijianjoy]

/assign @chensun

[Tomcli]

/lgtm

nit: Maybe we can have a simple unit test to make sure in all scenarios, it should always write down the outputs for conditions and iterationcount?

[hbelmiro]

nit: Maybe we can have a simple unit test to make sure in all scenarios, it should always write down the outputs for conditions and iterationcount?

@Tomcli any ideas on how to do that?
I only found tests that use the KFP API to invoke tasks and check results. For this particular issue, I'd have to read the files inside the pods. Would you know any existing test that I can use as a reference?

[Tomcli]

nit: Maybe we can have a simple unit test to make sure in all scenarios, it should always write down the outputs for conditions and iterationcount?

@Tomcli any ideas on how to do that? I only found tests that use the KFP API to invoke tasks and check results. For this particular issue, I'd have to read the files inside the pods. Would you know any existing test that I can use as a reference?

I don't think there's any test for the main.go. I'm thinking if you can implement a simple check file path exist function to make sure these files are exist after the test case. For the long term implementation, we may want to move this whole logic to use Argo's execution plugin. This way we don't have to spend extra time to create new pods and write files.

The test case is just nice to have since the main.go can be implemented in a more efficient way in the future.

[Tomcli]

@hbelmiro Can you redo your commits with git signoff? Kubeflow is moving to DCO CLA so all the commits from now on needed to be signed on git.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Tomcli

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:

• backend/OWNERS [Tomcli]

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

[google-oss-prow]

@hbelmiro: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
kubeflow-pipelines-samples-v2	8c554a8	link	false	/test kubeflow-pipelines-samples-v2

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. I understand the commands that are listed here.

[hbelmiro]

@Tomcli tests added. I just had to do a small refactor in main.go to make the code more testable.

[Tomcli]

thanks @hbelmiro
/lgtm