-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[MAINTENANCE] Remove Python 3.8 Support (EOL) #10441
Conversation
✅ Deploy Preview for niobium-lead-7998 canceled.
|
ee718ea
to
264436f
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## develop #10441 +/- ##
===========================================
+ Coverage 79.99% 80.09% +0.09%
===========================================
Files 460 461 +1
Lines 40007 39990 -17
===========================================
+ Hits 32005 32028 +23
+ Misses 8002 7962 -40 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -317,6 +317,7 @@ lint.ignore = [ | |||
# https://beta.ruff.rs/docs/rules/#flake8-pyi-pyi | |||
"PYI053", # string-or-bytes-too-long - causes mypy to fail on some of our type stubs | |||
"PYI054", # numeric-literal-too-long - causes mypy to fail on some of our type stubs | |||
"UP035", # TODO: remove once min version of pydantic supports using collections.abc types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This caused issues with our min version of pydantic when using things like collections.abc.Mapping
instead of typing.Mapping
.
I'll do a followup to in-line ignore them instead, I didn't want to do it in this PR because that will cause changes to a heck-ton of files (400+).
setup.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in setup.py
are what actually prevent installs of (new) gx versions with python 3.8.
return_value=( | ||
{}, | ||
{}, | ||
with ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this formatting change intended to be part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's a syntax/formatting change made possible by bumping our min version of python to 3.9 and updating the ruff
"python version target".
@@ -128,7 +128,7 @@ def register_datasource(cls, ds_type: Type[Datasource]) -> None: | |||
) | |||
|
|||
# rollback type registrations if exception occurs | |||
with cls.type_lookup.transaction() as ds_type_lookup, ds_type._type_lookup.transaction() as asset_type_lookup: # noqa: E501 | |||
with cls.type_lookup.transaction() as ds_type_lookup, ds_type._type_lookup.transaction() as asset_type_lookup: # fmt: skip # noqa: E501 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to add # fmt: skip
to get the formatter to chill here 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabling SIM117
didn't do the trick?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's already ignored, but that rule is about multiple nested context managers, not about the formatting with parens. Unless I'm misreading something.
with mock.patch( | ||
"great_expectations.data_context.data_context.abstract_data_context.init_analytics" | ||
) as mock_init, mock.patch("posthog.capture") as mock_submit: | ||
with ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just formatting changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thank you @tyler-hoffman and @Kilo59!
As of October 2024 Python 3.8 has reached End of Life.
https://devguide.python.org/versions/
https://endoflife.date/python
This PR removes Python 3.8 as a supported version and removes all CI related checks for it.
Going forward Python 3.9 is the minimum supported version of Python.
NOTE: Netlify still only supports 3.8 (and 2.7 lol), so its config is still set to 3.8. Sorry.