-
Notifications
You must be signed in to change notification settings - Fork 568
CASSPYTHON-3 Introduce pyproject.toml to explicitly declare build dependencies #1264
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
base: trunk
Are you sure you want to change the base?
Conversation
|
Ping @millerjp as well |
|
I've run this through our wheel build process for Python 3.12 against ARM64 and x86_64 Ubuntu. In both cases all expected native code was preserved. Might still have a problem though: I mean... at least libev worked... |
|
I removed the the "-s" arg to gcc from CFLAGS in order to diagnose the problem: and the problem... went away?!?!?!?!?! |
|
Worth noting that the wheels for 3.29.3 available from pypi also have this issue. Wheels for 3.29.2 from pypi do not have this problem. We introduced the stripping of symbols on 7/30/2024 as the fix for PYTHON-1387. Weird thing is that 3.29.2 was released in September of 2024 so wheels built there would've included that fix.... and they appear fine. I can't really explain any of this yet. The error itself doesn't make sense... and the chronology doesn't introduce an obvious point where a regression like this could've come in. |
|
Moving discussion about the wheel build issue to a distinct issue on python-driver-wheels. Let's keep this PR focused on conversation about the pyproject.toml conversion. |
|
I've confirmed that these changes (combined with an equivalent set of changes for python-driver-wheels) appear to build valid wheels (including the Cython modules that were missing in CASSPYTHON-3) for all platforms. |
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.
Pull request overview
This PR migrates the build configuration from a setup.py-centric approach to a modern pyproject.toml-based approach, addressing deprecation of distutils and explicitly declaring build dependencies. The changes significantly refactor setup.py by removing deprecated distutils imports, custom build commands, and pre-build checks while moving project metadata into pyproject.toml.
Changes:
- Introduced pyproject.toml with build-system configuration, project metadata, and custom tool configuration for build options
- Refactored setup.py to remove distutils usage, simplify extension building logic, and read configuration from pyproject.toml
- Removed custom commands (DocCommand), pre-build checks, and complex error handling from setup.py
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| pyproject.toml | New file defining build system requirements, project metadata, dependencies, and custom cassandra-driver build configuration options |
| setup.py | Significantly simplified to focus on extension building, now reads configuration from pyproject.toml and removes deprecated distutils usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,57 @@ | |||
| [build-system] | |||
| build-backend = "setuptools.build_meta" | |||
| requires = ["setuptools", "Cython>=3.0", "toml", "cassandra-driver"] | |||
Copilot
AI
Jan 13, 2026
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.
Critical circular dependency issue: The build-system requires "cassandra-driver" as a build dependency, but this is the package being built. This will cause build failures as pip cannot install cassandra-driver before building cassandra-driver. The "cassandra-driver" entry should be removed from the requires list.
| requires = ["setuptools", "Cython>=3.0", "toml", "cassandra-driver"] | |
| requires = ["setuptools", "Cython>=3.0", "toml"] |
| class build_extensions(build_ext): | ||
|
|
||
| error_message = """ | ||
| windows_compmile_error = \ |
Copilot
AI
Jan 13, 2026
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.
Spelling error: "compmile" should be "compile".
| windows_compmile_error = \ | |
| windows_compile_error = \ |
| try_murmur3 = driver_project_data["build-murmur3-extension"] | ||
| try_libev = driver_project_data["build-libev-extension"] | ||
| try_cython = driver_project_data["build-cython-extensions"] and not is_pypy | ||
|
|
Copilot
AI
Jan 13, 2026
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 refactored code removes support for command-line arguments that were present in the original: --no-murmur3, --no-libev, --no-cython, and --no-extensions. This is a breaking change that could affect users and build scripts that relied on these flags to control extension building. Consider either maintaining backward compatibility or documenting this breaking change clearly.
| # Backwards-compatible support for legacy command-line flags that used to | |
| # control extension building. These flags override the configuration in | |
| # pyproject.toml and are removed from sys.argv so that setuptools and other | |
| # tooling do not see them. | |
| legacy_flag_map = { | |
| "--no-murmur3": "murmur3", | |
| "--no-libev": "libev", | |
| "--no-cython": "cython", | |
| "--no-extensions": "all", | |
| } | |
| cleaned_argv = [] | |
| for _arg in sys.argv: | |
| flag_type = legacy_flag_map.get(_arg) | |
| if flag_type is None: | |
| cleaned_argv.append(_arg) | |
| continue | |
| if flag_type == "murmur3": | |
| try_murmur3 = False | |
| elif flag_type == "libev": | |
| try_libev = False | |
| elif flag_type == "cython": | |
| try_cython = False | |
| elif flag_type == "all": | |
| try_murmur3 = False | |
| try_libev = False | |
| try_cython = False | |
| sys.argv = cleaned_argv |
| setup( | ||
| version = __version__, | ||
| ext_modules = build_extension_list() | ||
| ) |
Copilot
AI
Jan 13, 2026
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 refactored code removes the check that warned users if the old standalone cqlengine package was installed alongside the driver. This warning helped prevent conflicts since cqlengine is now integrated into the driver. Removing this check could lead to confusing behavior for users who have both packages installed. Consider whether this warning is still needed or if it can be safely removed.
| @@ -0,0 +1,57 @@ | |||
| [build-system] | |||
| build-backend = "setuptools.build_meta" | |||
| requires = ["setuptools", "Cython>=3.0", "toml", "cassandra-driver"] | |||
Copilot
AI
Jan 13, 2026
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 build-system requires "Cython>=3.0" unconditionally, but in the original code, Cython was only added to setup_requires when actually needed (when try_cython was True and pre_build_check passed). This means Cython will now always be installed during the build process, even if the user doesn't want to build Cython extensions. This could increase build time and dependencies unnecessarily. Consider making Cython an optional build dependency or documenting why it's now always required.
| setup( | ||
| version = __version__, | ||
| ext_modules = build_extension_list() | ||
| ) |
Copilot
AI
Jan 13, 2026
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 refactored code removes the DocCommand custom command that was used to generate and test documentation via python setup.py doc or python setup.py doc --test. This is a breaking change that could affect documentation build workflows. Consider either maintaining this functionality or documenting the migration path for documentation building.
| has_subprocess = False | ||
| from pathlib import Path | ||
| from setuptools import setup, Extension | ||
| import toml |
Copilot
AI
Jan 13, 2026
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 code imports and uses the toml library, which is also listed as a build dependency. However, for Python 3.11+, the standard library includes tomllib for reading TOML files. For Python 3.9-3.10, the recommended approach is to use tomli (the backport of tomllib) rather than the older toml package. Consider using tomllib (with tomli as a fallback for older Python versions) for better compatibility and future-proofing.
| import toml | |
| try: | |
| import tomllib as toml | |
| except ImportError: | |
| import tomli as toml |
| except Exception: | ||
| sys.stderr.write("Failed to cythonize one or more modules. These will not be compiled as extensions (optional).\n") |
Copilot
AI
Jan 13, 2026
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 exception handler at line 172 catches all exceptions with a bare except Exception:, which will silently suppress all errors during Cythonization. While this matches the original behavior, it would be better to log the actual exception details (e.g., using sys.stderr.write(str(exc))) to help users diagnose build issues rather than just printing a generic message.
| except Exception: | |
| sys.stderr.write("Failed to cythonize one or more modules. These will not be compiled as extensions (optional).\n") | |
| except Exception as exc: | |
| sys.stderr.write("Failed to cythonize one or more modules. These will not be compiled as extensions (optional).\n") | |
| sys.stderr.write("Cythonization error: %s\n" % (exc,)) |
| try_murmur3 = driver_project_data["build-murmur3-extension"] | ||
| try_libev = driver_project_data["build-libev-extension"] | ||
| try_cython = driver_project_data["build-cython-extensions"] and not is_pypy |
Copilot
AI
Jan 13, 2026
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 logic for try_murmur3 and try_libev does not respect the try_extensions flag. These extensions could be built even when try_extensions is False (e.g., on unsupported platforms). These variables should be combined with try_extensions similar to how it was done in the original code. For example: try_murmur3 = try_extensions and driver_project_data["build-murmur3-extension"].
| try_murmur3 = driver_project_data["build-murmur3-extension"] | |
| try_libev = driver_project_data["build-libev-extension"] | |
| try_cython = driver_project_data["build-cython-extensions"] and not is_pypy | |
| try_murmur3 = try_extensions and driver_project_data["build-murmur3-extension"] | |
| try_libev = try_extensions and driver_project_data["build-libev-extension"] | |
| try_cython = try_extensions and driver_project_data["build-cython-extensions"] and not is_pypy |
| "Programming Language :: Python :: Implementation :: CPython", | ||
| "Programming Language :: Python :: Implementation :: PyPy", | ||
| "Topic :: Software Development :: Libraries :: Python Modules" | ||
| ] |
Copilot
AI
Jan 13, 2026
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 pyproject.toml is missing a requires-python field to specify the minimum Python version required. While the classifiers list Python 3.9-3.13, the requires-python field should be explicitly set (e.g., requires-python = ">=3.9") to ensure pip enforces this requirement during installation.
| ] | |
| ] | |
| requires-python = ">=3.9" |
Original motivation is CASSPYTHON-3, a lifted version of PYTHON-1428. These changes also take care of PYTHON-1334 ("Consider migrating dependency, other information in setup.py into pyproject.toml") and PYTHON-1421 ("Adapt setup.py to deprecation of distutils")
This PR represents a pretty significant re-working of setup.py so the question of what kind of release it belongs in is a very real one. The original motivation was to fix a specific problem with the wheels for 3.29.3 but so far I haven't found a way to do that other than the mechanism outlined here. That question is being considered on CASSPYTHON-3; we'll use this PR to discuss and review the changes themselves.