Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk): Upload namespaced pipeline definitions. Part of #4197 #8512

Conversation

elikatsis
Copy link
Member

Description of your changes:
Add a namespace field in the relevant Python wrappers for uploading namespaced pipeline definitions.

This is the SDK part pushed by #8196. See the aforementioned PR and the linked issued for details.

This PR is part of the overarching idea to support namespaced pipeline definitions.
The whole idea is thoroughly analyzed and exposed in this design doc: https://docs.google.com/document/d/1fM4y2L1IVqVj-iiNjYFRRktdCh7FQXgU2XpaYLaqt-A/edit?resourcekey=0-kd5loyP7w3PBD0ug6ECmLQ#

Objective of this PR is to:

  1. Extend the corresponding Python wrappers with a namespace field

Part of #4197
Blocked by #8511 + kfp-server-api release

Checklist:

@google-oss-prow
Copy link

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

@@ -22,7 +22,7 @@ kfp-pipeline-spec>=0.1.16,<0.2.0
# Update the lower version when kfp sdk depends on new apis/fields in
# kfp-server-api.
# Note, please also update ./requirements.in
kfp-server-api>=2.0.0a0,<3.0.0
kfp-server-api>=2.0.0a7,<3.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFYI, we're in progress updating APIs for v2, once that is done (ETA: end of the month or early January), the next release would be the first beta.

@juliusvonkohout
Copy link
Member

@elikatsis what is the status of this and the webinterface?

@elikatsis
Copy link
Member Author

@juliusvonkohout for this one we're waiting for a new release of the kfp-server-api package (https://pypi.org/project/kfp-server-api/#history).
@chensun any update on this? The initial plan was end of Dec or early Jan but I don't see a new release since October.

@juliusvonkohout
Copy link
Member



Add a namespace field in the relevant Python wrappers for uploading
namespaced pipeline definitions.
@elikatsis elikatsis force-pushed the feature-pangiann-upload-namespaced-pipelines-sdk branch from 1673b07 to 10bb01b Compare February 9, 2023 14:10
@google-oss-prow
Copy link

[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 connor-mccarthy for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

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

@elikatsis elikatsis marked this pull request as ready for review February 9, 2023 14:10
@elikatsis
Copy link
Member Author

Thanks @juliusvonkohout, great coincidence 😄

@chensun I've updated and rebased the branch. This should be good to go if everything gets green :)

@google-oss-prow
Copy link

@elikatsis: The following tests 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-sdk-execution-tests 10bb01b link true /test kubeflow-pipelines-sdk-execution-tests
kubeflow-pipelines-component-yaml 10bb01b link true /test kubeflow-pipelines-component-yaml
kubeflow-pipelines-sdk-python37 10bb01b link true /test kubeflow-pipelines-sdk-python37
kubeflow-pipelines-sdk-python38 10bb01b link true /test kubeflow-pipelines-sdk-python38
kubeflow-pipelines-sdk-python310 10bb01b link true /test kubeflow-pipelines-sdk-python310
kubeflow-pipelines-sdk-python39 10bb01b link true /test kubeflow-pipelines-sdk-python39

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.

@elikatsis
Copy link
Member Author

wth

+ pytest /home/prow/go/src/github.com/kubeflow/pipelines/test/sdk-execution-tests/sdk_execution_tests.py --asyncio-task-timeout 2700
============================= test session starts ==============================
platform linux -- Python 3.7.16, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/prow/go/src/github.com/kubeflow/pipelines
plugins: mock-3.8.2, asyncio-cooperative-0.28.0
collected 0 items / 1 error

==================================== ERRORS ====================================
_______ ERROR collecting test/sdk-execution-tests/sdk_execution_tests.py _______
test/sdk-execution-tests/sdk_execution_tests.py:21: in <module>
    from kfp import client
/usr/local/lib/python3.7/site-packages/kfp/__init__.py:23: in <module>
    from kfp.client import Client
/usr/local/lib/python3.7/site-packages/kfp/client/__init__.py:20: in <module>
    from kfp.client.client import Client
/usr/local/lib/python3.7/site-packages/kfp/client/client.py:58: in <module>
    class JobConfig:
/usr/local/lib/python3.7/site-packages/kfp/client/client.py:61: in JobConfig
    self, spec: kfp_server_api.ApiPipelineSpec,
E   AttributeError: module 'kfp_server_api' has no attribute 'ApiPipelineSpec'

There are some breaking changes in kfp_server_api I am not aware of. @chensun any suggestions?

@chensun
Copy link
Member

chensun commented Feb 10, 2023

wth

+ pytest /home/prow/go/src/github.com/kubeflow/pipelines/test/sdk-execution-tests/sdk_execution_tests.py --asyncio-task-timeout 2700
============================= test session starts ==============================
platform linux -- Python 3.7.16, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/prow/go/src/github.com/kubeflow/pipelines
plugins: mock-3.8.2, asyncio-cooperative-0.28.0
collected 0 items / 1 error

==================================== ERRORS ====================================
_______ ERROR collecting test/sdk-execution-tests/sdk_execution_tests.py _______
test/sdk-execution-tests/sdk_execution_tests.py:21: in <module>
    from kfp import client
/usr/local/lib/python3.7/site-packages/kfp/__init__.py:23: in <module>
    from kfp.client import Client
/usr/local/lib/python3.7/site-packages/kfp/client/__init__.py:20: in <module>
    from kfp.client.client import Client
/usr/local/lib/python3.7/site-packages/kfp/client/client.py:58: in <module>
    class JobConfig:
/usr/local/lib/python3.7/site-packages/kfp/client/client.py:61: in JobConfig
    self, spec: kfp_server_api.ApiPipelineSpec,
E   AttributeError: module 'kfp_server_api' has no attribute 'ApiPipelineSpec'

There are some breaking changes in kfp_server_api I am not aware of. @chensun any suggestions?

The breaking changes are from kfp-server-api 2.0.0b0. We're working on migrating SDK to use the latest API (ETA: finish by next week).
In the meanwhile, you can keep using kfp-server-api==2.0.0a6 to avoid breaking change, cause I think your change itself is small enough and may not be affected by the API changes.

@elikatsis
Copy link
Member Author

elikatsis commented Feb 10, 2023

@chensun
The thing is that, as stated since quite a few months ago, this PR requires the change of #8511 in the API and a kfp-server-api release that contains it. 2.0.0a6 doesn't have it.

How come there's such a huge change in the package without even a stable release in the meantime? Is this a solid depreciation plan? Removing the deprecated APIs and even the ability to install during a pre-release phase?

Older SDK versions don't work due to these changes, users need to find out the hard way what extra dependencies they need to install (I'm referring to the requirements-deprecated.txt list without an extras_require in the setup.py) or what new version restrictions they need to set.

Is there a way for a user to pull the "old" kfp-server-api package with new features that are getting in with a proper kfp requirement installation (or even some kfp-server-api requirement reference)?

As it seems, we have delivered the API for namespaced pipelines but the client all users use can't have it.

Do you have a detailed plan exposed somewhere? Users use v1 pipelines for their production and they will hardly transition to v2 until quite some time after a GA of v2 pipelines gets out. Since this is far enough in terms of time, it doesn't make sense to make it difficult to use the deprecated v1 API.

@juliusvonkohout
Copy link
Member

@elikatsis interestingly #8831 has just been merged

@juliusvonkohout
Copy link
Member

@StefanoFioravanzo @elikatsis @chensun
can we add this to the agenda for the next kfp meeting?

@chensun
Copy link
Member

chensun commented Mar 29, 2023

@elikatsis, I'm starting the v2 API migration in SDK client, this feature will be covered as part of that. And it would be available in the next SDK beta release.
I had the impression you were okay this being a new feature only available in KFP 2.0 as all other PRs were merged into master branch when it already tracks 2.0-alpha release. Is that still the case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants