-
Notifications
You must be signed in to change notification settings - Fork 565
Maintain compatibility with CPython 3.13 #1242
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
Conversation
@daskol You need to edit the files that use asyncore (Removed in python 3.13) to make it use asyncio
Maybe be you could edit the files to use asyncore or asyncio by case (python>=3.13 or not) |
@daskol and even after I edit for some reason I can't import the cassandra.io.libevwrapper |
@medaminezghal What about |
@daskol if you build the package without the C extensions it should work (of course after editing the files that use asyncore) and then you can build cqlsh in python 3.13 |
@daskol you can replace any code that use asyncore with asyncio safely https://docs.python.org/3/library/asyncore.html |
Do |
I think it’s not useful to use unmaintained package as provided in the description. So it’s better to use asyncio |
@daskol I think you need to fix the problem related to this issue from other project. |
Thanks for the PR @daskol! Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks! I mentioned this in another recently submitted PR but it's worth restating here: because the Python driver aims to support all Python runtimes that aren't end-of-life we still have to support Python 3.9.x and up. And since the asyncore reactor is more stable than the current asyncio version I have no plans to remove it anytime soon. There's a lot more detail in my comment. For the record: if you want to use the libev reactor you'll need libev installed and accessible via the lib path before the Python driver will even attempt to use it. You can find some additional info in our docs on installing the Python driver. |
@absurdfarce Yes, I signed agreement as soon as you posted this requirement in #1244.
This PR exactly improves support for CPython 3.9+. |
@daskol since the driver only supports 3.8+, is there any reason the deprecated initializer code cannot be simply removed entirely? Note: 3.8 was EOL on Oct 2024, so should really be 3.9+ |
@daskol is there any reason the deprecated initializer code cannot be simply removed entirely? |
Could this be merged relatively soon and a new release created with this fix? Python 3.13 is sort of broken because of this. |
I agree with this PR but it's worth stating that Python 3.13.x isn't "broken" without this change. Plenty of gcc instances will just report these as warnings about undefined method calls without breaking anything. In fact that's exactly what our Jenkins build does against Python 3.13:
That's against Focal which is using 9.3.0 IIRC. I agree it's not great but it's not the case that Python 3.13.x support is broken across the board. Also worth mentioning that the wheels for 3.29.2 work just fine. Those were also built on Focal runners. |
I'm not unsympathetic to your argument that we should remove these initializers entirely @bschoening but at the moment my inclination is to go with something like what's in this PR. Taking this approach rather than removing the calls entirely at least allows for the possibility that you could build this against something other than the officially supported versions of Python. We're obviously not changing the Python versions we support and I don't want to get in the habit of bending the rules to keep older versions happy... but in this case we have a completely acceptable PR which enables that kind of backwards support. I readily agree it's a judgement call but for now I'm inclined to go with it. This support will likely go away at some point in the future... but for now it seems like a reasonable compromise. |
Kicked off a Jenkins build just to make sure there aren't any unexpected regressions here. Hard to see how there could be but, you know... computers and all that. |
@absurdfarce, build does work (without building
I guess extra packages needed to be installed to avoid this crash, but they are not declared as dependencies. |
Hey @andy-slac, thanks for the clarification! Yeah, you're seeing those errors because libev itself isn't available on your box. The libevwrapper code referenced in this PR provides interop between the driver and libev but we still require the library itself. This is mentioned in the installation docs, specifically the section on libev. For what it's worth part of the motivation for my earlier comments was the mess around asyncio support in Python 3.12 and up. There's been a lot of confusion around that point and we've indicated to a number of folks that libev is the recommended reactor for Python 3.12+ (until we can get to a prod-ready asyncio reactor). So I didn't want users to see a suggestion that libev wasn't functionality on Python 3.13 without providing the additional context. |
@absurdfarce,
|
@andy-slac I'm not 💯 sure what the root cause might be in your case but a couple things do occur to me. First, you need to have both libev and it's corresponding dev package installed; without the dev package you won't get the headers necessary to build everything. Second, the directories where libev is installed on your system might not be on the default search path. setup.py will search a set of default paths to try and find libev (details here) and at least the paths you mention in your comment are different from those defaults. You can always try defining CASS_DRIVER_LIBEV_INCLUDES and CASS_DRIVER_LIBEV_LIBS to point to the correct directories on your system to see if that helps. |
Jenkins run has finished and looks good, saw improvement in the expected area around the libev extension build on Python 3.13. No other errors were introduced. |
@absurdfarce, in my case everything was installed properly, but wrapper extension failed to build because gcc generated an error for an implicit declaration of
Thanks for merging the patch, when should we expect a new release? |
Should this be PY_MAJOR_VERSION == 3? Isn't this checking for version < 2.7? |
Great catch @bschoening... you're absolutely right. I'm angry at myself for missing that! Changing that to:
in a ninja fix. I'll then run the full test suite on it just to make sure it's good. |
Fix in a5c6c5f |
Jenkins run is clean, calling this good |
See changes in Python C API in CPython 3.13 changelog. Initial discussion is in AUR.