Skip to content

drop python 2 support #1468

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 17, 2021
Merged

drop python 2 support #1468

merged 2 commits into from
May 17, 2021

Conversation

Priyankasaggu11929
Copy link
Contributor

@Priyankasaggu11929 Priyankasaggu11929 commented May 14, 2021 •
edited
Loading

What type of PR is this?

/kind feature
/kind clean-up

What this PR does / why we need it:

The PR makes changes to drop support for Python 2 from the next release (v18.0.0). The changes includes:

  • drop Python 2 from master branch
  • make sure all the tests use Python 3

Which issue(s) this PR fixes:

Fixes #1413

Does this PR introduce a user-facing change?

The `python2` support will be removed in 18.0.0 beta release. All the tests will use `python3` versions.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v1700-snapshot

Signed-off-by: Priyanka Saggu priyankasaggu11929@gmail.com

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2021
@k8s-ci-robot
Copy link
Contributor

@Priyankasaggu11929: The label(s) kind/clean-up cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?

/kind feature
/kind clean-up

What this PR does / why we need it:

The PR makes changes to drop support for Python 2 from the next release (v18.0.0). The changes includes:

  • drop Python 2 from master branch
  • make sure all the tests use Python 3
  • drop Python 2 from v18 beta release.

Which issue(s) this PR fixes:

Fixes #1413

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v1700-snapshot

Signed-off-by: Priyanka Saggu priyankasaggu11929@gmail.com

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.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 14, 2021
@k8s-ci-robot k8s-ci-robot requested review from roycaihw and yliaog May 14, 2021 14:28
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 14, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the drop-python-2-support branch 3 times, most recently from db5f1fc to c49368b Compare May 14, 2021 15:20
@Priyankasaggu11929
drop python2 support from k8s-client/python
4431c4d 
Signed-off-by: Priyanka Saggu <priyankasaggu11929@gmail.com>
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the drop-python-2-support branch 2 times, most recently from 16986e3 to 4a2a030 Compare May 14, 2021 18:40
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 14, 2021
@Priyankasaggu11929
Copy link
Contributor Author

Priyankasaggu11929 commented May 14, 2021 •
edited
Loading

@yliaog @roycaihw

Need advice here: https://travis-ci.org/github/kubernetes-client/python/jobs/771168725

I'm trying to update the scripts/update-pycodestyle.sh test file to make it compatible with python3 versions. To begin testing, I'm starting with python version 3.9

From the .travis.yaml file

  - stage: test
    python: 3.9
    env: TOXENV=update-pycodestyle

The pycodestyle when ran with python v3.9 is throwing errors & suggesting changes in the python scripts (for ex - in case of above failing job, it is prompting for changes in examples/multiple_clusters.py python file)

  • Q 1: So, I want to confirm that the right way to update the scripts/update-pycodestyle.sh (& other tests) would be step by step correcting the example files until it goes all green for python v3.9? right?

    • Correcting the example files made the job run successfully. So, I'm feeling yes, I need to keep correcting the example files wherever it breaks by applying suggested fixes.

  • Q 2: I would have to add the update-pycodestyle check for all the current set of python3 versions ( py3{5,6,7,8,9} )?

  • Q 3: What's the difference between py3{5,6,7,8,9} & py3{5,6,7,8,9}-functional?

Thank you!

@Priyankasaggu11929 Priyankasaggu11929 force-pushed the drop-python-2-support branch 2 times, most recently from 7c1c0c3 to 81c65bf Compare May 14, 2021 19:19
@Priyankasaggu11929
drop python2 support
6f09733 
  - remove python2 from the .travis.yaml file
  - remove python2 from the tox.ini file
  - remove `-y` flag from isort command in update-pycodestle.sh script
  - fix sequence of module imports to fix faiing pycodestyle check in the following files:
    - examples/multiple_clusters.py
    - examples/pick_kube_config_context.py
    - examples/pod_config_list.py
  - testing coverage & codecov tests with python3
  - testing `update-pycodestyle` for python v3.9 and arch ppc6le

Signed-off-by: Priyanka Saggu <priyankasaggu11929@gmail.com>
@Priyankasaggu11929 Priyankasaggu11929 mentioned this pull request May 14, 2021
Merged
2 tasks
@Priyankasaggu11929
Copy link
Contributor Author

Priyankasaggu11929 commented May 17, 2021 •
edited
Loading

@roycaihw , @yliaog

[Update]

So far, I have done the following to remove Python 2 support:

  • removed python2 from the .travis.yaml file
  • removed python2 from the tox.ini file
  • removed python2 from the .github/workflows/test.yaml file
  • removed -y flag from isort command in update-pycodestle.sh script (as it was throwing error for unknown flag -y while executing it with python3 versions)
  • fixed sequence of module imports in the following files, to fix the failing pycodestyle checks:
    • examples/multiple_clusters.py
    • examples/pick_kube_config_context.py
    • examples/pod_config_list.py
  • tested coverage & codecov tests with python3 version (currently v3.9)
  • tested updated update-pycodestyle.sh for python3 version v3.9 and architecture type, ppc6le

With these much changes, the CI jobs are green so far.

yliaog
yliaog reviewed May 17, 2021
View reviewed changes
Copy link
Contributor

@yliaog yliaog left a comment

Choose a reason for hiding this comment

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

please remove '[WIP]'

.travis.yml
env: TOXENV=update-pycodestyle
arch: ppc64le
arch: ppc64le
Copy link
Contributor

Choose a reason for hiding this comment

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

looks there are extra spaces at the end, please remove them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the spaces. Thanks!

scripts/update-pycodestyle.sh
@@ -67,7 +67,7 @@ done

echo "--- applying isort"
for SOURCE in $SOURCES; do
isort -y $SOURCE
isort $SOURCE
Copy link
Contributor

Choose a reason for hiding this comment

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

it should have been called before with python3, not sure why there was no errors before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to confirm, there is no action required here, right?

Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

no action, just wondering if you know why there was no error before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were no previous test running update-pycodestyle.sh script with python3.x versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, that explains it.

@Priyankasaggu11929 Priyankasaggu11929 changed the title [WIP] drop python 2 support drop python 2 support May 17, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 17, 2021
@yliaog
Copy link
Contributor

yliaog commented May 17, 2021

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Priyankasaggu11929, yliaog

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 17, 2021
@Priyankasaggu11929
Copy link
Contributor Author

Also, re-iterating over questions asked in this comment #1468 (comment) :

Q 2: Do I require to add the update-pycodestyle.sh check for all the current set of python3 versions ( py3{5,6,7,8,9} )?

Q 3: What's the difference between py3{5,6,7,8,9} & py3{5,6,7,8,9}-functional?

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 17, 2021
@k8s-ci-robot k8s-ci-robot merged commit 821618f into kubernetes-client:master May 17, 2021
@yliaog
Copy link
Contributor

yliaog commented May 17, 2021

sorry, missed your prior questions.
Q2: you only need to run one python3 version, say python 37, as all python3 versions share one common style guide

Q3: functional has e2e tests, the commands to show the difference is here:

python/tox.ini

Line 15 in 821618f

functional: {toxinidir}/scripts/kube-init.sh pytest -vvv -s []

@Priyankasaggu11929
Copy link
Contributor Author

Thank you @yliaog. That was really helpful clarification. :)

@jonasdlindner jonasdlindner mentioned this pull request Jun 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yliaog yliaog

@roycaihw roycaihw

Assignees

@yliaog yliaog

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drop Python 2 support
3 participants
@Priyankasaggu11929 @k8s-ci-robot @yliaog