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

chore(sdk): drop Python 3.7 for kfp sdk #10750

Merged
merged 29 commits into from
May 3, 2024

Conversation

rickyxie0929
Copy link
Contributor

@rickyxie0929 rickyxie0929 commented Apr 25, 2024

Description of your changes:
chore(sdk): drop Python 3.7 for kfp sdk

Checklist:

rickyxie0929 and others added 12 commits April 23, 2024 16:23
PiperOrigin-RevId: 627500444

**Description of your changes:**

**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](https://github.com/kubeflow/pipelines/blob/master/CONTRIBUTING.md#pull-request-title-convention).
<!--
   PR titles examples:
    * `fix(frontend): fixes empty page. Fixes kubeflow#1234`
       Use `fix` to indicate that this PR fixes a bug.
    * `feat(backend): configurable service account. Fixes kubeflow#1234, fixes kubeflow#1235`
       Use `feat` to indicate that this PR adds a new feature.
    * `chore: set up changelog generation tools`
       Use `chore` to indicate that this PR makes some changes that users don't need to know.
    * `test: fix CI failure. Part of kubeflow#1234`
        Use `part of` to indicate that a PR is working on an issue, but shouldn't close the issue when merged.
-->
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
Copy link
Member

@connor-mccarthy connor-mccarthy left a comment

Choose a reason for hiding this comment

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

I think the title of this PR is out-of-date since the changes are for the KFP SDK rather than GCPC SDK. Can we also use the Pull Request Title Convention?

sdk/python/kfp/__init__.py Outdated Show resolved Hide resolved
sdk/python/kfp/__init__.py Show resolved Hide resolved
Signed-off-by: rickyxie0929 <[email protected]>
@google-oss-prow google-oss-prow bot added size/S and removed size/XS labels Apr 25, 2024
Signed-off-by: rickyxie0929 <[email protected]>
@rickyxie0929
Copy link
Contributor Author

/retest

@rickyxie0929
Copy link
Contributor Author

/retest-required

@rickyxie0929
Copy link
Contributor Author

/restest

@rickyxie0929
Copy link
Contributor Author

/test kubeflow-pipelines-sdk-execution-tests

Signed-off-by: rickyxie0929 <[email protected]>
Signed-off-by: rickyxie0929 <[email protected]>
@google-oss-prow google-oss-prow bot added size/XL and removed size/S labels May 2, 2024
sdk/RELEASE.md Outdated Show resolved Hide resolved
Signed-off-by: rickyxie0929 <[email protected]>
Copy link
Member

@connor-mccarthy connor-mccarthy left a comment

Choose a reason for hiding this comment

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

What is part 2 of this PR? I see the title says "(part 1)".

@@ -32,7 +32,7 @@ deploymentSpec:
'
- "\nimport kfp\nfrom kfp import dsl\nfrom kfp.dsl import *\nfrom typing import\
\ *\n\ndef comp():\n pass\n\n"
image: python:3.7
image: python:3.8
Copy link
Member

Choose a reason for hiding this comment

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

I think the files under kubernetes_platform/ are for kfp-kubernetes. Should we do that in a separate change?

@@ -88,4 +88,4 @@ root:
isOptional: true
parameterType: STRING
schemaVersion: 2.1.0
sdkVersion: kfp-2.0.0-rc.2
Copy link
Member

Choose a reason for hiding this comment

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

nit: these changes aren't strictly necessary since they don't affect the Python version. We can leave them but FYI.

@rickyxie0929 rickyxie0929 changed the title chore(sdk): drop Python 3.7 for kfp sdk(part 1) chore(sdk): drop Python 3.7 for kfp sdk May 2, 2024
sdk/RELEASE.md Outdated Show resolved Hide resolved
Signed-off-by: rickyxie0929 <[email protected]>
Copy link

@rickyxie0929: 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
kfp-kubernetes-test-python37 926353e link true /test kfp-kubernetes-test-python37
kubeflow-pipelines-sdk-python37 926353e link true /test kubeflow-pipelines-sdk-python37
test-kfp-runtime-code-python37 926353e link true /test test-kfp-runtime-code-python37
kubeflow-pipeline-e2e-test 20ab4d8 link true /test kubeflow-pipeline-e2e-test
kfp-kubernetes-execution-tests 20ab4d8 link false /test kfp-kubernetes-execution-tests

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.

Copy link
Member

@chensun chensun left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Thanks!

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

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:

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

@rickyxie0929
Copy link
Contributor Author

/retest-required

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