-
Notifications
You must be signed in to change notification settings - Fork 626
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
Add support for Python 3.13 #2724
base: main
Are you sure you want to change the base?
Add support for Python 3.13 #2724
Conversation
Do they work locally? I think it's premature to add them on CI since we are depending on other packages support and wheels (they removed some C-API symbols) more than something on our side. |
Makes sense. @xrmx Would it be desirable to have a separate job that runs the tests only against the latest Python pre-release that is not mandatory so we get a heads-up if we will brake in upcoming releases of Python? |
I think CI is already slow enough :) |
I mean, the tests will have to be added eventually, so we're probably not getting around that. :) Is there anything in particular that is slow? Is it a concurrency issue, i.e., not enough runners? Would clustering tests somehow help? I can take a look if there's anything that I see that could be improved (appreciate any pointers about current pain points there!). I think getting tests to run against 3.13 RCs would be great for uncovering issues early. I of course get that that's not always possible, especially if there are dependencies. Could we start by testing some core part of the codebase that isn't blocked? I.e., not the instrumentations? |
The current pain point I think is that the checkout of core libraries from git is slow. There is a PR switching to uv that should help #2667
I'm not doubting it's useful. They are less useful if our instrumented libraries does not yet work with python 3.13 though. Some comments ago I asked if you have run them locally, if everything is fine (modulo you'll need the same exclusions we have for 3.12 in the workflows) then it's fine to add them. If we need to add temporary workarounds because packages don't have wheels already then I would prefer to wait at least for a final 3.13. |
Co-authored-by: Ivana Kellyer <[email protected]>
Co-authored-by: Ivana Kellyer <[email protected]>
Looks like we are reaching some kind of limit and some tests won't run? Anyway there is at least pydantic to bump. |
I do not know about the limits, sorry. |
I ran all the test suites using Python 3.13.0b1 and those are the ones that are failing:
|
So a question @xrmx : will otel only support Python 3.13 when all the instrumentation's work with 3.13? (could take a while for all those libraries to add support for 3.13) |
I know there will be a lot of work needed to make opentelementry-python-contrib compatible with Python 3.13. @xrmx You can close this PR if it cluttering the prs. We can reopen at a later time. I guess the way to approach this would be:
|
I think this is the wrong question to ask :) Also it's not that I decide here, I'll try to help :)
The correct question in my opinion would be: how can I help to have 3.13 supported when it will be released? Your list looks fine to me, the actual timeline depends on people doing the work. So first thing would be to understand why things are failing. For 3.12 we did a big "add support for 3.12", but there were a few things that can be fixed separately and we did so. For -contrib at least 3 people opened a PR to add 3.12 support. To the list of things to do there should be probably be to being able to run 3.13 tests in CI 😅 Maybe #2687 will do the trick. To elaborate a bit on the possible failures in tests, for 3.12 there were things that the python interpreter become more picky like warning about wrong assert methods. And these can be fixed right now. Another thing could be missing wheels / language compatibility issues, and as I said already I would avoid to add temporary workarounds for these but wait for updated packages instead. |
Co-authored-by: Ivana Kellyer <[email protected]>
Co-authored-by: Ivana Kellyer <[email protected]>
Updated the branch to use the new workflow generation script. When running the tests locally with @xrmx could you trigger the CI run, so we can check? |
All GH checks (except the changelog one) succeeded. (fixed this in the mean time) So I guess this is good to review! |
Shouldn't we support Python 3.13 in the api before we add support for the instrumentations? |
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.
I like the idea of testing new releases and fixing the errors early, but I'm afraid users think we are supporting py3.13 with this, and at some point, we need to do workarounds to keep things working. Maybe can we just move forward with the idea of using this PR as a tracking point to see/test which instrumentations break and map what we can do to help otel-python support python3.13 when it gets released?
@antonpirker please clean your base branch so you don't have other people commits, thanks! Then it would be nice to open a new PR with the following commits so we can reduce this PR to just the enablement: |
@@ -28,6 +28,9 @@ jobs: | |||
uses: actions/setup-python@v5 | |||
with: | |||
python-version: "{{ job_data.python_version }}" | |||
{%- if job_data.python_version == "3.13" %} |
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.
It looks like we can drop this?
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. we can drop this now. https://github.com/actions/setup-python/pull/958/files#diff-194218c48b9a0cdd03974145733804c2d992ca818529fe2fa69a501d8b5b1cc3R70
@antonpirker are you open to continue the fixes here?
@@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|||
### Fixed | |||
|
|||
- Add Python 3.13 support |
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.
update here too to the correct section
Description
Add support for Python 3.13 to all instrumentations where the instrumented library already support Python 3.13.
Updated
tox.ini
and also the workflow generation script. So for all instrumentation that support Python 3.13 already the tests will also run in CI.To make the tests pass I had to update some
test-requirements
. But only helper libs and not the libraries under test itself.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I ran the all instrumentation tests in Python 3.13 locally, and those are the libraries that do not support Python 3.13 yet. Those CI workflows have not yet updated to run under Python 3.13:
Note on
py313-test-instrumentation-system-metrics
: I think this can be fixed to be compatible to Python 3.13 because only the tests fail (but the tox environment is built and the tests run) But I have not enough knowledge yet to fix those two failing tests.Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.