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: Bump to Python3.10 #24112

Merged

Conversation

EugeneTorap
Copy link
Contributor

@EugeneTorap EugeneTorap commented May 18, 2023

SUMMARY

Running CI integration and unit tests for Py3.10 and Py3.11.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

setup.py Outdated Show resolved Hide resolved
# Conflicts:
#	.github/workflows/docker_build_push.sh
#	Dockerfile
@codecov
Copy link

codecov bot commented Jul 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.72%. Comparing base (c0f8dfc) to head (0651b6b).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #24112      +/-   ##
==========================================
+ Coverage   69.71%   69.72%   +0.01%     
==========================================
  Files        1920     1912       -8     
  Lines       75234    74920     -314     
  Branches     8423     8313     -110     
==========================================
- Hits        52447    52239     -208     
+ Misses      20725    20636      -89     
+ Partials     2062     2045      -17     
Flag Coverage Δ
mysql 77.88% <ø> (-0.01%) ⬇️
postgres 77.99% <ø> (-0.01%) ⬇️
python 82.91% <ø> (-0.01%) ⬇️
sqlite 77.43% <ø> (-0.01%) ⬇️
unit 56.78% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@john-bodley
Copy link
Member

john-bodley commented Jul 15, 2023

@betodealmeida, @eschutho, @michael-s-molina, @villebro et al. do we have a policy in terms of what versions of Python we support (I wasn’t able to find anything in the repo via a quick code search)? If not we probably should formalize this, i.e.,

@betodealmeida
Copy link
Member

@betodealmeida, @eschutho, @michael-s-molina, @villebro et al. do we have a policy in terms of what versions of Python we support (I wasn’t able to find anything in the repo via a quick code search)? If not we probably should formalize this, i.e.,

* The [official supported versions](https://devguide.python.org/versions/#supported-versions) (excluding `main`)

* The non-prerelease official supported versions

* The _n_ latest versions

* …

Agree, @john-bodley, we should have this formalized. My suggestions would be every major version of Superset should support the latest 3 major versions of Python available at the time (so 3.9-3.11 today), and probably when 4.0 is out we'll support 3.10-3.12.

@michael-s-molina
Copy link
Member

My suggestions would be every major version of Superset should support the latest 3 major versions of Python available at the time (so 3.9-3.11 today), and probably when 4.0 is out we'll support 3.10-3.12.

This is a great topic for the next Town Hall @rusackas

@mdeshmu
Copy link
Contributor

mdeshmu commented Oct 7, 2023

@michael-s-molina Is there any decision made on officially supported Python versions? It would be great if this is also officially documented.

If there is no decision made yet then I would like to resubmit this PR. Please let me know if any concerns.

# Conflicts:
#	.github/workflows/docker_build_push.sh
#	requirements/development.txt
#	requirements/testing.txt
@EugeneTorap
Copy link
Contributor Author

EugeneTorap commented Oct 8, 2023

Hi @betodealmeida @john-bodley @michael-s-molina @mdeshmu We have a problem with Py3.11 and prophet==1.1.1 but we can't update the prophet lib because that nasty bug hasn't been fixed yet.
@mdeshmu Can you ask the developers of this prophet library to fix this bug?

@mdeshmu
Copy link
Contributor

mdeshmu commented Oct 8, 2023

@EugeneTorap I will follow up on the bug.

Someone said here that combination of prophet 1.1.4 and holidays 0.32 is working. Can you check that?

Also, there is another potential Redis issue with Python 3.11.

@mdeshmu
Copy link
Contributor

mdeshmu commented Oct 25, 2023

@EugeneTorap prophet 1.1.5 is released and someone said here it works for them: facebook/prophet#2354 (comment)

@EugeneTorap
Copy link
Contributor Author

Thanks @mdeshmu for this information! Can you create a small PR to update the prophet to version 1.1.5?

@mistercrunch
Copy link
Member

Nice! Oh one more thing, can you add an entry on ./UPDATING.md about Python 3.10 being the new minimum python version to run Superset and referencing this PR.

Otherwise LGTM, but for something that important we could use more approvals.

@EugeneTorap
Copy link
Contributor Author

@john-bodley @villebro @dpgaspar @rusackas can you also approve & merge this PR?

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Nice, LGTM!

setup.py Outdated
@@ -208,13 +208,12 @@ def get_git_sha() -> str:
"starrocks": ["starrocks>=1.0.0"],
"doris": ["pydoris>=1.0.0, <2.0.0"],
},
python_requires="~=3.9",
python_requires="~=3.9", # TODO: change it to "~=3.10"
Copy link
Member

Choose a reason for hiding this comment

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

I missed this just now, can we make it

Suggested change
python_requires="~=3.9", # TODO: change it to "~=3.10"
python_requires="~=3.10",

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

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

This is breaking change for folks using Python 3.9. It should become a proposal for the next breaking window.

@michael-s-molina michael-s-molina added the risk:breaking-change Issues or PRs that will introduce breaking changes label Mar 22, 2024
@villebro
Copy link
Member

This is breaking change for folks using Python 3.9. It should become a proposal for the next breaking window.

@michael-s-molina thanks for reviewing, I suspected this would be the case.. In that case, I propose the following:

  • We change this PR to continue support for 3.9
  • Open a new card on the 5.0 board that removes 3.9 when the window opens again, assigning it to @EugeneTorap

Thoughts?

@michael-s-molina
Copy link
Member

We change this PR to continue support for 3.9

That would work.

Open a new card on the 5.0 board that removes 3.9 when the window opens again, assigning it to @EugeneTorap
Thoughts?

Added here.

@mistercrunch
Copy link
Member

mistercrunch commented Mar 22, 2024

Ok, so main / published docker builds start pointing to3.10, and docker-compose / helm chart start pointing to 3.10 as a result,... But we still:

  • officially support 3.9 in setup.py
  • run all or some of the tests on both 3.9 and 3.10?

I noticed we have a lot of "matrix" in GHA that are 3.9 only, would we want to have some or all of those do both? I wish we could DRY those up and have more consistency there.... The fact that this PR touches ~30 files to change the main python version is a bit insane (and due to GHA not having good variable/templating options for this type of stuff)

setup.py Outdated Show resolved Hide resolved
.github/actions/setup-backend/action.yml Outdated Show resolved Hide resolved
scripts/build_docker.py Show resolved Hide resolved
This reverts commit 4538a7b.
setup.py Outdated Show resolved Hide resolved
Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

I made the suggested changes. Surprising how editing on-the-web is pretty viable now with GitHub.

@michael-s-molina I'll need an approval from you to merge since you're still in "Request changes" from a previous review

@michael-s-molina michael-s-molina removed the risk:breaking-change Issues or PRs that will introduce breaking changes label Apr 2, 2024
@mistercrunch mistercrunch merged commit 3a34c7f into apache:master Apr 2, 2024
31 checks passed
@mistercrunch
Copy link
Member

UPDATE YOUR VENVs EVERYONE! :)

giphy

Or just use docker-compose for your dev env

@EugeneTorap EugeneTorap deleted the chore/bump-to-python3.10 branch April 2, 2024 16:57
jzhao62 pushed a commit to jzhao62/superset that referenced this pull request Apr 4, 2024
EandrewJones pushed a commit to UMD-ARLIS/superset that referenced this pull request Apr 5, 2024
EnxDev pushed a commit to EnxDev/superset that referenced this pull request Apr 12, 2024
qleroy pushed a commit to qleroy/superset that referenced this pull request Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Namespace | Anything related to documentation github_actions Pull requests that update GitHub Actions code size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants