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

[ZEPPELIN-5979] Support and use Python 3.9 in default #4690

Merged
merged 1 commit into from Nov 15, 2023

Conversation

pan3793
Copy link
Member

@pan3793 pan3793 commented Nov 9, 2023

What is this PR for?

Currently, Zeppelin only tests with Python 3.7(default) and 3.8.

According to https://endoflife.date/, Python 3.7 is EOL. This PR adds the Python 3.9 test with minimal tunes. Although Python 3.12 is available, I conservatively just moved to Python 3.9 for safety (I'm not a Python expert), also Python 3.9 is the first version officially supporting Apple Sillicon.

What type of PR is it?

Improvement

Todos

  • - Task

What is the Jira issue?

ZEPPELIN-5979

How should this be tested?

Pass GA.

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@@ -35,4 +36,4 @@ dependencies:
- r-irkernel
- r-shiny
- r-googlevis
- tensorflow=1.13
- tensorflow
Copy link
Member Author

Choose a reason for hiding this comment

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

It's only used in submarine testing.

BTW, does the community consider removing submarine support?

  • submarine was listed at here with no clear decision
  • there is no functionality change / bugfix in submarine module since 2020
  • submarine already provided the built notebook in the new version, and submarine docs does not mention zeppelin

thus, I suppose we can remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @jongyoul @Reamer @xunliu

I think it's better to discuss the proposal with Liu Xun first.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not use submarine. As long as it doesn't cause any problems, we should keep it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Reamer OK, thanks for your advice.

@pan3793
Copy link
Member Author

pan3793 commented Nov 10, 2023

also cc @zjffdu and @huage1994

python: [ 3.9 ]
flink: [116, 117]
include:
# Flink 1.15 supports Python 3.6, 3.7, and 3.8
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment needs to be updated

Copy link
Member Author

Choose a reason for hiding this comment

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

why? it's true

Copy link
Contributor

Choose a reason for hiding this comment

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

Umm, maybe I am nitpicking. I thought you were going to test all the three python versions, following the comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Flink 1.15 is an exception, it does not work with Python 3.9, that's why I leave comments to explain that.

@@ -35,4 +36,4 @@ dependencies:
- r-irkernel
- r-shiny
- r-googlevis
- tensorflow=1.13
- tensorflow
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @jongyoul @Reamer @xunliu

I think it's better to discuss the proposal with Liu Xun first.

@@ -342,8 +353,13 @@ jobs:
strategy:
fail-fast: false
matrix:
python: [ 3.7, 3.8 ]
python: [ 3.9 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

why not modify to [ 3.7, 3.8, 3.9 ] ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we don't need to test all combinations of Java and Python.

all_java_version * default_python_version plus default_java_version * all_python_version_except_default_one is sufficient.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need to test all combinations of Java and Python.

all_java_version * default_python_version plus default_java_version * all_python_version_except_default_one is sufficient.

Thank you very much for explaining.

@huage1994 huage1994 merged commit 73e1aa3 into apache:master Nov 15, 2023
31 checks passed
akoira pushed a commit to akoira/zeppelin that referenced this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants