Skip to content

Chore: Drop support for Python 3.8 #3399

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jomcgi
Copy link
Contributor

@jomcgi jomcgi commented Apr 1, 2025

Description

Python 3.8 was EoL @ 2024-10-07, our 6 month promise for support will end @ 2024-04-07.

Changes:

  • Removed all 3.8 references, updated basline refs to 3.9
  • Removed 3.8 sys.version_info checks
  • Fixed botocore test failure
  • Applied ruff formatting

Fixes open-telemetry/opentelemetry-python#4513

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Tested locally with tox
  • Tested in CI

Does This PR Require a Core Repo Change?

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch from 26c0f26 to 71eb61e Compare April 1, 2025 20:49
Comment on lines +148 to 153
# Starlette instrumentation is pinned to <0.15 from 2021-06-23
# This creates a dependency conflict with instrumentation-genai/opentelemetry-instrumentation-google-genai
# TODO: remove exclusion when #3317 is resolved.
exclude = [
"instrumentation-genai/opentelemetry-instrumentation-google-genai",
"instrumentation/opentelemetry-instrumentation-starlette",
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think flipping this exclusion makes more sense given how outdated the starlette instrumentation is.

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 is going to be resolved in the fastapi middleware PR we have open

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure the fastapi middleware PR will solve that, but I'm ok with excluding it. We'll probably need to exclude it in the fastapi PR as well to make CI pass.

@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch 2 times, most recently from fba15e8 to e2360f6 Compare April 1, 2025 22:44
tox.ini Outdated
Comment on lines 204 to 205
# excluded from pypy3 due to missing wheel
pypy3.10-test-exporter-prometheus-remote-write
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pypy3.9 is missing a wheel for cramjam causing this test to fail, if we feel it's necessary we could reopen milesgranger/cramjam#185

op.operation_name(): op
for op in globals().values()
if inspect.isclass(op)
and hasattr(op, "operation_name")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This condition was added here, lmdb.py and sns.py to resolve a test failure where inspect.isclass(op) returned true but issubclass(op, ... failed due to a value from globals not being recognized as a class.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering why they are not failing now though

@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch from e2360f6 to 4f57fa3 Compare April 1, 2025 23:00
@@ -59,7 +60,7 @@


class Boto3SQSGetter(Getter[CarrierT]):
def get(self, carrier: CarrierT, key: str) -> Optional[List[str]]:
def get(self, carrier: CarrierT, key: str) -> Optional[list[str]]:
Copy link
Contributor

@dimastbk dimastbk Apr 2, 2025

Choose a reason for hiding this comment

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

Maybe use from __future__ import annotations whenever possible? Upgrading a code from 3.9 to 3.10 (and 3.11-3.13) will not require many changes.

Maybe https://docs.astral.sh/ruff/rules/future-rewritable-type-annotation/ can help with it.

Copy link
Member

Choose a reason for hiding this comment

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

+1 I would prefer not touching too much code on that PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor Author

@jomcgi jomcgi Apr 16, 2025

Choose a reason for hiding this comment

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

After I followed through with these changes (pyupgrade, ruff/isort enforcing future annotations) the blast radius was massive.
I've removed type changes from this PR as that change does not need to be coupled to deprecating python 3.8.
I can follow-up with the changes I had in smaller separate PRs.

Issue to track enabling the checks: #3428

@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch from 4f57fa3 to 415fb0c Compare April 4, 2025 08:33
@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch 2 times, most recently from 5e91bf0 to cbac8ca Compare April 4, 2025 09:07
Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

I would have preferred to decouple bumping the baseline to the changes to the other hundreds of files 😅

@jomcgi
Copy link
Contributor Author

jomcgi commented Apr 11, 2025

I would have preferred to decouple bumping the baseline to the changes to the other hundreds of files 😅

Agree - it's why this is still in draft.
Most of the type changes can be submitted separately, this definitely doesn't need to be coupled.

@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch 3 times, most recently from 81f758e to b5dbc7b Compare April 16, 2025 08:24
Python 3.8 was EoL @ 2024-10-07, our 6 month promise for support ended on 2024-04-07.

Changes:
* Updated basline refs to 3.9
* Removed 3.8 sys.version_info checks
* Fixed botocore test failure
* Applied ruff formatting
@jomcgi jomcgi force-pushed the chore-4513/drop-py38-support branch from b5dbc7b to 7af57d4 Compare April 16, 2025 08:38
@jomcgi jomcgi changed the title [WIP] Chore: Drop support for Python 3.8 Chore: Drop support for Python 3.8 Apr 16, 2025
@jomcgi jomcgi marked this pull request as ready for review April 16, 2025 08:46
@jomcgi jomcgi requested a review from a team as a code owner April 16, 2025 08:46
@@ -19,7 +19,6 @@ classifiers = [
"Programming Language :: Python",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
Copy link
Member

Choose a reason for hiding this comment

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

We need to revisit this later 😓 we don't even support 3.7.

@@ -19,7 +19,6 @@ classifiers = [
"Programming Language :: Python",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Programming Language :: Python :: 3.7",

Copy link
Member

Choose a reason for hiding this comment

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

Need to update the requires-python >= 3.7 too

@@ -48,6 +48,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([[#3332](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3332)])


### Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Remind to update this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop support for Python 3.8