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

Remove Python 3.6 #1357

Merged
merged 15 commits into from
Apr 22, 2022
Merged

Remove Python 3.6 #1357

merged 15 commits into from
Apr 22, 2022

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented Dec 11, 2021

I don't think maintaining Python 3.6 is a burden for Starlette (I have a different opinion on Uvicorn tho).

If we remove it on Uvicorn, should we also do it on Starlette?

This PR is to allow discussion having the changes that it implies in the same place. :)

Reference: https://pypistats.org/packages/starlette

Updated 14/01/22: anyio 3.4.0 supports a feature to 3.7+.
Updated 27/03/22: setuptools and jinja2 doesn't support Python 3.6 anymore.

Note to future me: remove jinja2 import conditional about context function.

setup.py Outdated Show resolved Hide resolved
@Kludex
Copy link
Member Author

Kludex commented Dec 11, 2021

Is that a visual bug on the pipeline? Python 3.6 is not running.

@aminalaee
Copy link
Member

aminalaee commented Dec 11, 2021

Is that a visual bug on the pipeline? Python 3.6 is not running.

Isn't it expected since you removed it in your branch? This check won't run in your PR.
I guess it's the same when you add a new Matrix, in that PR the new matrix won't be required since the changes aren't in master.

@Kludex
Copy link
Member Author

Kludex commented Dec 11, 2021

Ok... It makes sense... How do people merge PRs like this one, then? 🤔

@aminalaee
Copy link
Member

Ok... It makes sense... How do people merge PRs like this one, then? 🤔

As far as I remember the pipeline check won't be required, the approval will be enough. Might need double checking...

@Kludex
Copy link
Member Author

Kludex commented Dec 11, 2021

Ok... It makes sense... How do people merge PRs like this one, then? thinking

As far as I remember the pipeline check won't be required, the approval will be enough. Might need double checking...

Don't worry. I just open this for discussion initially. We'll see later. :)

@Kludex Kludex requested a review from a team January 8, 2022 11:03
aminalaee
aminalaee previously approved these changes Jan 8, 2022
@Kludex Kludex mentioned this pull request Jan 8, 2022
8 tasks
@Kludex
Copy link
Member Author

Kludex commented Jan 8, 2022

Only @tomchristie can merge this, as the pipeline will never pass.

@tomchristie The reference about downloads per user is in the description of this PR, jfyk.

@tomchristie
Copy link
Member

I've updated the settings so that 3.7-3.10 are required.

I don't have any strong feelings on this one. I suppose the sensible thing is mostly just that we update our packages consistently when we go for this. (Not for ay hard reasons, just because at least they're all on the same page then.)

What is FastAPI's current version support?

Do we have an outstanding PR for this change in uvicorn too?

@aminalaee
Copy link
Member

aminalaee commented Jan 10, 2022

Sorry to jump in here, I don't know about FastAPI's plans for python 3.6 but this pypistats might be worth considering:
Screenshot 2022-01-10 at 10 46 20

Update: I'm not sure how accurate the numbers are but ignoring the recent peaks, there a few thousand downloads of Python3.6 happening every day.

@tomchristie
Copy link
Member

Link to those FastAPI stats... https://pypistats.org/packages/fastapi

@Kludex Kludex dismissed aminalaee’s stale review January 10, 2022 10:19

Let's wait some time before evaluating this again

@Kludex
Copy link
Member Author

Kludex commented Jan 10, 2022

I've talked to @tiangolo about this, and we reached the conclusion that we can wait some time before losing support for Python 3.6, so people can migrate to a new Python version.

We can reevaluate the numbers some time from now.

@tiangolo
Copy link
Member

Yep! As @Kludex says, if possible, and if it's not being cumbersome to maintain, I would hold this for some time. 🙏

I assume there might already be "old" apps that can't migrate to 3.7 right away. If it gets annoying/cumbersome then yeah. But keeping support for 3.6 for a while might be appreciated by a few developers already stressed that their other libs don't support > 3.6 yet. 😅

@Kludex
Copy link
Member Author

Kludex commented Feb 1, 2022

anyio 4.0 will drop Python 3.6, jfyk.

@adriangb adriangb added the clean up Refinement to existing functionality label Feb 2, 2022
@Kludex
Copy link
Member Author

Kludex commented Mar 1, 2022

Trio dropped support for Python 3.6.

@Kludex
Copy link
Member Author

Kludex commented Mar 1, 2022

Not important on the dependency tree, but coverage dropped support as well.

@Kludex
Copy link
Member Author

Kludex commented Apr 2, 2022

pytest dropped support as well.

@Kludex
Copy link
Member Author

Kludex commented Apr 2, 2022

Daily downloads from PyPI Download Stats about Starlette:

3.8: ~95k
3.7: ~78k
3.9: ~44k
3.10: ~16k
3.6: ~10k

3.18% of our users use Python 3.6.

Ref.: https://pypistats.org/packages/starlette (date of the info above is 29 March 2022)

Based on this, I'd like to go with this idea.

@Kludex Kludex requested a review from a team April 2, 2022 12:31
@Kludex Kludex requested a review from tomchristie April 6, 2022 07:01
Copy link

@br3ndonland br3ndonland left a comment

Choose a reason for hiding this comment

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

Thank you for the thoughtful, data-driven approach to this update. 📈 👀 👍

@Kludex Kludex added this to the Version 0.20.0 milestone Apr 21, 2022
README.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
@Kludex Kludex merged commit ce0709d into master Apr 22, 2022
@Kludex Kludex deleted the 3.6-removal branch April 22, 2022 05:47
@hugovk hugovk mentioned this pull request May 1, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean up Refinement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants