pipelines
pipelines copied to clipboard
fix(sdk): Fix deprecated client to work with kfp-server-api 2.0.1
Description of your changes:
"There are two ways of doing things at Google: the deprecated one, and the one that doesn't work yet" (psrc)
The deprecated client is currently broken, as it refers to APIs that don't exist anymore. This change makes things less broken, but there might be more fixes needed.
We had started by trying to upgrade to the pipelines V2 API, but it's still missing several Kubernetes-specific features that we need (affinities, host mounts, shared memory, etc.). Some have PRs in progress, some not even that.
We then tried using the kfp.deprecated code bundled in V2, because staying on kfp 1.8.5 is preventing us from upgrading several of our Conda dependencies: protobuf, grpc, python-kubernetes, etc. Those, in turn, block other upgrades.
Unfortunately, kfp.deprecated is suffering from bit rot, so some surgery is needed.
Basically:
- Job -> RecurringRun
- adapt code from the current client that warns for FOO_job() and forwards to FOO_recurring_run()
- adapt some code from create_recurring_run()
- version the APIs we consume: "Api" -> "V2beta1"
Checklist:
- [X] The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign chensun for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Hi @therc. 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.
/ok-to-test
This was tested with
kfp 2.8.0 pyhd8ed1ab_0 conda-forge
kfp-pipeline-spec 0.3.0 pyhd8ed1ab_0 conda-forge
kfp-server-api 2.0.1 pyhd8ed1ab_0 conda-forge
/rerun-all
/retest
/test all
I fixed the YAPF errors, but can't rerun the tests. I guess the failure and/or my new commit made the PR lose its trusted status.
I guess the failure and/or my new commit made the PR lose its trusted status.
that's odd. It's still labeled "ok-to-test", but now it says "10 workflows awaiting approval". @hbelmiro, have you ever seen that before?
/rerun-all
I guess the failure and/or my new commit made the PR lose its trusted status.
that's odd. It's still labeled "ok-to-test", but now it says "10 workflows awaiting approval". @hbelmiro, have you ever seen that before?
@gregsheremeta yes: https://github.com/kubeflow/pipelines/issues/10981
Had to trigger test-run-all-gcpc-modules again, by hand, but it's all green now. Blocked on approvals. How often are SDKs released? sdk/CONTRIBUTING.md doesn't mention anything about what would trigger a version bump. We'd be happy if it included @gregsheremeta's #10913 as well!
cc @chensun
Ping?
I looked it over and it looks fine, but I have 0 experience with v1 or the deprecated stuff ... so ideally @chensun can look at and ack this. I'm inclined to tag it lgtm since it's a deprecated file and the tests pass, but I'll wait for Chen.
We had started by trying to upgrade to the pipelines V2 API, but it's still missing several Kubernetes-specific features that we need (affinities, host mounts, shared memory, etc.). Some have PRs in progress, some not even that.
PRs would be great. We have several example PRs of adding these things back if that would help guide you.
Alternatively, please open issues for the things you need that are still missing.
/reopen
@hbelmiro: Reopened this PR.
In response to this:
/reopen
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.
@therc the CI is working now. Can you please rebase?
I imagine that we should remove the kfp.deprecated namespace if we have decided to not fix it, because it literally does not work without this PR.
Sorry, I should have closed this PR. The deprecated namespace was removed without much explanation in November in https://github.com/kubeflow/pipelines/pull/11366