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

ci: bump Python tests to 3.7 and add support for 3.8 #10110

Merged
merged 1 commit into from Aug 5, 2020

Conversation

villebro
Copy link
Member

SUMMARY

With the bump of pylint to version 2.5.3 per #10101, the Superset build env should now be fully 3.7+ compatible.

TEST PLAN

Run all python tests on 3.7 and ensure CI passes smoothly.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@villebro
Copy link
Member Author

I'm flagging this as a 0.37 dependency, as I feel adding official support for py38 is important as it's already in production use in some large deployments. If changing the test matrix is too much I can remove those for now, but I'd really like for us to consider py38 officially supported.

@ktmud
Copy link
Member

ktmud commented Jun 19, 2020

Are we dropping support for Python 3.6 yet?

@villebro
Copy link
Member Author

I propose we don't, but next release I think we should.

@villebro villebro changed the title [WIP] feat: bump Python tests to 3.7 and add support for 3.8 chore: bump Python tests to 3.7 and add support for 3.8 Jul 5, 2020
@villebro
Copy link
Member Author

villebro commented Jul 5, 2020

@mistercrunch how do you feel about moving tests to py37? If this is ok, who can change the required status checks to reference the 3.7 tests?

@etr2460
Copy link
Member

etr2460 commented Jul 10, 2020

After discussion today, folks were in favor of moving to python 3.7 as the supported version and removing official support for Python 3.6 after the 0.37 release

@bkyryliuk
Copy link
Member

looking forward to it !

@mistercrunch
Copy link
Member

mistercrunch commented Jul 15, 2020

I think it's ok for this project to be prescriptive of the python version (being an app, an not a library). It makes it easier on us as the build matrix can be focussed on a single version, the docker can be aligned with that version, and that everyone running Superset in production should align with the prescribed version for that release. So say if it's 3.7 after this version, it's "run on anything else at your own risk".

The alternative is to cover a version range of python, and to do that well we need to explode our build matrix. After the whole 2.7 to 3.6 fiasco I'd rather stay away from being straddling across python versions.

Copy link
Member

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

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

lgtm with one comment

@@ -133,6 +133,7 @@ def get_git_sha():
classifiers=[
"Programming Language :: Python :: 3.6",
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove this line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's keep it there a while longer and drop it in a separate PR where dataclasses and other backwards compatibility libs are also dropped.

@mistercrunch
Copy link
Member

Close/reopened to re-trigger CI that was stuck

@villebro villebro removed the v0.37 label Jul 26, 2020
@willbarrett
Copy link
Member

@villebro I'm thinking we need to ask Apache to update the required checks to 3.7, or temporarily also run against 3.6

@villebro
Copy link
Member Author

villebro commented Aug 3, 2020

Ok @willbarrett , let's temporarily enable 3.6 and 3.7 and disable 3.6 once Apache updates. I'll make the change and reach out to ASF infra.

Edit: decided against it to avoid slowing down CI.

@villebro villebro changed the title chore: bump Python tests to 3.7 and add support for 3.8 ci: bump Python tests to 3.7 and add support for 3.8 Aug 5, 2020
@villebro
Copy link
Member Author

villebro commented Aug 5, 2020

ASF Infra ticket: https://issues.apache.org/jira/browse/INFRA-20643

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2020

Codecov Report

Merging #10110 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10110      +/-   ##
==========================================
+ Coverage   63.62%   63.65%   +0.03%     
==========================================
  Files         764      764              
  Lines       36158    36158              
  Branches     3438     3438              
==========================================
+ Hits        23004    23017      +13     
+ Misses      13041    13028      -13     
  Partials      113      113              
Flag Coverage Δ
#cypress 54.50% <ø> (ø)
#javascript 59.92% <ø> (ø)
#python 59.09% <ø> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
superset/connectors/sqla/models.py 89.54% <0.00%> (+0.14%) ⬆️
superset/views/core.py 74.33% <0.00%> (+0.25%) ⬆️
superset/models/core.py 87.22% <0.00%> (+0.83%) ⬆️
superset/db_engine_specs/mysql.py 91.48% <0.00%> (+12.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57dc762...293bf6a. Read the comment docs.

@villebro villebro merged commit 3983fff into apache:master Aug 5, 2020
@villebro villebro deleted the villebro/bump-py37 branch September 6, 2020 17:03
Ofeknielsen pushed a commit to ofekisr/incubator-superset that referenced this pull request Oct 5, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 0.38.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants