Update Jenkins build to work with pyproject.toml#1271
Update Jenkins build to work with pyproject.toml#1271absurdfarce merged 1 commit intoapache:trunkfrom
Conversation
absurdfarce
left a comment
There was a problem hiding this comment.
Mainly explanation
| base["libev-includes"] = [sys.argv[2]] | ||
| base["libev-libs"] = [sys.argv[3]] | ||
|
|
||
| print(toml.dumps(pyproject)) |
There was a problem hiding this comment.
Helper script to update pyproject.toml to point at the correct location for libev includes/libs. Called in Jenkinsfile below.
| DEFAULT_DSE = ['dse-5.1.35', 'dse-6.8.30', 'dse-6.9.0'] | ||
| DEFAULT_HCD = ['hcd-1.0.0'] | ||
| DEFAULT_RUNTIME = ['3.9.23', '3.10.18', '3.11.13', '3.12.11', '3.13.5'] | ||
| DEFAULT_RUNTIME = ['3.10.18', '3.11.13', '3.12.11', '3.13.5'] |
There was a problem hiding this comment.
Bumped minimum version to 3.10 since we won't be officially supporting 3.9 with Python driver 3.30.0. We'll need to add 3.14 separately but I need to get that version installed on the Jenkins runners first.
There was a problem hiding this comment.
Why not install uv then use that to install 3.14 + dependencies, all as part of the jenkins flow?
There was a problem hiding this comment.
A fair question @Dev-iL. The Python versions listed here correspond to pre-installed Python versions on the Jenkins runner. We don't want to pay the cost to download and install a runtime on every CI run so we pre-bake Python versions into the images Jenkins uses to run tests.
| # Install a version of pyyaml<6.0 compatible with ccm-3.1.5 as of Aug 2023 | ||
| # this works around the python-3.10+ compatibility problem as described in DSP-23524 | ||
| pip install wheel | ||
| pip install "Cython<3.0" "pyyaml<6.0" --no-build-isolation |
There was a problem hiding this comment.
Cython is now handled by the build backend so there's no need to install it in the venv here
| python -m venv libev-venv | ||
| . ./libev-venv/bin/activate | ||
| pip install toml | ||
| python fix-jenkinsfile-libev.py ./pyproject.toml "/usr/include" "/usr/lib/x86_64-linux-gnu" | sponge ./pyproject.toml |
There was a problem hiding this comment.
The heart of the matter. These paths match the install location for the package installs used on the Jenkins runners.
Note that this is done in a separate venv to avoid contaminating the actual build venv with unrelated packages.
| set +o allexport | ||
|
|
||
| cat ./pyproject.toml | ||
| pip install --verbose --editable . |
There was a problem hiding this comment.
We need a local install of the built driver; without this pytest will discover (and apparently prefer) the local versions of the corresponding Python packages so unless the built native libs are also available in the same directory structure we'll get lots of complaints about libev being unavailable.
| libev-includes = [] | ||
| libev-libs = [] | ||
| libev-includes = ["/usr/include/libev", "/usr/local/include", "/opt/local/include", "/usr/include"] | ||
| libev-libs = ["/usr/local/lib", "/opt/local/lib", "/usr/lib64"] |
There was a problem hiding this comment.
We used to have these defaults embedded in setup.py which we would apply when the appropriate conditions were met. In the spirit of making the build more explicit it seemed better to just put these values directly in pyproject.toml as defaults.
|
Hey @bschoening I'm guessing you're not super-concerned about the Jenkinsfile changes but I would like your feedback on the pyproject.toml/setup.py changes embedded here. |
|
Ping @millerjp for visibility |
| pyenv shell ${PYTHON_VERSION} | ||
| python -m venv jenkins-venv | ||
| . ./jenkins-venv/bin/activate |
There was a problem hiding this comment.
I moved everything in to a contained venv because we were seeing weird behaviours in which pyenv wasn't necessarily updating the local Python instance which in turn led to packages being applied inconsistently. To avoid all of that it seemed worthwhile to be very explicit about the idea of a container for all necessary Python dependencies... which is pretty much a venv.
setup.py
Outdated
| include_dirs=libev_includes, | ||
| libraries=['ev'], | ||
| library_dirs=libev_libs, | ||
| runtime_library_dirs=libev_libs) |
There was a problem hiding this comment.
I discovered this option in the setuptools docs. At the time I thought the underlying problem was that native shared objects were getting compiled without proper references to their dependencies, a situation which this config addresses exactly. That wasn't actually the underlying problem, however, and since this config creates a problem on Windows builds (since Windows compilers apparently don't support anything like this) I'm gonna try a test build without it. Assuming there isn't any regression I'm gonna argue to just keep it out.
There was a problem hiding this comment.
Confirmed that build still looks good after removal of this option so we're going to go without it.
patch by Bret McGuire; reviewed by Bret McGuire and Brad Schoening reference: apache#1271
08d5f36 to
897731a
Compare
No description provided.